Bug 1545699 - Don't sync-reload chrome:// user-agent stylesheets from nsChromeRegistry. r=bzbarsky
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 19 Apr 2019 16:30:00 +0200
changeset 530111 24ad41213ce44bef9fe3d7f8af56ad9ca38e941f
parent 530110 25176eee4a95239a5bf2c6558fe0ac46cd6df594
child 530112 9d3956ea78a8ddb36e0ee683e9f4ef7cc604e7c9
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)
reviewersbzbarsky
bugs1545699
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 1545699 - Don't sync-reload chrome:// user-agent stylesheets from nsChromeRegistry. r=bzbarsky We assume in a bunch of other places that user agent stylesheets don't really change dynamically. It's not clear to me what this code is trying to accomplish or how is it supposed to work in a multi-process world, but I've left the author stylesheets code for now... Also, I'm pretty sure the styleset doesn't handle null sheets, so add a null-check to the author stylesheets. Differential Revision: https://phabricator.services.mozilla.com/D28211
chrome/nsChromeRegistry.cpp
dom/base/Document.cpp
dom/base/Document.h
--- a/chrome/nsChromeRegistry.cpp
+++ b/chrome/nsChromeRegistry.cpp
@@ -334,80 +334,50 @@ nsresult nsChromeRegistry::RefreshWindow
   // Deal with our subframes first.
   nsDOMWindowList* frames = aWindow->GetFrames();
   uint32_t length = frames->GetLength();
   for (uint32_t j = 0; j < length; j++) {
     nsCOMPtr<nsPIDOMWindowOuter> piWindow = frames->IndexedGetter(j);
     RefreshWindow(piWindow);
   }
 
-  nsresult rv;
   // Get the document.
   RefPtr<Document> document = aWindow->GetDoc();
   if (!document) return NS_OK;
 
-  // Deal with the agent sheets first.  Have to do all the style sets by hand.
-  RefPtr<PresShell> presShell = document->GetPresShell();
-  if (presShell) {
-    // Reload only the chrome URL agent style sheets.
-    nsTArray<RefPtr<StyleSheet>> agentSheets;
-    rv = presShell->GetAgentStyleSheets(agentSheets);
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    nsTArray<RefPtr<StyleSheet>> newAgentSheets;
-    for (StyleSheet* sheet : agentSheets) {
-      nsIURI* uri = sheet->GetSheetURI();
-
-      if (IsChromeURI(uri)) {
-        // Reload the sheet.
-        RefPtr<StyleSheet> newSheet;
-        rv = document->LoadChromeSheetSync(uri, true, &newSheet);
-        if (NS_FAILED(rv)) return rv;
-        if (newSheet) {
-          newAgentSheets.AppendElement(newSheet);
-          return NS_OK;
-        }
-      } else {  // Just use the same sheet.
-        rv = newAgentSheets.AppendElement(sheet) ? NS_OK : NS_ERROR_FAILURE;
-        if (NS_FAILED(rv)) return rv;
-      }
-    }
-
-    rv = presShell->SetAgentStyleSheets(newAgentSheets);
-    NS_ENSURE_SUCCESS(rv, rv);
-  }
-
   size_t count = document->SheetCount();
 
   // Build an array of style sheets we need to reload.
   nsTArray<RefPtr<StyleSheet>> oldSheets(count);
   nsTArray<RefPtr<StyleSheet>> newSheets(count);
 
   // Iterate over the style sheets.
   for (size_t i = 0; i < count; i++) {
     // Get the style sheet
     oldSheets.AppendElement(document->SheetAt(i));
   }
 
   // Iterate over our old sheets and kick off a sync load of the new
   // sheet if and only if it's a non-inline sheet with a chrome URL.
+  //
+  // FIXME(emilio): What about user sheets? Also, does this do anything useful
+  // anymore?
   for (StyleSheet* sheet : oldSheets) {
     MOZ_ASSERT(sheet,
                "SheetAt shouldn't return nullptr for "
                "in-range sheet indexes");
     nsIURI* uri = sheet->GetSheetURI();
 
     if (!sheet->IsInline() && IsChromeURI(uri)) {
       // Reload the sheet.
-      RefPtr<StyleSheet> newSheet;
       // XXX what about chrome sheets that have a title or are disabled?  This
       // only works by sheer dumb luck.
-      document->LoadChromeSheetSync(uri, false, &newSheet);
-      // Even if it's null, we put in in there.
-      newSheets.AppendElement(newSheet);
+      if (RefPtr<StyleSheet> newSheet = document->LoadChromeSheetSync(uri)) {
+        newSheets.AppendElement(newSheet);
+      }
     } else {
       // Just use the same sheet.
       newSheets.AppendElement(sheet);
     }
   }
 
   // Now notify the document that multiple sheets have been added and removed.
   document->UpdateStyleSheets(oldSheets, newSheets);
--- a/dom/base/Document.cpp
+++ b/dom/base/Document.cpp
@@ -8742,21 +8742,20 @@ void Document::PreloadStyle(
   nsCOMPtr<nsICSSLoaderObserver> obs = new StubCSSLoaderObserver();
 
   // Charset names are always ASCII.
   CSSLoader()->LoadSheet(uri, true, NodePrincipal(), aEncoding, obs,
                          Element::StringToCORSMode(aCrossOriginAttr),
                          aReferrerPolicy, aIntegrity);
 }
 
-nsresult Document::LoadChromeSheetSync(nsIURI* uri, bool isAgentSheet,
-                                       RefPtr<mozilla::StyleSheet>* aSheet) {
-  css::SheetParsingMode mode =
-      isAgentSheet ? css::eAgentSheetFeatures : css::eAuthorSheetFeatures;
-  return CSSLoader()->LoadSheetSync(uri, mode, isAgentSheet, aSheet);
+RefPtr<StyleSheet> Document::LoadChromeSheetSync(nsIURI* uri) {
+  RefPtr<StyleSheet> sheet;
+  CSSLoader()->LoadSheetSync(uri, css::eAuthorSheetFeatures, false, &sheet);
+  return sheet;
 }
 
 void Document::ResetDocumentDirection() {
   if (!(nsContentUtils::IsChromeDoc(this) || IsXULDocument())) {
     return;
   }
   UpdateDocumentStates(NS_DOCUMENT_STATE_RTL_LOCALE, true);
 }
--- a/dom/base/Document.h
+++ b/dom/base/Document.h
@@ -2848,35 +2848,30 @@ class Document : public nsINode,
 
   /**
    * Called by images to forget an image preload when they start doing
    * the real load.
    */
   void ForgetImagePreload(nsIURI* aURI);
 
   /**
-   * Called by nsParser to preload style sheets.  Can also be merged into the
-   * parser if and when the parser is merged with libgklayout.  aCrossOriginAttr
-   * should be a void string if the attr is not present.
+   * Called by nsParser to preload style sheets.  aCrossOriginAttr should be a
+   * void string if the attr is not present.
    */
   void PreloadStyle(nsIURI* aURI, const Encoding* aEncoding,
                     const nsAString& aCrossOriginAttr,
                     ReferrerPolicyEnum aReferrerPolicy,
                     const nsAString& aIntegrity);
 
   /**
-   * Called by the chrome registry to load style sheets.  Can be put
-   * back there if and when when that module is merged with libgklayout.
+   * Called by the chrome registry to load style sheets.
    *
-   * This always does a synchronous load.  If aIsAgentSheet is true,
-   * it also uses the system principal and enables unsafe rules.
-   * DO NOT USE FOR UNTRUSTED CONTENT.
-   */
-  nsresult LoadChromeSheetSync(nsIURI* aURI, bool aIsAgentSheet,
-                               RefPtr<StyleSheet>* aSheet);
+   * This always does a synchronous load, and parses as a normal document sheet.
+   */
+  RefPtr<StyleSheet> LoadChromeSheetSync(nsIURI* aURI);
 
   /**
    * Returns true if the locale used for the document specifies a direction of
    * right to left. For chrome documents, this comes from the chrome registry.
    * This is used to determine the current state for the :-moz-locale-dir
    * pseudoclass so once can know whether a document is expected to be rendered
    * left-to-right or right-to-left.
    */