Bug 1549783 - Ensure that cached contexts come after non-cached ones in EnsureSubscribed, r=farre
authorNika Layzell <nika@thelayzells.com>
Wed, 08 May 2019 07:24:31 +0000
changeset 531924 790d1bff41400ac2266adb5340b8911e2251d6d3
parent 531923 eb384663078998538a5af4dfa7f45972fe29da49
child 531925 2f50e7dfbc564477f53313400d953cf2381e1cdd
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfarre
bugs1549783
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1549783 - Ensure that cached contexts come after non-cached ones in EnsureSubscribed, r=farre Differential Revision: https://phabricator.services.mozilla.com/D30230
docshell/base/BrowsingContextGroup.cpp
--- a/docshell/base/BrowsingContextGroup.cpp
+++ b/docshell/base/BrowsingContextGroup.cpp
@@ -38,40 +38,43 @@ void BrowsingContextGroup::Unsubscribe(C
 }
 
 void BrowsingContextGroup::EnsureSubscribed(ContentParent* aProcess) {
   MOZ_DIAGNOSTIC_ASSERT(aProcess);
   if (mSubscribers.Contains(aProcess)) {
     return;
   }
 
-  // Subscribe to the BrowsingContext, and send down initial state!
   Subscribe(aProcess);
 
-  // Iterate over each of our browsing contexts, locating those which are not in
-  // their parent's children list. We can then use those as starting points to
-  // get a pre-order walk of each tree.
   nsTArray<BrowsingContext::IPCInitializer> inits(mContexts.Count());
-  for (auto iter = mContexts.Iter(); !iter.Done(); iter.Next()) {
-    auto* context = iter.Get()->GetKey();
 
-    // If we have a parent, and are in our parent's `Children` list, skip
-    // ourselves as we'll be found in the pre-order traversal of our parent.
-    if (context->GetParent() &&
-        context->GetParent()->GetChildren().IndexOf(context) !=
-            BrowsingContext::Children::NoIndex) {
-      continue;
-    }
+  // First, perform a pre-order walk of our BrowsingContext objects from our
+  // toplevels. This should visit every active BrowsingContext.
+  for (auto& context : mToplevels) {
+    MOZ_DIAGNOSTIC_ASSERT(!IsContextCached(context),
+                          "cached contexts must have a parent");
 
-    // Add all elements to the list in pre-order.
     context->PreOrderWalk([&](BrowsingContext* aContext) {
       inits.AppendElement(aContext->GetIPCInitializer());
     });
   }
 
+  // Ensure that cached BrowsingContext objects are also visited, by visiting
+  // them after mToplevels.
+  for (auto iter = mCachedContexts.Iter(); !iter.Done(); iter.Next()) {
+    iter.Get()->GetKey()->PreOrderWalk([&](BrowsingContext* aContext) {
+      inits.AppendElement(aContext->GetIPCInitializer());
+    });
+  }
+
+  // We should have visited every browsing context.
+  MOZ_DIAGNOSTIC_ASSERT(inits.Length() == mContexts.Count(),
+                        "Visited the wrong number of contexts!");
+
   // Send all of our contexts to the target content process.
   Unused << aProcess->SendRegisterBrowsingContextGroup(inits);
 }
 
 bool BrowsingContextGroup::IsContextCached(BrowsingContext* aContext) const {
   MOZ_DIAGNOSTIC_ASSERT(aContext);
   return mCachedContexts.Contains(aContext);
 }