Bug 1382357 - Wait to destroy frames until after we've successfully fetched the binding. r=heycam
authorBobby Holley <bobbyholley@gmail.com>
Wed, 19 Jul 2017 17:27:51 -0700
changeset 418535 5a43e98b2000fa178b0454cb005dff11762340f4
parent 418534 06e2da701e49f5a474bac8a1e819b82ccac86dc6
child 418536 747035414236731b91080e801f393be22017a5b8
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1382357
milestone56.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 1382357 - Wait to destroy frames until after we've successfully fetched the binding. r=heycam The issue here is that the DestroyFramesFor call does a ClearServoDataFromSubtree, and then we subsequently fail to load the bindings, leaving ourselves with an unstyled subtree that isn't marked with descendant bits and isn't rooted at a display:none element, which is forbidden. This can trigger crashes when we call the innerText getter on one of the unstyled elements, since that will first flush layout (which won't find the unstyled subtree), and then invoke IsOrHasAncestorWithDisplayNone, which passes LazyComputeBehavior::Assert. MozReview-Commit-ID: 7roBkH9fTGZ
dom/xbl/nsXBLService.cpp
--- a/dom/xbl/nsXBLService.cpp
+++ b/dom/xbl/nsXBLService.cpp
@@ -111,30 +111,32 @@ public:
   void DocumentLoaded(nsIDocument* aBindingDoc)
   {
     // We only need the document here to cause frame construction, so
     // we need the current doc, not the owner doc.
     nsIDocument* doc = mBoundElement->GetUncomposedDoc();
     if (!doc)
       return;
 
-    // Destroy the frames for mBoundElement.
+    // Get the binding.
+    bool ready = false;
+    nsXBLService::GetInstance()->BindingReady(mBoundElement, mBindingURI, &ready);
+    if (!ready)
+      return;
+
+    // Destroy the frames for mBoundElement. Do this after getting the binding,
+    // since if the binding fetch fails then we don't want to destroy the
+    // frames.
     nsIContent* destroyedFramesFor = nullptr;
     nsIPresShell* shell = doc->GetShell();
     if (shell) {
       shell->DestroyFramesFor(mBoundElement, &destroyedFramesFor);
     }
     MOZ_ASSERT(!mBoundElement->GetPrimaryFrame());
 
-    // Get the binding.
-    bool ready = false;
-    nsXBLService::GetInstance()->BindingReady(mBoundElement, mBindingURI, &ready);
-    if (!ready)
-      return;
-
     // If |mBoundElement| is (in addition to having binding |mBinding|)
     // also a descendant of another element with binding |mBinding|,
     // then we might have just constructed it due to the
     // notification of its parent.  (We can know about both if the
     // binding loads were triggered from the DOM rather than frame
     // construction.)  So we have to check both whether the element
     // has a primary frame and whether it's in the frame manager maps
     // before sending a ContentInserted notification, or bad things