[RootScroller] IFrame should use content box

This CL makes iframe candidates use the content box of the iframe when
considering whether an iframe is "viewport filling" and eligible for
promotion to root scroller.

The reason iframes are special is because of how layout works for root
scroller iframes. When an iframe is made the root scroller, the layout
size we use to layout its content is taken from its parent frame. This
is done because of the special semantics of the Android URL bar and how
it affects the layout of the page [1]. We pass through the layout size
in order to preserve this behavior with respect to the URL bar.

Unfortunately, this means that promoting an iframe with a padding would
cause the content to reflow into the parent frame's layout size (which
cannot set a padding on the LayoutView) which can cause surprising
results like in the linked bug.

This CL restricts iframe promotion to just those iframe's whose content
rect matches the viewport. General elements like scrolling <div>s may
continue to be promoted with padding since they don't suffer from this
issue.

[1] https://developers.google.com/web/updates/2016/12/url-bar-resizing

Bug: 945779
Change-Id: I5074f43832a2faa5b268ab99ef5bd861bdff21dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1545506
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#646375}

third_party/blink/renderer/core/page/scrolling/root_scroller_controller.cc

@@ -36,15 +36,26 @@ bool FillsViewport(const Element& element) {
   if (!element.GetLayoutObject())
     return false;
 
-  LayoutObject* layout_object = element.GetLayoutObject();
+  DCHECK(element.GetLayoutObject()->IsBox());
+
+  LayoutBox* layout_box = ToLayoutBox(element.GetLayoutObject());
 
   // TODO(bokan): Broken for OOPIF. crbug.com/642378.
   Document& top_document = element.GetDocument().TopDocument();
   if (!top_document.GetLayoutView())
     return false;
 
-  FloatQuad quad = layout_object->LocalToAbsoluteQuad(
-      FloatRect(ToLayoutBox(layout_object)->PhysicalPaddingBoxRect()));
+  // We need to be more strict for iframes and use the content box since the
+  // iframe will use the parent's layout size. Using the padding box would mean
+  // the content would relayout on promotion/demotion. The layout size matching
+  // the parent is done to ensure consistent semantics with respect to how the
+  // mobile URL bar affects layout, which isn't a concern for non-iframe
+  // elements because those semantics will already be applied to the element.
+  LayoutRect rect = layout_box->IsLayoutIFrame()
+                        ? layout_box->PhysicalContentBoxRect()
+                        : layout_box->PhysicalPaddingBoxRect();
+
+  FloatQuad quad = layout_box->LocalToAbsoluteQuad(FloatRect(rect));
 
   if (!quad.IsRectilinear())
     return false;

third_party/blink/renderer/core/page/scrolling/root_scroller_test.cc

@@ -2555,6 +2555,91 @@ TEST_F(ImplicitRootScrollerSimTest, PromotionChangesLayoutSize) {
       << "Once loaded, the iframe should be promoted.";
 }
 
+// Ensure that we're using the content box for an iframe. Promotion will cause
+// the content to use the layout size of the parent frame so having padding or
+// a border would cause us to relayout.
+TEST_F(ImplicitRootScrollerSimTest, IframeUsesContentBox) {
+  WebView().ResizeWithBrowserControls(IntSize(800, 600), 0, 0, false);
+  SimRequest main_request("https://example.com/test.html", "text/html");
+  SimRequest child_request("https://example.com/child.html", "text/html");
+  LoadURL("https://example.com/test.html");
+  main_request.Complete(R"HTML(
+          <!DOCTYPE>
+          <style>
+            iframe {
+              position: absolute;
+              top: 0;
+              left: 0;
+              width: 100%;
+              height: 100%;
+              border: none;
+              box-sizing: border-box;
+
+            }
+            body, html {
+              margin: 0;
+              width: 100%;
+              height: 100%;
+              overflow:hidden;
+            }
+
+          </style>
+          <iframe id="container" src="child.html">
+      )HTML");
+  child_request.Complete(R"HTML(
+        <!DOCTYPE html>
+        <style>
+          div {
+            border: 5px solid black;
+            background-color: red;
+            width: 99%;
+            height: 100px;
+          }
+          html {
+            height: 200%;
+          }
+        </style>
+        <div></div>
+  )HTML");
+
+  Compositor().BeginFrame();
+
+  Element* iframe = GetDocument().getElementById("container");
+
+  ASSERT_EQ(iframe,
+            GetDocument().GetRootScrollerController().EffectiveRootScroller())
+      << "The iframe should start off promoted.";
+
+  // Adding padding should cause the iframe to be demoted.
+  {
+    iframe->setAttribute(html_names::kStyleAttr, "padding-left: 20%");
+    Compositor().BeginFrame();
+
+    EXPECT_NE(iframe,
+              GetDocument().GetRootScrollerController().EffectiveRootScroller())
+        << "The iframe should be demoted once it has padding.";
+  }
+
+  // Replacing padding with a border should also ensure the iframe remains
+  // demoted.
+  {
+    iframe->setAttribute(html_names::kStyleAttr, "border: 5px solid black");
+    Compositor().BeginFrame();
+
+    EXPECT_NE(iframe,
+              GetDocument().GetRootScrollerController().EffectiveRootScroller())
+        << "The iframe should be demoted once it has border.";
+  }
+
+  // Removing the border should now cause the iframe to be promoted once again.
+  iframe->setAttribute(html_names::kStyleAttr, "");
+  Compositor().BeginFrame();
+
+  ASSERT_EQ(iframe,
+            GetDocument().GetRootScrollerController().EffectiveRootScroller())
+      << "The iframe should once again be promoted when border is removed";
+}
+
 // Test that we don't promote any elements implicitly if the main document has
 // vertical scrolling.
 TEST_F(ImplicitRootScrollerSimTest, OverflowInMainDocumentRestrictsImplicit) {