Fix fragment anchor invoked without layout in iframe

In https://crrev.com/45236fd563e9df53dc45579be1f3d0b4784885a2 I did a
heavy refactoring of how fragment anchors work and when they get
invoked. This refactoring removed the layout call from
LocalFrameView::ProcessUrlFragment. This is desirable since it should be
layout that drives the fragment anchor, not the other way around. In
general avoiding surprise layouts is good. I assumed this would be ok
since we'll invoke the fragment anchor again in the next layout.

However, there's a case where this isn't true. When a document finishes
loading (Document::ImplicitClose) it will invoke the fragment anchor.
The fragment anchor will then be removed since the document is already
in the "load completed" state. If the iframe finishes loading without
any layouts this call will be the only invocation and it'll happen on
a dirty layout tree.

The solution in this CL is to remove that invocation in ImplicitClose
while layout is dirty potentially dirty. If layout is dirty it'll be
invoked when we perform the layout. This leaves the only two ways
for the anchor to be invoked as:
 - Right after a layout
 - When the fragment is first set and layout is already clean

Change-Id: I3f038a4e35038a0cd9e4a17593b9b6b3d810068f
Bug: 934405
Reviewed-on: https://chromium-review.googlesource.com/c/1481054
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: Stefan Zager <szager@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636548}

third_party/blink/renderer/core/dom/document.cc

@@ -3501,9 +3501,6 @@ void Document::ImplicitClose() {
       View()->UpdateLayout();
   }
 
-  if (View())
-    View()->InvokeFragmentAnchor();
-
   load_event_progress_ = kLoadEventCompleted;
 
   if (GetFrame() && GetLayoutView()) {

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

@@ -8,6 +8,7 @@
 #include "third_party/blink/renderer/core/frame/local_frame_view.h"
 #include "third_party/blink/renderer/core/frame/web_local_frame_impl.h"
 #include "third_party/blink/renderer/core/html/html_anchor_element.h"
+#include "third_party/blink/renderer/core/html/html_frame_owner_element.h"
 #include "third_party/blink/renderer/core/layout/layout_object.h"
 #include "third_party/blink/renderer/core/page/focus_controller.h"
 #include "third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h"
@@ -21,7 +22,16 @@ namespace {
 
 using test::RunPendingTasks;
 
-class ElementFragmentAnchorTest : public SimTest {};
+class ElementFragmentAnchorTest : public SimTest {
+  void SetUp() override {
+    SimTest::SetUp();
+
+    // Focus handlers aren't run unless the page is focused.
+    GetDocument().GetPage()->GetFocusController().SetFocused(true);
+
+    WebView().MainFrameWidget()->Resize(WebSize(800, 600));
+  }
+};
 
 // Ensure that the focus event handler is run before the rAF callback. We'll
 // change the background color from a rAF set in the focus handler and make
@@ -54,9 +64,6 @@ TEST_F(ElementFragmentAnchorTest, FocusHandlerRunBeforeRaf) {
       });
   )HTML"));
 
-  // Focus handlers aren't run unless the page is focused.
-  GetDocument().GetPage()->GetFocusController().SetFocused(true);
-
   // We're still waiting on the stylesheet to load so the load event shouldn't
   // yet dispatch and rendering is deferred.
   ASSERT_FALSE(GetDocument().IsRenderingReady());
@@ -89,6 +96,118 @@ TEST_F(ElementFragmentAnchorTest, FocusHandlerRunBeforeRaf) {
             Color(0, 255, 0).NameForLayoutTreeAsText());
 }
 
+// This test ensures that when an iframe's document is closed, and the parent
+// has dirty layout, the iframe is laid out prior to invoking its fragment
+// anchor. Without performing this layout, the anchor cannot scroll to the
+// correct location and it will be cleared since the document is closed.
+TEST_F(ElementFragmentAnchorTest, IframeFragmentNoLayoutUntilLoad) {
+  SimRequest main_resource("https://example.com/test.html", "text/html");
+  SimRequest child_resource("https://example.com/child.html#fragment",
+                            "text/html");
+  LoadURL("https://example.com/test.html");
+
+  // Don't clcose the main document yet, since that'll cause it to layout.
+  main_resource.Write(R"HTML(
+      <!DOCTYPE html>
+      <style>
+        iframe {
+          border: 0;
+          width: 300px;
+          height: 200px;
+        }
+      </style>
+      <iframe id="child" src="child.html#fragment"></iframe>
+    )HTML");
+
+  // When the iframe document is loaded, it'll try to scroll the fragment into
+  // view. Ensure it does so correctly by laying out first.
+  child_resource.Complete(R"HTML(
+      <!DOCTYPE html>
+      <div style="height:500px;">content</div>
+      <div id="fragment">fragment content</div>
+    )HTML");
+  Compositor().BeginFrame();
+
+  HTMLFrameOwnerElement* iframe =
+      To<HTMLFrameOwnerElement>(GetDocument().getElementById("child"));
+  ScrollableArea* child_viewport =
+      iframe->contentDocument()->View()->LayoutViewport();
+  Element* fragment = iframe->contentDocument()->getElementById("fragment");
+
+  IntRect fragment_rect_in_frame(
+      fragment->GetLayoutObject()->AbsoluteBoundingBoxRect());
+  IntRect viewport_rect(IntPoint(),
+                        child_viewport->VisibleContentRect().Size());
+
+  EXPECT_TRUE(viewport_rect.Contains(fragment_rect_in_frame))
+      << "Fragment element at [" << fragment_rect_in_frame.ToString()
+      << "] was not scrolled into viewport rect [" << viewport_rect.ToString()
+      << "]";
+
+  main_resource.Finish();
+}
+
+// This test ensures that we correctly scroll the fragment into view in the
+// case that the iframe has finished load but layout becomes dirty (in both
+// parent and iframe) before we've had a chance to scroll the fragment into
+// view.
+TEST_F(ElementFragmentAnchorTest, IframeFragmentDirtyLayoutAfterLoad) {
+  SimRequest main_resource("https://example.com/test.html", "text/html");
+  SimRequest child_resource("https://example.com/child.html#fragment",
+                            "text/html");
+  LoadURL("https://example.com/test.html");
+
+  // Don't clcose the main document yet, since that'll cause it to layout.
+  main_resource.Write(R"HTML(
+      <!DOCTYPE html>
+      <style>
+        iframe {
+          border: 0;
+          width: 300px;
+          height: 200px;
+        }
+      </style>
+      <iframe id="child" src="child.html#fragment"></iframe>
+    )HTML");
+
+  // Use text so that changing the iframe width will change the y-location of
+  // the fragment.
+  child_resource.Complete(R"HTML(
+      <!DOCTYPE html>
+      Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum
+      Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum
+      Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum
+      Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum
+      Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum
+      Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum
+      Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum
+      Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum Lorem Ipsum
+      <div id="fragment">fragment content</div>
+    )HTML");
+
+  HTMLFrameOwnerElement* iframe =
+      To<HTMLFrameOwnerElement>(GetDocument().getElementById("child"));
+  iframe->setAttribute(html_names::kStyleAttr, "width:100px");
+
+  Compositor().BeginFrame();
+
+  ScrollableArea* child_viewport =
+      iframe->contentDocument()->View()->LayoutViewport();
+  Element* fragment = iframe->contentDocument()->getElementById("fragment");
+
+  IntRect fragment_rect_in_frame(
+      fragment->GetLayoutObject()->AbsoluteBoundingBoxRect());
+  IntRect viewport_rect(IntPoint(),
+                        child_viewport->VisibleContentRect().Size());
+
+  EXPECT_TRUE(viewport_rect.Contains(fragment_rect_in_frame))
+      << "Fragment element at [" << fragment_rect_in_frame.ToString()
+      << "] was not scrolled into viewport rect [" << viewport_rect.ToString()
+      << "]";
+
+  main_resource.Finish();
+}
+
 }  // namespace
 
 }  // namespace blink