Bug 1166351 - Prioritize getting the nsXBLDocumentInfo out of the bound document's nsBindingManager instead of the nsXULPrototypeCache. r?bholley draft
authorMike Conley <mconley@mozilla.com>
Mon, 16 May 2016 19:07:26 -0400
changeset 368570 29756024260ab416b5d1723e33311c4a7f4c6aa3
parent 368390 999e489461931506ef888c61891cd71de2f3d034
child 521323 cddba0f280c72bed2a3959abdc823e4a72d58222
push id18588
push usermconley@mozilla.com
push dateThu, 19 May 2016 02:50:33 +0000
reviewersbholley
bugs1166351, 990290
milestone49.0a1
Bug 1166351 - Prioritize getting the nsXBLDocumentInfo out of the bound document's nsBindingManager instead of the nsXULPrototypeCache. r?bholley This is kind of a long story, stay with me on this. In bug 990290, a WeakMap was added to any JS scope that loaded an XBL binding. That WeakMap stored the JS prototypes that are injected into a bound node's prototype chain. When a binding is removed, we search the prototype chain for the JS prototype that we'd added, and remove it. The XUL prototype cache caches numerous things, including nsXBLDocumentInfo, which we use to get at the nsXBLPrototypeBinding for a particular binding, which is then used to generate the class object that's put into the WeakMap. When the XUL prototype cache is flushed, that means that when a binding is bound, its definition needs to be reloaded off of the disk. If, however, there was a pre-existing instance of the binding already being used in a document, now we were in a funny case. We were in a funny case, because when attempting to remove a binding, we would look up the nsXBLPrototypeBinding via the nsXBLDocumentInfo that's being held within the nsXULPrototypeCache, find (or load off the disk) a _new_ nsXBLDocumentInfo and generate a _new_ nsXBLPrototypeBinding that did not match to the one that we'd already stored in the WeakMap. This would mean that removal would go wrong, and things would break horribly. This patch makes it so that we prioritize checking the nsBindingManager for a document for the nsXBLDocumentInfo before checking the global nsXULPrototypeCache. That way, even if the cache gets cleared, if the binding was ever used in this document, it'll be in the nsBindingManager, and we'll get the same nsXULProtoypeBinding that we'd been using before, and sanity will prevail. MozReview-Commit-ID: G8iLDbCPRAC
dom/xbl/nsXBLService.cpp
--- a/dom/xbl/nsXBLService.cpp
+++ b/dom/xbl/nsXBLService.cpp
@@ -853,127 +853,128 @@ nsXBLService::LoadBindingDocumentInfo(ns
   }
 
   RefPtr<nsXBLDocumentInfo> info;
 
   nsCOMPtr<nsIURI> documentURI;
   nsresult rv = aBindingURI->CloneIgnoringRef(getter_AddRefs(documentURI));
   NS_ENSURE_SUCCESS(rv, rv);
 
+  nsBindingManager *bindingManager = nullptr;
+
+  // The first thing to check is the binding manager, which (if it exists)
+  // should have a reference to the nsXBLDocumentInfo if this document
+  // has ever loaded this binding before.
+  if (aBoundDocument) {
+    bindingManager = aBoundDocument->BindingManager();
+    info = bindingManager->GetXBLDocumentInfo(documentURI);
+    if (aBoundDocument->IsStaticDocument() &&
+        IsChromeOrResourceURI(aBindingURI)) {
+      aForceSyncLoad = true;
+    }
+  }
+
+  // It's possible the document is already being loaded. If so, there's no
+  // document yet, but we need to glom on our request so that it will be
+  // processed whenever the doc does finish loading.
+  NodeInfo *ni = nullptr;
+  if (aBoundElement)
+    ni = aBoundElement->NodeInfo();
+
+  if (!info && bindingManager &&
+      (!ni || !(ni->Equals(nsGkAtoms::scrollbar, kNameSpaceID_XUL) ||
+                ni->Equals(nsGkAtoms::thumb, kNameSpaceID_XUL) ||
+                ((ni->Equals(nsGkAtoms::input) ||
+                  ni->Equals(nsGkAtoms::select)) &&
+                 aBoundElement->IsHTMLElement()))) && !aForceSyncLoad) {
+    nsCOMPtr<nsIStreamListener> listener;
+    if (bindingManager)
+      listener = bindingManager->GetLoadingDocListener(documentURI);
+    if (listener) {
+      nsXBLStreamListener* xblListener =
+        static_cast<nsXBLStreamListener*>(listener.get());
+      // Create a new load observer.
+      if (!xblListener->HasRequest(aBindingURI, aBoundElement)) {
+        nsXBLBindingRequest* req = new nsXBLBindingRequest(aBindingURI, aBoundElement);
+        xblListener->AddRequest(req);
+      }
+      return NS_OK;
+    }
+  }
+
 #ifdef MOZ_XUL
-  // We've got a file.  Check our XBL document cache.
+  // The second line of defense is the global nsXULPrototypeCache,
+  // if it's being used.
   nsXULPrototypeCache* cache = nsXULPrototypeCache::GetInstance();
   bool useXULCache = cache && cache->IsEnabled();
 
-  if (useXULCache) {
-    // The first line of defense is the chrome cache.
+  if (!info && useXULCache) {
     // This cache crosses the entire product, so that any XBL bindings that are
     // part of chrome will be reused across all XUL documents.
     info = cache->GetXBLDocumentInfo(documentURI);
   }
+
+  bool useStartupCache = useXULCache && IsChromeOrResourceURI(documentURI);
+
+  if (!info) {
+    // Next, look in the startup cache
+    if (!info && useStartupCache) {
+      rv = nsXBLDocumentInfo::ReadPrototypeBindings(documentURI, getter_AddRefs(info));
+      if (NS_SUCCEEDED(rv)) {
+        cache->PutXBLDocumentInfo(info);
+      }
+    }
+  }
 #endif
 
   if (!info) {
-    // The second line of defense is the binding manager's document table.
-    nsBindingManager *bindingManager = nullptr;
+    // Finally, if all lines of defense fail, we go and fetch the binding
+    // document.
 
-    if (aBoundDocument) {
-      bindingManager = aBoundDocument->BindingManager();
-      info = bindingManager->GetXBLDocumentInfo(documentURI);
-      if (aBoundDocument->IsStaticDocument() &&
-          IsChromeOrResourceURI(aBindingURI)) {
-        aForceSyncLoad = true;
-      }
-    }
-
-    NodeInfo *ni = nullptr;
-    if (aBoundElement)
-      ni = aBoundElement->NodeInfo();
+    // Always load chrome synchronously
+    bool chrome;
+    if (NS_SUCCEEDED(documentURI->SchemeIs("chrome", &chrome)) && chrome)
+      aForceSyncLoad = true;
 
-    if (!info && bindingManager &&
-        (!ni || !(ni->Equals(nsGkAtoms::scrollbar, kNameSpaceID_XUL) ||
-                  ni->Equals(nsGkAtoms::thumb, kNameSpaceID_XUL) ||
-                  ((ni->Equals(nsGkAtoms::input) ||
-                    ni->Equals(nsGkAtoms::select)) &&
-                   aBoundElement->IsHTMLElement()))) && !aForceSyncLoad) {
-      // The third line of defense is to investigate whether or not the
-      // document is currently being loaded asynchronously.  If so, there's no
-      // document yet, but we need to glom on our request so that it will be
-      // processed whenever the doc does finish loading.
-      nsCOMPtr<nsIStreamListener> listener;
-      if (bindingManager)
-        listener = bindingManager->GetLoadingDocListener(documentURI);
-      if (listener) {
-        nsXBLStreamListener* xblListener =
-          static_cast<nsXBLStreamListener*>(listener.get());
-        // Create a new load observer.
-        if (!xblListener->HasRequest(aBindingURI, aBoundElement)) {
-          nsXBLBindingRequest* req = new nsXBLBindingRequest(aBindingURI, aBoundElement);
-          xblListener->AddRequest(req);
-        }
-        return NS_OK;
+    nsCOMPtr<nsIDocument> document;
+    rv = FetchBindingDocument(aBoundElement, aBoundDocument, documentURI,
+                              aBindingURI, aOriginPrincipal, aForceSyncLoad,
+                              getter_AddRefs(document));
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    if (document) {
+      nsBindingManager *xblDocBindingManager = document->BindingManager();
+      info = xblDocBindingManager->GetXBLDocumentInfo(documentURI);
+      if (!info) {
+        NS_ERROR("An XBL file is malformed.  Did you forget the XBL namespace on the bindings tag?");
+        return NS_ERROR_FAILURE;
       }
-    }
+      xblDocBindingManager->RemoveXBLDocumentInfo(info); // Break the self-imposed cycle.
 
+      // If the doc is a chrome URI, then we put it into the XUL cache.
 #ifdef MOZ_XUL
-    // Next, look in the startup cache
-    bool useStartupCache = useXULCache && IsChromeOrResourceURI(documentURI);
-    if (!info && useStartupCache) {
-      rv = nsXBLDocumentInfo::ReadPrototypeBindings(documentURI, getter_AddRefs(info));
-      if (NS_SUCCEEDED(rv)) {
+      if (useStartupCache) {
         cache->PutXBLDocumentInfo(info);
 
-        if (bindingManager) {
-          // Cache it in our binding manager's document table.
-          bindingManager->PutXBLDocumentInfo(info);
-        }
+        // now write the bindings into the startup cache
+        info->WritePrototypeBindings();
       }
-    }
 #endif
-
-    if (!info) {
-      // Finally, if all lines of defense fail, we go and fetch the binding
-      // document.
-
-      // Always load chrome synchronously
-      bool chrome;
-      if (NS_SUCCEEDED(documentURI->SchemeIs("chrome", &chrome)) && chrome)
-        aForceSyncLoad = true;
-
-      nsCOMPtr<nsIDocument> document;
-      rv = FetchBindingDocument(aBoundElement, aBoundDocument, documentURI,
-                                aBindingURI, aOriginPrincipal, aForceSyncLoad,
-                                getter_AddRefs(document));
-      NS_ENSURE_SUCCESS(rv, rv);
+    }
+  }
 
-      if (document) {
-        nsBindingManager *xblDocBindingManager = document->BindingManager();
-        info = xblDocBindingManager->GetXBLDocumentInfo(documentURI);
-        if (!info) {
-          NS_ERROR("An XBL file is malformed.  Did you forget the XBL namespace on the bindings tag?");
-          return NS_ERROR_FAILURE;
-        }
-        xblDocBindingManager->RemoveXBLDocumentInfo(info); // Break the self-imposed cycle.
-
-        // If the doc is a chrome URI, then we put it into the XUL cache.
-#ifdef MOZ_XUL
-        if (useStartupCache) {
-          cache->PutXBLDocumentInfo(info);
-
-          // now write the bindings into the startup cache
-          info->WritePrototypeBindings();
-        }
-#endif
-
-        if (bindingManager) {
-          // Also put it in our binding manager's document table.
-          bindingManager->PutXBLDocumentInfo(info);
-        }
-      }
-    }
+  if (info && bindingManager) {
+    // Cache it in our binding manager's document table. This way,
+    // we can ensure that if the document has loaded this binding
+    // before, it can continue to use it even if the XUL prototype
+    // cache gets flushed. That way, if a flush does occur, we
+    // don't get into a weird state where we're using different
+    // XBLDocumentInfos for the same XBL document in a single
+    // document that has loaded some bindings.
+    bindingManager->PutXBLDocumentInfo(info);
   }
 
   info.forget(aResult);
 
   return NS_OK;
 }
 
 nsresult