Bug 1468133: Remove the optimization to lazily load non-SVG styles since it's not relevant anymore. r=heycam draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 11 Jun 2018 06:38:42 +0200
changeset 809240 ce6dfeda55b30276a22d65e3e5ef424ee8956c5d
parent 809239 59322b179c0fdb4423d42b624da7bf64ee5a2327
push id113595
push userbmo:emilio@crisal.io
push dateThu, 21 Jun 2018 14:28:59 +0000
reviewersheycam
bugs1468133, 686875, 77999, 18509, 1462742, 1157592, 1468145
milestone62.0a1
Bug 1468133: Remove the optimization to lazily load non-SVG styles since it's not relevant anymore. r=heycam This was a memory-saving optimization introduced as part of dependencies for bug 686875, but a more general system landed in bug 77999 for Gecko and https://github.com/servo/servo/pull/18509 for Servo. So now it's probably even a bit of a pessimization (though probably not huge), and given this causes bugs like bug 1462742, bug 1157592, and bug 1468145, and fishiness like the one pointed out in this bug, we may as well remove it. The performance impact of having to lookup through more rules should be minimal given the bloom filter and the rule hash optimizations. This makes me wonder whether we could remove the whole concept of on-demand UA sheets, since they've caused pain, for example, when the frontend people try loading <svg>s from NAC (since that triggers sheet loading from frame construction, which is not good). I'm not concerned about loading mathml.css and svg.css everywhere, though xul.css may not be as doable since it adds a bunch of attribute-dependent selectors. Though on the other hand I asserted in the xul.css code and we don't load it in content with <video> / <input type="date/time/etc"> and such, afaict, so maybe now that legacy addons are gone we can remove that sheet from content processes altogether. MozReview-Commit-ID: 9JCWNZj6BkT
dom/base/nsDocument.cpp
dom/base/nsIDocument.h
dom/svg/SVGDocument.cpp
dom/svg/SVGDocument.h
dom/svg/SVGForeignObjectElement.cpp
dom/svg/SVGForeignObjectElement.h
dom/svg/SVGSVGElement.cpp
layout/base/nsDocumentViewer.cpp
layout/printing/nsPrintJob.cpp
layout/svg/svg.css
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -5353,22 +5353,16 @@ already_AddRefed<AnonymousContent>
 nsIDocument::InsertAnonymousContent(Element& aElement, ErrorResult& aRv)
 {
   nsIPresShell* shell = GetShell();
   if (!shell || !shell->GetCanvasFrame()) {
     aRv.Throw(NS_ERROR_UNEXPECTED);
     return nullptr;
   }
 
-  // We're about to insert random content here that will be rendered. We're
-  // going to need more than svg.css here...
-  if (IsSVGDocument()) {
-    AsSVGDocument()->EnsureNonSVGUserAgentStyleSheetsLoaded();
-  }
-
   nsAutoScriptBlocker scriptBlocker;
   nsCOMPtr<Element> container = shell->GetCanvasFrame()
                                      ->GetCustomContentContainer();
   if (!container) {
     aRv.Throw(NS_ERROR_UNEXPECTED);
     return nullptr;
   }
 
--- a/dom/base/nsIDocument.h
+++ b/dom/base/nsIDocument.h
@@ -2012,17 +2012,23 @@ public:
     return mType == eXUL;
   }
   bool IsUnstyledDocument()
   {
     return IsLoadedAsData() || IsLoadedAsInteractiveData();
   }
   bool LoadsFullXULStyleSheetUpFront()
   {
-    return IsXULDocument() || AllowXULXBL();
+    if (IsXULDocument()) {
+      return true;
+    }
+    if (IsSVGDocument()) {
+      return false;
+    }
+    return AllowXULXBL();
   }
 
   bool IsScriptEnabled();
 
   bool IsTopLevelContentDocument() const { return mIsTopLevelContentDocument; }
   void SetIsTopLevelContentDocument(bool aIsTopLevelContentDocument)
   {
     mIsTopLevelContentDocument = aIsTopLevelContentDocument;
--- a/dom/svg/SVGDocument.cpp
+++ b/dom/svg/SVGDocument.cpp
@@ -25,141 +25,30 @@ using namespace mozilla::css;
 using namespace mozilla::dom;
 
 namespace mozilla {
 namespace dom {
 
 //----------------------------------------------------------------------
 // Implementation
 
-//----------------------------------------------------------------------
-// nsISupports methods:
-
-nsresult
-SVGDocument::InsertChildBefore(nsIContent* aKid, nsIContent* aBeforeThis,
-                               bool aNotify)
-{
-  if (aKid->IsElement() && !aKid->IsSVGElement()) {
-    // We can get here when well formed XML with a non-SVG root element is
-    // served with the SVG MIME type, for example. In that case we need to load
-    // the non-SVG UA sheets or else we can get bugs like bug 1016145.  Note
-    // that we have to do this _before_ the XMLDocument::InsertChildBefore,
-    // since that can try to construct frames, and we need to have the sheets
-    // loaded by then.
-    EnsureNonSVGUserAgentStyleSheetsLoaded();
-  }
-
-  return XMLDocument::InsertChildBefore(aKid, aBeforeThis, aNotify);
-}
-
 nsresult
 SVGDocument::Clone(mozilla::dom::NodeInfo *aNodeInfo, nsINode **aResult,
                    bool aPreallocateChildren) const
 {
   NS_ASSERTION(aNodeInfo->NodeInfoManager() == mNodeInfoManager,
                "Can't import this document into another document!");
 
   RefPtr<SVGDocument> clone = new SVGDocument();
   nsresult rv = CloneDocHelper(clone.get(), aPreallocateChildren);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return CallQueryInterface(clone.get(), aResult);
 }
 
-void
-SVGDocument::EnsureNonSVGUserAgentStyleSheetsLoaded()
-{
-  if (mHasLoadedNonSVGUserAgentStyleSheets) {
-    return;
-  }
-
-  if (IsStaticDocument()) {
-    // If we're a static clone of a document, then
-    // nsIDocument::CreateStaticClone will handle cloning the original
-    // document's sheets, including the on-demand non-SVG UA sheets,
-    // for us.
-    return;
-  }
-
-  mHasLoadedNonSVGUserAgentStyleSheets = true;
-
-  if (IsBeingUsedAsImage()) {
-    // nsDocumentViewer::CreateStyleSet skipped loading all user-agent/user
-    // style sheets in this case, but we'll need B2G/Fennec's
-    // content.css. We could load all the sheets registered with the
-    // nsIStyleSheetService (and maybe we should) but most likely it isn't
-    // desirable or necessary for foreignObject in SVG-as-an-image. Instead we
-    // only load the "agent-style-sheets" that nsStyleSheetService::Init()
-    // pulls in from the category manager. That keeps memory use of
-    // SVG-as-an-image down.
-    //
-    // We do this before adding the other sheets below because
-    // EnsureOnDemandBuiltInUASheet prepends, and B2G/Fennec's/GeckoView's
-    // content.css must come after those UASheet() etc.
-    //
-    // FIXME(emilio, bug 1468133): We may already have loaded some of the other
-    // on-demand built-in UA sheets, including svg.css, so this looks somewhat
-    // bogus... Also, this should probably just use the stylesheet service which
-    // also has the right sheets cached and parsed here...
-    nsCOMPtr<nsICategoryManager> catMan =
-      do_GetService(NS_CATEGORYMANAGER_CONTRACTID);
-    if (catMan) {
-      nsCOMPtr<nsISimpleEnumerator> sheets;
-      catMan->EnumerateCategory("agent-style-sheets", getter_AddRefs(sheets));
-      if (sheets) {
-        bool hasMore;
-        while (NS_SUCCEEDED(sheets->HasMoreElements(&hasMore)) && hasMore) {
-          nsCOMPtr<nsISupports> sheet;
-          if (NS_FAILED(sheets->GetNext(getter_AddRefs(sheet))))
-            break;
-
-          nsCOMPtr<nsISupportsCString> icStr = do_QueryInterface(sheet);
-          MOZ_ASSERT(icStr,
-                     "category manager entries must be nsISupportsCStrings");
-
-          nsAutoCString name;
-          icStr->GetData(name);
-
-          nsCString spec;
-          catMan->GetCategoryEntry("agent-style-sheets", name.get(),
-                                   getter_Copies(spec));
-
-          mozilla::css::Loader* cssLoader = CSSLoader();
-          if (cssLoader->GetEnabled()) {
-            nsCOMPtr<nsIURI> uri;
-            NS_NewURI(getter_AddRefs(uri), spec);
-            if (uri) {
-              RefPtr<StyleSheet> sheet;
-              cssLoader->LoadSheetSync(uri,
-                                       mozilla::css::eAgentSheetFeatures,
-                                       true, &sheet);
-              if (sheet) {
-                EnsureOnDemandBuiltInUASheet(sheet);
-              }
-            }
-          }
-        }
-      }
-    }
-  }
-
-  auto cache = nsLayoutStylesheetCache::Singleton();
-
-  EnsureOnDemandBuiltInUASheet(cache->FormsSheet());
-  EnsureOnDemandBuiltInUASheet(cache->CounterStylesSheet());
-  EnsureOnDemandBuiltInUASheet(cache->HTMLSheet());
-  if (nsLayoutUtils::ShouldUseNoFramesSheet(this)) {
-    EnsureOnDemandBuiltInUASheet(cache->NoFramesSheet());
-  }
-  if (nsLayoutUtils::ShouldUseNoScriptSheet(this)) {
-    EnsureOnDemandBuiltInUASheet(cache->NoScriptSheet());
-  }
-  EnsureOnDemandBuiltInUASheet(cache->UASheet());
-}
-
 } // namespace dom
 } // namespace mozilla
 
 ////////////////////////////////////////////////////////////////////////
 // Exported creation functions
 
 nsresult
 NS_NewSVGDocument(nsIDocument** aInstancePtrResult)
--- a/dom/svg/SVGDocument.h
+++ b/dom/svg/SVGDocument.h
@@ -17,46 +17,37 @@ namespace mozilla {
 class SVGContextPaint;
 
 namespace dom {
 
 class SVGForeignObjectElement;
 
 class SVGDocument final : public XMLDocument
 {
-  friend class SVGForeignObjectElement; // To call EnsureNonSVGUserAgentStyleSheetsLoaded
-  friend class nsIDocument; // Same reason.
-
 public:
   SVGDocument()
     : XMLDocument("image/svg+xml")
-    , mHasLoadedNonSVGUserAgentStyleSheets(false)
   {
     mType = eSVG;
   }
 
-  virtual nsresult InsertChildBefore(nsIContent* aKid, nsIContent* aBeforeThis,
-                                     bool aNotify) override;
   virtual nsresult Clone(mozilla::dom::NodeInfo *aNodeInfo, nsINode **aResult,
                          bool aPreallocateChildren) const override;
 
   void SetCurrentContextPaint(const SVGContextPaint* aContextPaint)
   {
     mCurrentContextPaint = aContextPaint;
   }
 
   const SVGContextPaint* GetCurrentContextPaint() const
   {
     return mCurrentContextPaint;
   }
 
 private:
-  void EnsureNonSVGUserAgentStyleSheetsLoaded();
-
-  bool mHasLoadedNonSVGUserAgentStyleSheets;
 
   // This is maintained by AutoSetRestoreSVGContextPaint.
   const SVGContextPaint* mCurrentContextPaint = nullptr;
 };
 
 } // namespace dom
 } // namespace mozilla
 
--- a/dom/svg/SVGForeignObjectElement.cpp
+++ b/dom/svg/SVGForeignObjectElement.cpp
@@ -102,42 +102,16 @@ SVGForeignObjectElement::HasValidDimensi
          mLengthAttributes[ATTR_WIDTH].GetAnimValInSpecifiedUnits() > 0 &&
          mLengthAttributes[ATTR_HEIGHT].IsExplicitlySet() &&
          mLengthAttributes[ATTR_HEIGHT].GetAnimValInSpecifiedUnits() > 0;
 }
 
 //----------------------------------------------------------------------
 // nsIContent methods
 
-nsresult
-SVGForeignObjectElement::BindToTree(nsIDocument* aDocument,
-                                    nsIContent* aParent,
-                                    nsIContent* aBindingParent,
-                                    bool aCompileEventHandlers)
-{
-  nsresult rv = SVGGraphicsElement::BindToTree(aDocument, aParent,
-                                               aBindingParent,
-                                               aCompileEventHandlers);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  nsIDocument* doc = GetComposedDoc();
-  if (doc && doc->IsSVGDocument()) {
-    // We assume that we're going to have HTML content, so we ensure that the
-    // UA style sheets that nsDocumentViewer::CreateStyleSet skipped when
-    // it saw the document was an SVG document are loaded.
-    //
-    // We setup these style sheets during binding, not element construction,
-    // because elements can be moved from the document that creates them to
-    // another document.
-    doc->AsSVGDocument()->EnsureNonSVGUserAgentStyleSheetsLoaded();
-  }
-
-  return rv;
-}
-
 NS_IMETHODIMP_(bool)
 SVGForeignObjectElement::IsAttributeMapped(const nsAtom* name) const
 {
   static const MappedAttributeEntry* const map[] = {
     sFEFloodMap,
     sFiltersMap,
     sFontSpecificationMap,
     sGradientStopMap,
--- a/dom/svg/SVGForeignObjectElement.h
+++ b/dom/svg/SVGForeignObjectElement.h
@@ -31,19 +31,16 @@ protected:
 public:
   // nsSVGElement specializations:
   virtual gfxMatrix PrependLocalTransformsTo(
     const gfxMatrix &aMatrix,
     SVGTransformTypes aWhich = eAllTransforms) const override;
   virtual bool HasValidDimensions() const override;
 
   // nsIContent interface
-  virtual nsresult BindToTree(nsIDocument* aDocument, nsIContent* aParent,
-                              nsIContent* aBindingParent,
-                              bool aCompileEventHandlers) override;
   NS_IMETHOD_(bool) IsAttributeMapped(const nsAtom* name) const override;
 
   virtual nsresult Clone(mozilla::dom::NodeInfo *aNodeInfo, nsINode **aResult,
                          bool aPreallocateChildren) const override;
 
   // WebIDL
   already_AddRefed<SVGAnimatedLength> X();
   already_AddRefed<SVGAnimatedLength> Y();
--- a/dom/svg/SVGSVGElement.cpp
+++ b/dom/svg/SVGSVGElement.cpp
@@ -490,21 +490,20 @@ SVGSVGElement::BindToTree(nsIDocument* a
     }
   }
 
   nsresult rv = SVGGraphicsElement::BindToTree(aDocument, aParent,
                                               aBindingParent,
                                               aCompileEventHandlers);
   NS_ENSURE_SUCCESS(rv,rv);
 
-  nsIDocument* doc = GetComposedDoc();
-  if (doc) {
-    // Setup the style sheet during binding, not element construction,
-    // because we could move the root SVG element from the document
-    // that created it to another document.
+  if (nsIDocument* doc = GetComposedDoc()) {
+    // Setup the style sheet during binding, not element construction, because
+    // we could move the root SVG element from the document that created it to
+    // another document.
     auto cache = nsLayoutStylesheetCache::Singleton();
     doc->EnsureOnDemandBuiltInUASheet(cache->SVGSheet());
   }
 
   if (mTimedDocumentRoot && smilController) {
     rv = mTimedDocumentRoot->SetParent(smilController);
     if (mStartAnimationOnBindToTree) {
       mTimedDocumentRoot->Begin();
--- a/layout/base/nsDocumentViewer.cpp
+++ b/layout/base/nsDocumentViewer.cpp
@@ -2435,29 +2435,16 @@ nsDocumentViewer::CreateStyleSet(nsIDocu
   // Make sure this does the same thing as PresShell::AddSheet wrt ordering.
 
   // this should eventually get expanded to allow for creating
   // different sets for different media
 
   UniquePtr<ServoStyleSet> styleSet = MakeUnique<ServoStyleSet>();
 
   // The document will fill in the document sheets when we create the presshell
-
-  if (aDocument->IsBeingUsedAsImage()) {
-    MOZ_ASSERT(aDocument->IsSVGDocument(),
-               "Do we want to skip most sheets for this new image type?");
-
-    // SVG-as-an-image must be kept as light and small as possible. We
-    // deliberately skip loading everything and leave svg.css (and html.css and
-    // xul.css) to be loaded on-demand.
-    // XXXjwatt Nothing else is loaded on-demand, but I don't think that
-    // should matter for SVG-as-an-image. If it does, I want to know why!
-    return styleSet;
-  }
-
   auto cache = nsLayoutStylesheetCache::Singleton();
 
   // Handle the user sheets.
   StyleSheet* sheet = nullptr;
   if (nsContentUtils::IsInChromeDocshell(aDocument)) {
     sheet = cache->UserChromeSheet();
   } else {
     sheet = cache->UserContentSheet();
@@ -2468,91 +2455,73 @@ nsDocumentViewer::CreateStyleSet(nsIDocu
   }
 
   // Append chrome sheets (scrollbars + forms).
   sheet = cache->ScrollbarsSheet();
   if (sheet) {
     styleSet->PrependStyleSheet(SheetType::Agent, sheet);
   }
 
-  if (!aDocument->IsSVGDocument()) {
-    // !!! IMPORTANT - KEEP THIS BLOCK IN SYNC WITH
-    // !!! SVGDocument::EnsureNonSVGUserAgentStyleSheetsLoaded.
-
-    // SVGForeignObjectElement::BindToTree calls SVGDocument::
-    // EnsureNonSVGUserAgentStyleSheetsLoaded to loads these UA sheet
-    // on-demand. (Excluding the quirks sheet, which should never be loaded for
-    // an SVG document, and excluding xul.css which will be loaded on demand by
-    // nsXULElement::BindToTree.)
-
-    sheet = cache->FormsSheet();
+  sheet = cache->FormsSheet();
+  if (sheet) {
+    styleSet->PrependStyleSheet(SheetType::Agent, sheet);
+  }
+
+  if (aDocument->LoadsFullXULStyleSheetUpFront()) {
+    // This is the only place components.css gets loaded, unlike xul.css
+    sheet = cache->XULComponentsSheet();
+    if (sheet) {
+      styleSet->PrependStyleSheet(SheetType::Agent, sheet);
+    }
+
+    // nsXULElement::BindToTree loads xul.css on-demand if we don't load it
+    // up-front here.
+    sheet = cache->XULSheet();
     if (sheet) {
       styleSet->PrependStyleSheet(SheetType::Agent, sheet);
     }
-
-    if (aDocument->LoadsFullXULStyleSheetUpFront()) {
-      // This is the only place components.css gets loaded, unlike xul.css
-      sheet = cache->XULComponentsSheet();
-      if (sheet) {
-        styleSet->PrependStyleSheet(SheetType::Agent, sheet);
-      }
-
-      // nsXULElement::BindToTree loads xul.css on-demand if we don't load it
-      // up-front here.
-      sheet = cache->XULSheet();
-      if (sheet) {
-        styleSet->PrependStyleSheet(SheetType::Agent, sheet);
-      }
-    }
-
-    sheet = cache->MinimalXULSheet();
-    if (sheet) {
-      // Load the minimal XUL rules for scrollbars and a few other XUL things
-      // that non-XUL (typically HTML) documents commonly use.
-      styleSet->PrependStyleSheet(SheetType::Agent, sheet);
-    }
-
-    sheet = cache->CounterStylesSheet();
+  }
+
+  sheet = cache->MinimalXULSheet();
+  if (sheet) {
+    // Load the minimal XUL rules for scrollbars and a few other XUL things
+    // that non-XUL (typically HTML) documents commonly use.
+    styleSet->PrependStyleSheet(SheetType::Agent, sheet);
+  }
+
+  sheet = cache->CounterStylesSheet();
+  if (sheet) {
+    styleSet->PrependStyleSheet(SheetType::Agent, sheet);
+  }
+
+  if (nsLayoutUtils::ShouldUseNoScriptSheet(aDocument)) {
+    sheet = cache->NoScriptSheet();
     if (sheet) {
       styleSet->PrependStyleSheet(SheetType::Agent, sheet);
     }
-
-    if (nsLayoutUtils::ShouldUseNoScriptSheet(aDocument)) {
-      sheet = cache->NoScriptSheet();
-      if (sheet) {
-        styleSet->PrependStyleSheet(SheetType::Agent, sheet);
-      }
-    }
-
-    if (nsLayoutUtils::ShouldUseNoFramesSheet(aDocument)) {
-      sheet = cache->NoFramesSheet();
-      if (sheet) {
-        styleSet->PrependStyleSheet(SheetType::Agent, sheet);
-      }
-    }
-
-    // We don't add quirk.css here; nsPresContext::CompatibilityModeChanged will
-    // append it if needed.
-
-    sheet = cache->HTMLSheet();
+  }
+
+  if (nsLayoutUtils::ShouldUseNoFramesSheet(aDocument)) {
+    sheet = cache->NoFramesSheet();
     if (sheet) {
       styleSet->PrependStyleSheet(SheetType::Agent, sheet);
     }
-
-    styleSet->PrependStyleSheet(SheetType::Agent, cache->UASheet());
-  } else {
-    // SVG documents may have scrollbars and need the scrollbar styling.
-    sheet = cache->MinimalXULSheet();
-    if (sheet) {
-      styleSet->PrependStyleSheet(SheetType::Agent, sheet);
-    }
-  }
-
-  nsStyleSheetService* sheetService = nsStyleSheetService::GetInstance();
-  if (sheetService) {
+  }
+
+  // We don't add quirk.css here; nsPresContext::CompatibilityModeChanged will
+  // append it if needed.
+
+  sheet = cache->HTMLSheet();
+  if (sheet) {
+    styleSet->PrependStyleSheet(SheetType::Agent, sheet);
+  }
+
+  styleSet->PrependStyleSheet(SheetType::Agent, cache->UASheet());
+
+  if (nsStyleSheetService* sheetService = nsStyleSheetService::GetInstance()) {
     for (StyleSheet* sheet : *sheetService->AgentStyleSheets()) {
       styleSet->AppendStyleSheet(SheetType::Agent, sheet);
     }
     for (StyleSheet* sheet : Reversed(*sheetService->UserStyleSheets())) {
       styleSet->PrependStyleSheet(SheetType::User, sheet);
     }
   }
 
--- a/layout/printing/nsPrintJob.cpp
+++ b/layout/printing/nsPrintJob.cpp
@@ -2339,23 +2339,16 @@ nsPrintJob::ReflowPrintObject(const Uniq
   aPO->mViewManager = new nsViewManager();
 
   rv = aPO->mViewManager->Init(printData->mPrintDC);
   NS_ENSURE_SUCCESS(rv,rv);
 
   UniquePtr<ServoStyleSet> styleSet =
     mDocViewerPrint->CreateStyleSet(aPO->mDocument);
 
-  if (aPO->mDocument->IsSVGDocument()) {
-    // The SVG document only loads minimal-xul.css, so it doesn't apply other
-    // styles. We should add ua.css for applying style which related to print.
-    auto cache = nsLayoutStylesheetCache::Singleton();
-    styleSet->PrependStyleSheet(SheetType::Agent, cache->UASheet());
-  }
-
   aPO->mPresShell = aPO->mDocument->CreateShell(aPO->mPresContext,
                                                 aPO->mViewManager,
                                                 std::move(styleSet));
   if (!aPO->mPresShell) {
     return NS_ERROR_FAILURE;
   }
 
   // If we're printing selection then remove the unselected nodes from our
--- a/layout/svg/svg.css
+++ b/layout/svg/svg.css
@@ -79,32 +79,8 @@ foreignObject {
   mask: inherit;
   opacity: inherit;
 }
 
 *:-moz-focusring {
   /* Don't specify the outline-color, we should always use initial value. */
   outline: 1px dotted;
 }
-
-/* nsDocumentViewer::CreateStyleSet doesn't load ua.css.
- * A few styles are common to html and SVG though
- * so we copy the rules below from that file.
- */
-
-/* Links */
-
-*|*:any-link {
-  cursor: pointer;
-}
-
-*|*:any-link:-moz-focusring {
-  /* Don't specify the outline-color, we should always use initial value. */
-  outline: 1px dotted;
-}
-
-/*
- * SVG-as-an-image needs this rule
- */
-*|*::-moz-viewport, *|*::-moz-viewport-scroll, *|*::-moz-canvas, *|*::-moz-scrolled-canvas {
-  display: block !important;
-  background-color: inherit;
-}