Bug 816784 part 2 - Optimize NukeCrossCompartmentWrappers() to iterate only the targetting wrappers. r?jonco
Changed the type of argument |targetFilter| of NukeCrossCompartmentWrappers()
from CompartmentFilter to JSCompartment* because it is always a single target
compartment, and we can optimize the iteration not to iterate the outer map.
MozReview-Commit-ID: 7cDCgJI0H9z
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -9628,18 +9628,17 @@ public:
nsAutoString addonId;
if (NS_SUCCEEDED(pc->GetAddonId(addonId)) && !addonId.IsEmpty()) {
// We want to nuke all references to the add-on compartment.
xpc::NukeAllWrappersForCompartment(cx, cpt,
win->IsInnerWindow() ? js::DontNukeWindowReferences
: js::NukeWindowReferences);
} else {
// We only want to nuke wrappers for the chrome->content case
- js::NukeCrossCompartmentWrappers(cx, BrowserCompartmentMatcher(),
- js::SingleCompartment(cpt),
+ js::NukeCrossCompartmentWrappers(cx, BrowserCompartmentMatcher(), cpt,
win->IsInnerWindow() ? js::DontNukeWindowReferences
: js::NukeWindowReferences,
js::NukeIncomingReferences);
}
}
}
return NS_OK;
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -1226,17 +1226,17 @@ struct CompartmentsWithPrincipals : publ
virtual bool match(JSCompartment* c) const override {
return JS_GetCompartmentPrincipals(c) == principals;
}
};
extern JS_FRIEND_API(bool)
NukeCrossCompartmentWrappers(JSContext* cx,
const CompartmentFilter& sourceFilter,
- const CompartmentFilter& targetFilter,
+ JSCompartment* target,
NukeReferencesToWindow nukeReferencesToWindow,
NukeReferencesFromTarget nukeReferencesFromTarget);
/* Specify information about DOMProxy proxies in the DOM, for use by ICs. */
/*
* The DOMProxyShadowsCheck function will be called to check if the property for
* id should be gotten from the prototype, or if there is an own property that
--- a/js/src/proxy/CrossCompartmentWrapper.cpp
+++ b/js/src/proxy/CrossCompartmentWrapper.cpp
@@ -510,64 +510,74 @@ js::NukeCrossCompartmentWrapper(JSContex
* obj's global. The snag here is that we need to avoid cutting wrappers that
* point to the window object on page navigation (inner window destruction)
* and only do that on tab close (outer window destruction). Thus the
* option of how to handle the global object.
*/
JS_FRIEND_API(bool)
js::NukeCrossCompartmentWrappers(JSContext* cx,
const CompartmentFilter& sourceFilter,
- const CompartmentFilter& targetFilter,
+ JSCompartment* target,
js::NukeReferencesToWindow nukeReferencesToWindow,
js::NukeReferencesFromTarget nukeReferencesFromTarget)
{
CHECK_REQUEST(cx);
JSRuntime* rt = cx->runtime();
EvictAllNurseries(rt);
for (CompartmentsIter c(rt, SkipAtoms); !c.done(); c.next()) {
if (!sourceFilter.match(c))
continue;
// If the compartment matches both the source and target filter, we may
// want to cut both incoming and outgoing wrappers.
bool nukeAll = (nukeReferencesFromTarget == NukeAllReferences &&
- targetFilter.match(c));
+ target == c.get());
- // Iterate the wrappers looking for anything interesting.
- for (JSCompartment::WrapperEnum e(c); !e.empty(); e.popFront()) {
- // Some cross-compartment wrappers are for strings. We're not
- // interested in those.
- const CrossCompartmentKey& k = e.front().key();
+ // Iterate only the wrappers that have target compartment matched unless
+ // |nukeAll| is true. The wrappers for strings that we're not interested
+ // in won't be here because they have compartment nullptr. Use Maybe to
+ // avoid copying from conditionally initializing WrapperEnum.
+ mozilla::Maybe<JSCompartment::WrapperEnum> e;
+ if (MOZ_LIKELY(!nukeAll))
+ e.emplace(c, target);
+ else
+ e.emplace(c);
+ for (; !e->empty(); e->popFront()) {
+ // Skip debugger references because NukeCrossCompartmentWrapper()
+ // doesn't know how to nuke them yet, see bug 1084626 for more
+ // information.
+ const CrossCompartmentKey& k = e->front().key();
if (!k.is<JSObject*>())
continue;
- AutoWrapperRooter wobj(cx, WrapperValue(e));
- JSObject* wrapped = UncheckedUnwrap(wobj);
+ AutoWrapperRooter wobj(cx, WrapperValue(*e));
+
+ // Unwrap from the wrapped object in CrossCompartmentKey instead of
+ // the wrapper, this could save us a bit of time.
+ JSObject* wrapped = UncheckedUnwrap(k.as<JSObject*>());
// We never nuke script source objects, since only ever used internally by the JS
// engine, and are expected to remain valid throughout a scripts lifetime.
if (MOZ_UNLIKELY(wrapped->is<ScriptSourceObject>())) {
continue;
}
// We only skip nuking window references that point to a target
// compartment, not the ones that belong to it.
if (nukeReferencesToWindow == DontNukeWindowReferences &&
MOZ_LIKELY(!nukeAll) && IsWindowProxy(wrapped))
{
continue;
}
- if (MOZ_UNLIKELY(nukeAll) || targetFilter.match(wrapped->compartment())) {
- // We found a wrapper to nuke.
- e.removeFront();
- NukeCrossCompartmentWrapper(cx, wobj);
- }
+ // Now this is the wrapper we want to nuke.
+ e->removeFront();
+ NukeCrossCompartmentWrapper(cx, wobj);
}
}
return true;
}
// Given a cross-compartment wrapper |wobj|, update it to point to
// |newTarget|. This recomputes the wrapper with JS_WrapValue, and thus can be
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -588,18 +588,17 @@ NukeAllWrappersForCompartment(JSContext*
js::NukeReferencesToWindow nukeReferencesToWindow)
{
// First, nuke all wrappers into or out of the target compartment. Once
// the compartment is marked as nuked, WrapperFactory will refuse to
// create new live wrappers for it, in either direction. This means that
// we need to be sure that we don't have any existing cross-compartment
// wrappers which may be replaced with dead wrappers during unrelated
// wrapper recomputation *before* we set that bit.
- js::NukeCrossCompartmentWrappers(cx, js::AllCompartments(),
- js::SingleCompartment(compartment),
+ js::NukeCrossCompartmentWrappers(cx, js::AllCompartments(), compartment,
nukeReferencesToWindow,
js::NukeAllReferences);
// At this point, we should cross-compartment wrappers for the nuked
// compartment. Set the wasNuked bit so WrapperFactory will return a
// DeadObjectProxy when asked to create a new wrapper for it, and mark as
// unscriptable.
auto compartmentPrivate = xpc::CompartmentPrivate::Get(compartment);