Bug 1540220 - Cleanup a bit the lazy style resolution APIs. r=heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 09 Apr 2019 18:05:04 +0000
changeset 468610 07a87bbee9ca37c2852b620e52b62679719e64f4
parent 468609 ae9783be99324502017f3c9bfc92cb811fd1993e
child 468611 e1fa16165c528ccb201e8f9e2a24ce6833aeeded
push id35843
push usernbeleuzu@mozilla.com
push dateTue, 09 Apr 2019 22:08:13 +0000
treeherdermozilla-central@a31032a16330 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1540220
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 1540220 - Cleanup a bit the lazy style resolution APIs. r=heycam There are some that only have one caller, and some slightly confusing naming. Hopefully make it a bit clearer. Differential Revision: https://phabricator.services.mozilla.com/D25457
layout/base/nsPresContext.cpp
layout/generic/nsImageFrame.cpp
layout/generic/nsImageMap.cpp
layout/generic/nsImageMap.h
layout/style/ServoStyleSet.cpp
layout/style/ServoStyleSet.h
layout/style/nsComputedDOMStyle.cpp
--- a/layout/base/nsPresContext.cpp
+++ b/layout/base/nsPresContext.cpp
@@ -1034,30 +1034,30 @@ static bool CheckOverflow(const nsStyleD
     *aStyles =
         ScrollStyles(StyleOverflow::Hidden, StyleOverflow::Hidden, aDisplay);
   } else {
     *aStyles = ScrollStyles(aDisplay);
   }
   return true;
 }
 
+// https://drafts.csswg.org/css-overflow/#overflow-propagation
 static Element* GetPropagatedScrollStylesForViewport(
     nsPresContext* aPresContext, ScrollStyles* aStyles) {
   Document* document = aPresContext->Document();
   Element* docElement = document->GetRootElement();
 
   // docElement might be null if we're doing this after removing it.
   if (!docElement) {
     return nullptr;
   }
 
   // Check the style on the document root element
   ServoStyleSet* styleSet = aPresContext->StyleSet();
-  RefPtr<ComputedStyle> rootStyle =
-      styleSet->ResolveStyleFor(docElement, LazyComputeBehavior::Allow);
+  RefPtr<ComputedStyle> rootStyle = styleSet->ResolveStyleLazily(*docElement);
   if (CheckOverflow(rootStyle->StyleDisplay(), aStyles)) {
     // tell caller we stole the overflow style from the root element
     return docElement;
   }
 
   // Don't look in the BODY for non-HTML documents or HTML documents
   // with non-HTML roots.
   // XXX this should be earlier; we shouldn't even look at the document root
@@ -1071,18 +1071,22 @@ static Element* GetPropagatedScrollStyle
   if (!bodyElement) {
     // No body, nothing to do here.
     return nullptr;
   }
 
   MOZ_ASSERT(bodyElement->IsHTMLElement(nsGkAtoms::body),
              "GetBodyElement returned something bogus");
 
-  RefPtr<ComputedStyle> bodyStyle =
-      styleSet->ResolveStyleFor(bodyElement, LazyComputeBehavior::Allow);
+  // FIXME(emilio): We could make these just a ResolveServoStyle call if we
+  // looked at `display` on the root, and updated styles properly before doing
+  // this on first construction:
+  //
+  // https://github.com/w3c/csswg-drafts/issues/3779
+  RefPtr<ComputedStyle> bodyStyle = styleSet->ResolveStyleLazily(*bodyElement);
 
   if (CheckOverflow(bodyStyle->StyleDisplay(), aStyles)) {
     // tell caller we stole the overflow style from the body element
     return bodyElement;
   }
 
   return nullptr;
 }
--- a/layout/generic/nsImageFrame.cpp
+++ b/layout/generic/nsImageFrame.cpp
@@ -15,16 +15,17 @@
 #include "mozilla/ComputedStyle.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/Encoding.h"
 #include "mozilla/EventStates.h"
 #include "mozilla/gfx/2D.h"
 #include "mozilla/gfx/Helpers.h"
 #include "mozilla/gfx/PathHelpers.h"
 #include "mozilla/dom/GeneratedImageContent.h"
+#include "mozilla/dom/HTMLAreaElement.h"
 #include "mozilla/dom/HTMLImageElement.h"
 #include "mozilla/dom/ResponsiveImageSelector.h"
 #include "mozilla/layers/RenderRootStateManager.h"
 #include "mozilla/layers/WebRenderLayerManager.h"
 #include "mozilla/MouseEvents.h"
 #include "mozilla/PresShell.h"
 #include "mozilla/Unused.h"
 
@@ -2293,24 +2294,24 @@ nsresult nsImageFrame::HandleEvent(nsPre
 
 Maybe<nsIFrame::Cursor> nsImageFrame::GetCursor(const nsPoint& aPoint) {
   nsImageMap* map = GetImageMap();
   if (!map) {
     return nsFrame::GetCursor(aPoint);
   }
   nsIntPoint p;
   TranslateEventCoords(aPoint, p);
-  nsCOMPtr<nsIContent> area = map->GetArea(p.x, p.y);
+  HTMLAreaElement* area = map->GetArea(p.x, p.y);
   if (!area) {
     return nsFrame::GetCursor(aPoint);
   }
 
   // Use the cursor from the style of the *area* element.
-  RefPtr<ComputedStyle> areaStyle = PresShell()->StyleSet()->ResolveStyleFor(
-      area->AsElement(), LazyComputeBehavior::Allow);
+  RefPtr<ComputedStyle> areaStyle =
+    PresShell()->StyleSet()->ResolveStyleLazily(*area);
   StyleCursorKind kind = areaStyle->StyleUI()->mCursor;
   if (kind == StyleCursorKind::Auto) {
     kind = StyleCursorKind::Default;
   }
   return Some(Cursor{kind, AllowCustomCursorImage::Yes, std::move(areaStyle)});
 }
 
 nsresult nsImageFrame::AttributeChanged(int32_t aNameSpaceID,
--- a/layout/generic/nsImageMap.cpp
+++ b/layout/generic/nsImageMap.cpp
@@ -733,28 +733,28 @@ void nsImageMap::AddArea(HTMLAreaElement
   aArea->SetPrimaryFrame(mImageFrame);
 
   nsAutoString coords;
   aArea->GetAttr(kNameSpaceID_None, nsGkAtoms::coords, coords);
   area->ParseCoords(coords);
   mAreas.AppendElement(std::move(area));
 }
 
-nsIContent* nsImageMap::GetArea(nscoord aX, nscoord aY) const {
+HTMLAreaElement* nsImageMap::GetArea(nscoord aX, nscoord aY) const {
   NS_ASSERTION(mMap, "Not initialized");
   for (const auto& area : mAreas) {
     if (area->IsInside(aX, aY)) {
       return area->mArea;
     }
   }
 
   return nullptr;
 }
 
-nsIContent* nsImageMap::GetAreaAt(uint32_t aIndex) const {
+HTMLAreaElement* nsImageMap::GetAreaAt(uint32_t aIndex) const {
   return mAreas.ElementAt(aIndex)->mArea;
 }
 
 void nsImageMap::Draw(nsIFrame* aFrame, DrawTarget& aDrawTarget,
                       const ColorPattern& aColor,
                       const StrokeOptions& aStrokeOptions) {
   for (auto& area : mAreas) {
     area->Draw(aFrame, aDrawTarget, aColor, aStrokeOptions);
--- a/layout/generic/nsImageMap.h
+++ b/layout/generic/nsImageMap.h
@@ -38,27 +38,27 @@ class nsImageMap final : public nsStubMu
   nsImageMap();
 
   void Init(nsImageFrame* aImageFrame, nsIContent* aMap);
 
   /**
    * Return the first area element (in content order) for the given aX,aY pixel
    * coordinate or nullptr if the coordinate is outside all areas.
    */
-  nsIContent* GetArea(nscoord aX, nscoord aY) const;
+  mozilla::dom::HTMLAreaElement* GetArea(nscoord aX, nscoord aY) const;
 
   /**
    * Return area elements count associated with the image map.
    */
   uint32_t AreaCount() const { return mAreas.Length(); }
 
   /**
    * Return area element at the given index.
    */
-  nsIContent* GetAreaAt(uint32_t aIndex) const;
+  mozilla::dom::HTMLAreaElement* GetAreaAt(uint32_t aIndex) const;
 
   void Draw(nsIFrame* aFrame, DrawTarget& aDrawTarget,
             const ColorPattern& aColor,
             const StrokeOptions& aStrokeOptions = StrokeOptions());
 
   /**
    * Called just before the nsImageFrame releases us.
    * Used to break the cycle caused by the DOM listener.
--- a/layout/style/ServoStyleSet.cpp
+++ b/layout/style/ServoStyleSet.cpp
@@ -307,25 +307,16 @@ void ServoStyleSet::SetAuthorStyleDisabl
   // XXX Workaround for the assertion in InvalidateStyleForDocumentStateChanges
   // which is called by nsIPresShell::SetAuthorStyleDisabled via nsIPresShell::
   // RestyleForCSSRuleChanges. It is not really necessary because we don't need
   // to rebuild stylist for this change. But we have bug around this, and we
   // may want to rethink how things should work. See bug 1437785.
   SetStylistStyleSheetsDirty();
 }
 
-already_AddRefed<ComputedStyle> ServoStyleSet::ResolveStyleFor(
-    Element* aElement, LazyComputeBehavior aMayCompute) {
-  if (aMayCompute == LazyComputeBehavior::Allow) {
-    return ResolveStyleLazily(aElement, PseudoStyleType::NotPseudo);
-  }
-
-  return ResolveServoStyle(*aElement);
-}
-
 const ServoElementSnapshotTable& ServoStyleSet::Snapshots() {
   MOZ_ASSERT(GetPresContext(), "Styling a document without a shell?");
   return GetPresContext()->RestyleManager()->Snapshots();
 }
 
 void ServoStyleSet::ResolveMappedAttrDeclarationBlocks() {
   if (nsHTMLStyleSheet* sheet = mDocument->GetAttributeStyleSheet()) {
     sheet->CalculateMappedServoDeclarations();
@@ -493,23 +484,16 @@ already_AddRefed<ComputedStyle> ServoSty
       }
     }
   }
 
   MOZ_ASSERT(computedValues);
   return computedValues.forget();
 }
 
-already_AddRefed<ComputedStyle> ServoStyleSet::ResolveStyleLazily(
-    Element* aElement, PseudoStyleType aPseudoType,
-    StyleRuleInclusion aRuleInclusion) {
-  PreTraverseSync();
-  return ResolveStyleLazilyInternal(aElement, aPseudoType, aRuleInclusion);
-}
-
 already_AddRefed<ComputedStyle>
 ServoStyleSet::ResolveInheritingAnonymousBoxStyle(PseudoStyleType aType,
                                                   ComputedStyle* aParentStyle) {
   MOZ_ASSERT(PseudoStyle::IsInheritingAnonBox(aType));
   MOZ_ASSERT_IF(aParentStyle, !StylistNeedsUpdate());
 
   UpdateStylistIfNeeded();
 
@@ -1140,63 +1124,64 @@ void ServoStyleSet::CompatibilityModeCha
 }
 
 void ServoStyleSet::ClearNonInheritingComputedStyles() {
   for (RefPtr<ComputedStyle>& ptr : mNonInheritingComputedStyles) {
     ptr = nullptr;
   }
 }
 
-already_AddRefed<ComputedStyle> ServoStyleSet::ResolveStyleLazilyInternal(
-    Element* aElement, PseudoStyleType aPseudoType,
+already_AddRefed<ComputedStyle> ServoStyleSet::ResolveStyleLazily(
+    Element& aElement, PseudoStyleType aPseudoType,
     StyleRuleInclusion aRuleInclusion) {
+  PreTraverseSync();
   MOZ_ASSERT(GetPresContext(),
              "For now, no style resolution without a pres context");
-  GetPresContext()->EffectCompositor()->PreTraverse(aElement, aPseudoType);
+  GetPresContext()->EffectCompositor()->PreTraverse(&aElement, aPseudoType);
   MOZ_ASSERT(!StylistNeedsUpdate());
 
   AutoSetInServoTraversal guard(this);
 
   /**
    * NB: This is needed because we process animations and transitions on the
    * pseudo-elements themselves, not on the parent's EagerPseudoStyles.
    *
    * That means that that style doesn't account for animations, and we can't do
    * that easily from the traversal without doing wasted work.
    *
    * As such, we just lie here a bit, which is the entrypoint of
    * getComputedStyle, the only API where this can be observed, to look at the
    * style of the pseudo-element if it exists instead.
    */
-  Element* elementForStyleResolution = aElement;
+  Element* elementForStyleResolution = &aElement;
   PseudoStyleType pseudoTypeForStyleResolution = aPseudoType;
   if (aPseudoType == PseudoStyleType::before) {
-    if (Element* pseudo = nsLayoutUtils::GetBeforePseudo(aElement)) {
+    if (Element* pseudo = nsLayoutUtils::GetBeforePseudo(&aElement)) {
       elementForStyleResolution = pseudo;
       pseudoTypeForStyleResolution = PseudoStyleType::NotPseudo;
     }
   } else if (aPseudoType == PseudoStyleType::after) {
-    if (Element* pseudo = nsLayoutUtils::GetAfterPseudo(aElement)) {
+    if (Element* pseudo = nsLayoutUtils::GetAfterPseudo(&aElement)) {
       elementForStyleResolution = pseudo;
       pseudoTypeForStyleResolution = PseudoStyleType::NotPseudo;
     }
   } else if (aPseudoType == PseudoStyleType::marker) {
-    if (Element* pseudo = nsLayoutUtils::GetMarkerPseudo(aElement)) {
+    if (Element* pseudo = nsLayoutUtils::GetMarkerPseudo(&aElement)) {
       elementForStyleResolution = pseudo;
       pseudoTypeForStyleResolution = PseudoStyleType::NotPseudo;
     }
   }
 
   RefPtr<ComputedStyle> computedValues =
       Servo_ResolveStyleLazily(elementForStyleResolution,
                                pseudoTypeForStyleResolution, aRuleInclusion,
                                &Snapshots(), mRawSet.get())
           .Consume();
 
-  if (GetPresContext()->EffectCompositor()->PreTraverse(aElement,
+  if (GetPresContext()->EffectCompositor()->PreTraverse(&aElement,
                                                         aPseudoType)) {
     computedValues =
         Servo_ResolveStyleLazily(elementForStyleResolution,
                                  pseudoTypeForStyleResolution, aRuleInclusion,
                                  &Snapshots(), mRawSet.get())
             .Consume();
   }
 
--- a/layout/style/ServoStyleSet.h
+++ b/layout/style/ServoStyleSet.h
@@ -125,20 +125,16 @@ class ServoStyleSet {
 
   void AddSizeOfIncludingThis(nsWindowSizes& aSizes) const;
   const RawServoStyleSet* RawSet() const { return mRawSet.get(); }
 
   bool GetAuthorStyleDisabled() const { return mAuthorStyleDisabled; }
 
   void SetAuthorStyleDisabled(bool aStyleDisabled);
 
-  // FIXME(emilio): All the callers pass Allow here
-  already_AddRefed<ComputedStyle> ResolveStyleFor(
-      dom::Element* aElement, LazyComputeBehavior aMayCompute);
-
   // Get a CopmutedStyle for a text node (which no rules will match).
   //
   // The returned ComputedStyle will have nsCSSAnonBoxes::mozText() as its
   // pseudo.
   //
   // (Perhaps mozText should go away and we shouldn't even create style
   // contexts for such content nodes, when text-combine-upright is not
   // present.  However, not doing any rule matching for them is a first step.)
@@ -174,21 +170,24 @@ class ServoStyleSet {
   // type is one that allows user action pseudo-classes after it or allows
   // style attributes; otherwise, it is ignored.
   already_AddRefed<ComputedStyle> ResolvePseudoElementStyle(
       dom::Element* aOriginatingElement, PseudoStyleType aType,
       ComputedStyle* aParentStyle, dom::Element* aPseudoElement);
 
   // Resolves style for a (possibly-pseudo) Element without assuming that the
   // style has been resolved. If the element was unstyled and a new style
-  // context was resolved, it is not stored in the DOM. (That is, the element
-  // remains unstyled.)
+  // was resolved, it is not stored in the DOM. (That is, the element remains
+  // unstyled.)
+  //
+  // TODO(emilio): Element argument should be `const`.
   already_AddRefed<ComputedStyle> ResolveStyleLazily(
-      dom::Element* aElement, PseudoStyleType,
-      StyleRuleInclusion aRules = StyleRuleInclusion::All);
+      dom::Element&,
+      PseudoStyleType = PseudoStyleType::NotPseudo,
+      StyleRuleInclusion = StyleRuleInclusion::All);
 
   // Get a ComputedStyle for an anonymous box. The pseudo type must be an
   // inheriting anon box.
   already_AddRefed<ComputedStyle> ResolveInheritingAnonymousBoxStyle(
       PseudoStyleType, ComputedStyle* aParentStyle);
 
   // Get a ComputedStyle for an anonymous box. The pseudo type must be
   // a non-inheriting anon box.
@@ -483,20 +482,16 @@ class ServoStyleSet {
 
   /**
    * Update the stylist as needed to ensure style data is up-to-date.
    *
    * This should only be called if StylistNeedsUpdate returns true.
    */
   void UpdateStylist();
 
-  already_AddRefed<ComputedStyle> ResolveStyleLazilyInternal(
-      dom::Element* aElement, PseudoStyleType aPseudoType,
-      StyleRuleInclusion aRules = StyleRuleInclusion::All);
-
   void RunPostTraversalTasks();
 
   void PrependSheetOfType(SheetType aType, StyleSheet* aSheet);
 
   void AppendSheetOfType(SheetType aType, StyleSheet* aSheet);
 
   void InsertSheetOfType(SheetType aType, StyleSheet* aSheet,
                          StyleSheet* aBeforeSheet);
--- a/layout/style/nsComputedDOMStyle.cpp
+++ b/layout/style/nsComputedDOMStyle.cpp
@@ -554,17 +554,17 @@ already_AddRefed<ComputedStyle> nsComput
   // No frame has been created, or we have a pseudo, or we're looking
   // for the default style, so resolve the style ourselves.
   ServoStyleSet* styleSet = presShell->StyleSet();
 
   StyleRuleInclusion rules = aStyleType == eDefaultOnly
                                  ? StyleRuleInclusion::DefaultOnly
                                  : StyleRuleInclusion::All;
   RefPtr<ComputedStyle> result =
-      styleSet->ResolveStyleLazily(aElement, pseudoType, rules);
+      styleSet->ResolveStyleLazily(*aElement, pseudoType, rules);
   return result.forget();
 }
 
 already_AddRefed<ComputedStyle>
 nsComputedDOMStyle::GetUnanimatedComputedStyleNoFlush(Element* aElement,
                                                       nsAtom* aPseudo) {
   RefPtr<ComputedStyle> style = GetComputedStyleNoFlush(aElement, aPseudo);
   if (!style) {