Bug 816784 part 2 - Optimize NukeCrossCompartmentWrappers() to iterate only the targetting wrappers. r?jonco draft
authorTing-Yu Chou <janus926@gmail.com>
Wed, 03 May 2017 13:56:54 +0800
changeset 580986 14fe55de8a7768d745b34e26eabc208fd7d4960b
parent 580124 710e0a43f09220aa3d8d74ef3c3e99387208afab
child 580987 b0f5dd1093fb720149dcf3dc45945d21a650cc5e
child 581011 d4c6706c597bde50a75533f55ab3df4f139799a3
child 581019 4ff88266cd96cb678690e0986e1905580015a3e4
child 581021 b5558701487fe4fe30023a4fa47a6d26f68cdb3a
child 581022 540c816dff8d7d831de43578605f3c98b587b285
child 582208 c2846145a6d5765366e41bf8c71c50982b7f93f6
push id59738
push userbmo:janus926@gmail.com
push dateFri, 19 May 2017 06:47:57 +0000
reviewersjonco
bugs816784
milestone55.0a1
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
dom/base/nsGlobalWindow.cpp
js/src/jsfriendapi.h
js/src/proxy/CrossCompartmentWrapper.cpp
js/xpconnect/src/XPCJSRuntime.cpp
--- 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);