Bug 1543762 - Flush less in cross-document getComputedStyle situations. r=heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 12 Apr 2019 10:04:33 +0000
changeset 469252 0fd4c1a6ade556a67ed86d89235d3f1e0e6ce036
parent 469251 11fb8048959d0d2a1e12027680684ff9fbebf760
child 469253 4e987641febce56ca92c086093965f2192531387
push id112776
push usershindli@mozilla.com
push dateFri, 12 Apr 2019 16:20:17 +0000
treeherdermozilla-inbound@b4501ced5619 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1543762, 1537903
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 1543762 - Flush less in cross-document getComputedStyle situations. r=heycam I wrote this while looking at bug 1537903, but actually it doesn't help there, since all the extra time is spent actually computing styles. I think this is still worth landing it though. The reasoning for not caring of this case is that we mint an style anyway out of the blue anyway. There's no point in restyling the whole document. Differential Revision: https://phabricator.services.mozilla.com/D27124
layout/style/nsComputedDOMStyle.cpp
layout/style/nsComputedDOMStyle.h
--- a/layout/style/nsComputedDOMStyle.cpp
+++ b/layout/style/nsComputedDOMStyle.cpp
@@ -94,33 +94,43 @@ already_AddRefed<CSSValue> GetBackground
         nsCSSProps::ValueToKeywordEnum(aLayers.mLayers[i].*aMember, aTable));
     valueList->AppendCSSValue(val.forget());
   }
 
   return valueList.forget();
 }
 
 // Whether aDocument needs to restyle for aElement
-static bool DocumentNeedsRestyle(const Document* aDocument, Element* aElement,
-                                 nsAtom* aPseudo) {
-  PresShell* presShell = aDocument->GetPresShell();
+static bool ElementNeedsRestyle(Element* aElement, nsAtom* aPseudo) {
+  const Document* doc = aElement->GetComposedDoc();
+  if (!doc) {
+    // If the element is out of the document we don't return styles for it, so
+    // nothing to do.
+    return false;
+  }
+
+  PresShell* presShell = doc->GetPresShell();
   if (!presShell) {
-    return true;
+    // If there's no pres-shell we'll just either mint a new style from our
+    // caller document, or return no styles, so nothing to do (unless our owner
+    // element needs to get restyled, which could cause us to gain a pres shell,
+    // but the caller checks that).
+    return false;
   }
 
-  nsPresContext* presContext = presShell->GetPresContext();
-  MOZ_ASSERT(presContext);
-
   // Unfortunately we don't know if the sheet change affects mElement or not, so
   // just assume it will and that we need to flush normally.
   ServoStyleSet* styleSet = presShell->StyleSet();
   if (styleSet->StyleSheetsHaveChanged()) {
     return true;
   }
 
+  nsPresContext* presContext = presShell->GetPresContext();
+  MOZ_ASSERT(presContext);
+
   // Pending media query updates can definitely change style on the element. For
   // example, if you change the zoom factor and then call getComputedStyle, you
   // should be able to observe the style with the new media queries.
   //
   // TODO(emilio): Does this need to also check the user font set? (it affects
   // ch / ex units).
   if (presContext->HasPendingMediaQueryUpdates()) {
     // So gotta flush.
@@ -146,17 +156,17 @@ static bool DocumentNeedsRestyle(const D
 
   // For Servo, we need to process the restyle-hint-invalidations first, to
   // expand LaterSiblings hint, so that we can look whether ancestors need
   // restyling.
   RestyleManager* restyleManager = presContext->RestyleManager();
   restyleManager->ProcessAllPendingAttributeAndStateInvalidations();
 
   if (!presContext->EffectCompositor()->HasPendingStyleUpdates() &&
-      !aDocument->GetServoRestyleRoot()) {
+      !doc->GetServoRestyleRoot()) {
     return false;
   }
 
   // Then if there is a restyle root, we check if the root is an ancestor of
   // this content. If it is not, then we don't need to restyle immediately.
   // Note this is different from Gecko: we only check if any ancestor needs
   // to restyle _itself_, not descendants, since dirty descendants can be
   // another subtree.
@@ -760,39 +770,31 @@ void nsComputedDOMStyle::SetResolvedComp
 void nsComputedDOMStyle::SetFrameComputedStyle(mozilla::ComputedStyle* aStyle,
                                                uint64_t aGeneration) {
   ClearComputedStyle();
   mComputedStyle = aStyle;
   mComputedStyleGeneration = aGeneration;
   mPresShellId = mPresShell->GetPresShellId();
 }
 
-bool nsComputedDOMStyle::NeedsToFlush(Document* aDocument) const {
-  // If mElement is not in the same document, we could do some checks to know if
-  // there are some pending restyles can be ignored across documents (since we
-  // will use the caller document's style), but it can be complicated and should
-  // be an edge case, so we just don't bother to do the optimization in this
-  // case.
-  //
-  // FIXME(emilio): This is likely to want GetComposedDoc() instead of
-  // OwnerDoc().
-  if (aDocument != mElement->OwnerDoc()) {
+bool nsComputedDOMStyle::NeedsToFlush() const {
+  // We always compute styles from the element's owner document.
+  if (ElementNeedsRestyle(mElement, mPseudo)) {
     return true;
   }
-  if (DocumentNeedsRestyle(aDocument, mElement, mPseudo)) {
-    return true;
-  }
+
+  Document* doc = mElement->OwnerDoc();
   // If parent document is there, also needs to check if there is some change
   // that needs to flush this document (e.g. size change for iframe).
-  while (Document* parentDocument = aDocument->GetParentDocument()) {
-    Element* element = parentDocument->FindContentForSubDocument(aDocument);
-    if (DocumentNeedsRestyle(parentDocument, element, nullptr)) {
+  while (Document* parentDocument = doc->GetParentDocument()) {
+    Element* element = parentDocument->FindContentForSubDocument(doc);
+    if (ElementNeedsRestyle(element, nullptr)) {
       return true;
     }
-    aDocument = parentDocument;
+    doc = parentDocument;
   }
 
   return false;
 }
 
 void nsComputedDOMStyle::UpdateCurrentStyleSources(bool aNeedsLayoutFlush) {
   nsCOMPtr<Document> document = do_QueryReferent(mDocumentWeak);
   if (!document) {
@@ -801,17 +803,17 @@ void nsComputedDOMStyle::UpdateCurrentSt
   }
 
   // TODO(emilio): We may want to handle a few special-cases here:
   //
   //  * https://github.com/w3c/csswg-drafts/issues/1964
   //  * https://github.com/w3c/csswg-drafts/issues/1548
 
   // If the property we are computing relies on layout, then we must flush.
-  const bool needsToFlush = aNeedsLayoutFlush || NeedsToFlush(document);
+  const bool needsToFlush = aNeedsLayoutFlush || NeedsToFlush();
   if (needsToFlush) {
     // Flush _before_ getting the presshell, since that could create a new
     // presshell.  Also note that we want to flush the style on the document
     // we're computing style in, not on the document mElement is in -- the two
     // may be different.
     document->FlushPendingNotifications(aNeedsLayoutFlush ? FlushType::Layout
                                                           : FlushType::Style);
   }
--- a/layout/style/nsComputedDOMStyle.h
+++ b/layout/style/nsComputedDOMStyle.h
@@ -278,17 +278,16 @@ class nsComputedDOMStyle final : public 
   /* Display properties */
   already_AddRefed<CSSValue> DoGetTransform();
   already_AddRefed<CSSValue> DoGetTransformOrigin();
   already_AddRefed<CSSValue> DoGetPerspectiveOrigin();
 
   /* Column properties */
   already_AddRefed<CSSValue> DoGetColumnRuleWidth();
 
-
   // For working around a MSVC bug. See related comment in
   // GenerateComputedDOMStyleGenerated.py.
   already_AddRefed<CSSValue> DummyGetter();
 
   /* Helper functions */
   void SetValueFromComplexColor(nsROCSSPrimitiveValue* aValue,
                                 const mozilla::StyleColor& aColor);
   void SetValueToPosition(const mozilla::Position& aPosition,
@@ -385,19 +384,19 @@ class nsComputedDOMStyle final : public 
   already_AddRefed<CSSValue> CreatePrimitiveValueForBasicShape(
       const mozilla::UniquePtr<mozilla::StyleBasicShape>& aStyleBasicShape);
   void BoxValuesToString(nsAString& aString,
                          const nsTArray<nsStyleCoord>& aBoxValues,
                          bool aClampNegativeCalc);
   void BasicShapeRadiiToString(nsAString& aCssText,
                                const mozilla::BorderRadius&);
 
-  // Find out if we can safely skip flushing for aDocument (i.e. pending
-  // restyles does not affect mContent).
-  bool NeedsToFlush(Document*) const;
+  // Find out if we can safely skip flushing (i.e. pending restyles do not
+  // affect mElement).
+  bool NeedsToFlush() const;
 
   static ComputedStyleMap* GetComputedStyleMap();
 
   // We don't really have a good immutable representation of "presentation".
   // Given the way GetComputedStyle is currently used, we should just grab the
   // presshell, if any, from the document.
   nsWeakPtr mDocumentWeak;
   RefPtr<Element> mElement;