Bug 1568536 - Don't propagate viewport styles from display: none or display: contents elements. r=dholbert

Resolution is at:

 * https://github.com/w3c/csswg-drafts/issues/3779#issuecomment-481762912

Tests are at https://chromium-review.googlesource.com/c/chromium/src/+/1547974,
I'll make sure to wait until they're in the tree and ensure they're passing.

Note that we need to ensure in the frame constructor that the document element
is styled _before_ calling UpdateViewportScrollStylesOverride(). This saves
duplicated work (since ResolveStyleLazily throws away the style).

ResolveStyleLazily already returns out of date styles, unless the element is not
styled yet, or is in a `display: none` subtree, in which case it computes and
returns a new (up-to-date) style.

So the switch to the FFI call only should change behavior for the display: none
subtree case (since we ensure the unstyled case isn't hit by styling the
document earlier). But that case is one of the cases we're interested in
changing, conveniently.

Depends on D40080

Differential Revision: https://phabricator.services.mozilla.com/D39204

--HG--
extra : moz-landing-system : lando

layout/base/nsCSSFrameConstructor.cpp

@@ -2229,6 +2229,16 @@ nsIFrame* nsCSSFrameConstructor::ConstructDocElementFrame(
     GetRootFrame()->SetComputedStyleWithoutNotification(sc);
   }
 
+  ServoStyleSet* set = mPresShell->StyleSet();
+
+  // Ensure the document element is styled at this point.
+  if (!aDocElement->HasServoData()) {
+    // NOTE(emilio): If the root has a non-null binding, we'll stop at the
+    // document element and won't process any children, loading the bindings
+    // (or failing to do so) will take care of the rest.
+    set->StyleNewSubtree(aDocElement);
+  }
+
   // Make sure to call UpdateViewportScrollStylesOverride before
   // SetUpDocElementContainingBlock, since it sets up our scrollbar state
   // properly.
@@ -2249,16 +2259,6 @@ nsIFrame* nsCSSFrameConstructor::ConstructDocElementFrame(
   if (!mTempFrameTreeState)
     state.mPresShell->CaptureHistoryState(getter_AddRefs(mTempFrameTreeState));
 
-  // --------- CREATE AREA OR BOX FRAME -------
-  ServoStyleSet* set = mPresShell->StyleSet();
-  // Ensure the document element is styled at this point.
-  if (!aDocElement->HasServoData()) {
-    // NOTE(emilio): If the root has a non-null binding, we'll stop at the
-    // document element and won't process any children, loading the bindings
-    // (or failing to do so) will take care of the rest.
-    set->StyleNewSubtree(aDocElement);
-  }
-
   RefPtr<ComputedStyle> computedStyle =
       ServoStyleSet::ResolveServoStyle(*aDocElement);
 

layout/base/nsPresContext.cpp

@@ -1019,9 +1019,21 @@ gfxSize nsPresContext::ScreenSizeInchesForFontInflation(bool* aChanged) {
   return deviceSizeInches;
 }
 
-static bool CheckOverflow(ComputedStyle* aComputedStyle,
+static bool CheckOverflow(const ComputedStyle* aComputedStyle,
                           ScrollStyles* aStyles) {
+  // If they're not styled yet, we'll get around to it when constructing frames
+  // for the element.
+  if (!aComputedStyle) {
+    return false;
+  }
   const nsStyleDisplay* display = aComputedStyle->StyleDisplay();
+
+  // If they will generate no box, just don't.
+  if (display->mDisplay == StyleDisplay::None ||
+      display->mDisplay == StyleDisplay::Contents) {
+    return false;
+  }
+
   if (display->mOverflowX == StyleOverflow::Visible &&
       display->mOverscrollBehaviorX == StyleOverscrollBehavior::Auto &&
       display->mOverscrollBehaviorY == StyleOverscrollBehavior::Auto &&
@@ -1040,19 +1052,23 @@ static bool CheckOverflow(ComputedStyle* aComputedStyle,
 }
 
 // https://drafts.csswg.org/css-overflow/#overflow-propagation
+//
+// NOTE(emilio): We may need to use out-of-date styles for this, since this is
+// called from nsCSSFrameConstructor::ContentRemoved. We could refactor this a
+// bit to avoid doing that, and also fix correctness issues (we don't invalidate
+// properly when we insert a body element and there is a previous one, for
+// example).
 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->ResolveStyleLazily(*docElement);
+  const auto* rootStyle = Servo_Element_GetMaybeOutOfDateStyle(docElement);
   if (CheckOverflow(rootStyle, aStyles)) {
     // tell caller we stole the overflow style from the root element
     return docElement;
@@ -1069,20 +1085,13 @@ static Element* GetPropagatedScrollStylesForViewport(
 
   Element* bodyElement = document->AsHTMLDocument()->GetBodyElement();
   if (!bodyElement) {
-    // No body, nothing to do here.
     return nullptr;
   }
 
   MOZ_ASSERT(bodyElement->IsHTMLElement(nsGkAtoms::body),
              "GetBodyElement returned something bogus");
 
-  // 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);
-
+  const auto* bodyStyle = Servo_Element_GetMaybeOutOfDateStyle(bodyElement);
   if (CheckOverflow(bodyStyle, aStyles)) {
     // tell caller we stole the overflow style from the body element
     return bodyElement;

layout/generic/WritingModes.h

@@ -450,7 +450,7 @@ class WritingMode {
   /**
    * Construct writing mode based on a ComputedStyle.
    */
-  explicit WritingMode(ComputedStyle* aComputedStyle) {
+  explicit WritingMode(const ComputedStyle* aComputedStyle) {
     NS_ASSERTION(aComputedStyle, "we need an ComputedStyle here");
     InitFromStyleVisibility(aComputedStyle->StyleVisibility());
   }

testing/web-platform/meta/css/css-overflow/overflow-body-propagation-002.html.ini

@@ -1,4 +0,0 @@
-[overflow-body-propagation-002.html]
-  expected:
-    if os == "android": PASS
-    FAIL

testing/web-platform/meta/css/css-overflow/overflow-body-propagation-003.html.ini

@@ -1,4 +0,0 @@
-[overflow-body-propagation-003.html]
-  expected:
-    if os == "android": PASS
-    FAIL