Get the main_frame_scroll_offset/viewport_size from the frame owner’s local root

What: In FrameView::UpdateViewportIntersection, get the
main_frame_scroll_offset from the frame owner’s local root’s
intersection_state, instead of from the frame owner’s top Frame.

Why: We already store this in the local root's
viewport_intersection_state. Use that value, instead of additionally
storing it to the top Frame.

This is not only a redundancy, but can be a problem in sticky frame
tracking when there are multiple subframes such as
A(B1(C1(D1)),B2(C2(D2))), because an update in C1 or C2 will override
the value that's previously set by the other, which could eventually
cause false positively recordings of large-sticky-ads. Storing them in
the local root will prevent this racing.


Bug: 1013070, 1063760
Change-Id: If7bdc05a1b487bf4cfe70a3823b28d78ef0f938f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2110473Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Stefan Zager <szager@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753280}

content/browser/site_per_process_browsertest.cc

@@ -15551,6 +15551,131 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
   }
 }
 
+// Tests that main_frame_scroll_offset is not shared by frames in the same
+// process. This is a regression test for https://crbug.com/1063760.
+//
+// Set up the frame tree to be A(B1(C1(D1)),B2(C2(D2))) where B1 is above B2.
+// Scroll page A up to the point that B1(C1(D1)) intersects with A, while
+// B2(C2(D2)) is still within A. After the scrolling, C1 will see the up-to-date
+// value of the main_frame_scroll_offset, while in C2 it's still 0 because there
+// was no animation frame scheduled. Finally, we hide D2 to force an update in
+// C2. It's expected that the main_frame_scroll_offset sent from C2 to D2 is 0
+// rather than the current scroll offset.
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, MainFrameScrollOffset) {
+  GURL a_url = embedded_test_server()->GetURL(
+      "a.com", "/frame_tree/scrollable_page_with_two_frames.html");
+  GURL b_url = embedded_test_server()->GetURL(
+      "b.com", "/frame_tree/page_with_large_iframe.html");
+  GURL c_url = embedded_test_server()->GetURL(
+      "c.com", "/frame_tree/page_with_large_iframe.html");
+  GURL d_url = embedded_test_server()->GetURL("d.com", "/title1.html");
+
+  EXPECT_TRUE(NavigateToURL(shell(), a_url));
+  FrameTreeNode* a_node = web_contents()->GetFrameTree()->root();
+
+  FrameTreeNode* b1_node = a_node->child_at(0);
+  NavigateFrameToURL(b1_node, b_url);
+
+  FrameTreeNode* c1_node = b1_node->child_at(0);
+  NavigateFrameToURL(c1_node, c_url);
+
+  FrameTreeNode* d1_node = c1_node->child_at(0);
+  NavigateFrameToURL(d1_node, d_url);
+
+  FrameTreeNode* b2_node = a_node->child_at(1);
+  NavigateFrameToURL(b2_node, b_url);
+
+  FrameTreeNode* c2_node = b2_node->child_at(0);
+  NavigateFrameToURL(c2_node, c_url);
+
+  FrameTreeNode* d2_node = c2_node->child_at(0);
+  NavigateFrameToURL(d2_node, d_url);
+
+  float scale_factor = 1.0f;
+  if (IsUseZoomForDSFEnabled())
+    scale_factor = GetFrameDeviceScaleFactor(shell()->web_contents());
+
+  // This will intercept messages sent from C1 to D1, describing D1's viewport
+  // intersection.
+  scoped_refptr<UpdateViewportIntersectionMessageFilter>
+      c1_to_d1_message_filter = new UpdateViewportIntersectionMessageFilter();
+  c1_node->current_frame_host()->GetProcess()->AddFilter(
+      c1_to_d1_message_filter.get());
+
+  // This will intercept messages sent from C2 to D2, describing D2's viewport
+  // intersection.
+  scoped_refptr<UpdateViewportIntersectionMessageFilter>
+      c2_to_d2_message_filter = new UpdateViewportIntersectionMessageFilter();
+  c2_node->current_frame_host()->GetProcess()->AddFilter(
+      c2_to_d2_message_filter.get());
+
+  // Run requestAnimationFrame in A, B1, C1, B2, C2 to make sure initial layout
+  // has completed and initial IPCs are sent.
+  ASSERT_TRUE(EvalJsAfterLifecycleUpdate(a_node->current_frame_host(), "", "")
+                  .error.empty());
+  ASSERT_TRUE(EvalJsAfterLifecycleUpdate(b1_node->current_frame_host(), "", "")
+                  .error.empty());
+  ASSERT_TRUE(EvalJsAfterLifecycleUpdate(c1_node->current_frame_host(), "", "")
+                  .error.empty());
+  ASSERT_TRUE(EvalJsAfterLifecycleUpdate(b2_node->current_frame_host(), "", "")
+                  .error.empty());
+  ASSERT_TRUE(EvalJsAfterLifecycleUpdate(c2_node->current_frame_host(), "", "")
+                  .error.empty());
+
+  c1_to_d1_message_filter->Clear();
+  c2_to_d2_message_filter->Clear();
+
+  // Scroll page A up to the point that B1(C1(D1)) intersects with A, while
+  // B2(C2(D2)) is still within A.
+  int scroll_offset_y = d1_node->render_manager()
+                            ->GetProxyToParent()
+                            ->cross_process_frame_connector()
+                            ->intersection_state()
+                            .viewport_offset.y() +
+                        1;
+  ASSERT_TRUE(
+      ExecJs(a_node->current_frame_host(),
+             content::JsReplace("window.scrollTo(0, $1)", scroll_offset_y)));
+  c1_to_d1_message_filter->Wait();
+
+  // After the scrolling, C1 will see the up-to-date value of the
+  // main_frame_scroll_offset, while in C2 it's still 0 because there was no
+  // animation frame scheduled.
+  EXPECT_FALSE(c2_to_d2_message_filter->MessageReceived());
+
+  EXPECT_EQ(c1_to_d1_message_filter->GetIntersectionState()
+                .main_frame_scroll_offset.y(),
+            static_cast<int>(scroll_offset_y * scale_factor));
+
+  // Run requestAnimationFrame in A, B1, C1, B2, C2 to make sure IPCs are sent.
+  ASSERT_TRUE(EvalJsAfterLifecycleUpdate(a_node->current_frame_host(), "", "")
+                  .error.empty());
+  ASSERT_TRUE(EvalJsAfterLifecycleUpdate(b1_node->current_frame_host(), "", "")
+                  .error.empty());
+  ASSERT_TRUE(EvalJsAfterLifecycleUpdate(c1_node->current_frame_host(), "", "")
+                  .error.empty());
+  ASSERT_TRUE(EvalJsAfterLifecycleUpdate(b2_node->current_frame_host(), "", "")
+                  .error.empty());
+  ASSERT_TRUE(EvalJsAfterLifecycleUpdate(c2_node->current_frame_host(), "", "")
+                  .error.empty());
+
+  c1_to_d1_message_filter->Clear();
+  c2_to_d2_message_filter->Clear();
+
+  // Hide D2 to force an update in C2. It's expected that the
+  // main_frame_scroll_offset sent from C2 to D2 is 0 rather than the current
+  // scroll offset.
+  ASSERT_TRUE(ExecJs(
+      c2_node->current_frame_host(),
+      "let f = document.getElementsByTagName('iframe')[0];f.style.display = "
+      "'none';"));
+
+  c2_to_d2_message_filter->Wait();
+  EXPECT_EQ(c2_to_d2_message_filter->GetIntersectionState()
+                .main_frame_scroll_offset.y(),
+            0);
+}
+
 class SitePerProcessCompositorViewportBrowserTest
     : public SitePerProcessBrowserTest,
       public testing::WithParamInterface<double> {

content/test/data/frame_tree/page_with_large_iframe.html

+<!DOCTYPE html>
+<html>
+<body>
+<style>
+iframe {
+  width: 90vw;
+  height: 90vh;
+}
+</style>
+<iframe></iframe>
+</body>
+</html>

content/test/data/frame_tree/scrollable_page_with_two_frames.html

+<!DOCTYPE html>
+<style>
+iframe {
+  width: 90vw;
+  height: 40vh;
+}
+div {
+  position:absolute;
+  top: 10000px;
+  left: 50px;
+  width: 100px;
+  height: 100px;
+}
+</style>
+<html>
+<body>
+<iframe></iframe>
+<br>
+<iframe></iframe>
+<br>
+Scrollable page contains two iframes.
+<div></div>
+</body>
+</html>

third_party/blink/renderer/core/frame/frame.h

@@ -274,9 +274,7 @@ class CORE_EXPORT Frame : public GarbageCollected<Frame> {
   // Called when the focus controller changes the focus to this frame.
   virtual void DidFocus() = 0;
 
-  virtual void SetMainFrameViewportSize(const IntSize&) {}
   virtual IntSize GetMainFrameViewportSize() const = 0;
-  virtual void SetMainFrameScrollOffset(const IntPoint&) {}
   virtual IntPoint GetMainFrameScrollOffset() const = 0;
 
  protected:

third_party/blink/renderer/core/frame/local_frame.cc

@@ -1516,22 +1516,25 @@ void LocalFrame::SetViewportIntersectionFromParent(
       frame_view->ScheduleAnimation();
     }
   }
-  Tree().Top().SetMainFrameViewportSize(
-      intersection_state.main_frame_viewport_size);
-  Tree().Top().SetMainFrameScrollOffset(
-      IntPoint(intersection_state.main_frame_scroll_offset));
 }
 
 IntSize LocalFrame::GetMainFrameViewportSize() const {
-  if (!IsMainFrame())
-    return Tree().Top().GetMainFrameViewportSize();
-  return View()->GetScrollableArea()->VisibleContentRect().Size();
+  LocalFrame& local_root = LocalFrameRoot();
+  return local_root.IsMainFrame()
+             ? local_root.View()
+                   ->GetScrollableArea()
+                   ->VisibleContentRect()
+                   .Size()
+             : IntSize(local_root.intersection_state_.main_frame_viewport_size);
 }
 
 IntPoint LocalFrame::GetMainFrameScrollOffset() const {
-  if (!IsMainFrame())
-    return Tree().Top().GetMainFrameScrollOffset();
-  return FlooredIntPoint(View()->GetScrollableArea()->GetScrollOffset());
+  LocalFrame& local_root = LocalFrameRoot();
+  return local_root.IsMainFrame()
+             ? FlooredIntPoint(
+                   local_root.View()->GetScrollableArea()->GetScrollOffset())
+             : IntPoint(
+                   local_root.intersection_state_.main_frame_scroll_offset);
 }
 
 FrameOcclusionState LocalFrame::GetOcclusionState() const {

third_party/blink/renderer/core/frame/remote_frame.cc

@@ -602,28 +602,18 @@ void RemoteFrame::DidUpdateFramePolicy(const FramePolicy& frame_policy) {
   To<RemoteFrameOwner>(Owner())->SetFramePolicy(frame_policy);
 }
 
-void RemoteFrame::SetMainFrameViewportSize(
-    const IntSize& main_frame_viewport_size) {
-  DCHECK(IsMainFrame());
-  main_frame_viewport_size_ = main_frame_viewport_size;
-}
-
 IntSize RemoteFrame::GetMainFrameViewportSize() const {
-  if (!IsMainFrame())
-    return Tree().Top().GetMainFrameViewportSize();
-  return main_frame_viewport_size_;
-}
-
-void RemoteFrame::SetMainFrameScrollOffset(
-    const IntPoint& main_frame_scroll_offset) {
-  DCHECK(IsMainFrame());
-  main_frame_scroll_offset_ = main_frame_scroll_offset;
+  HTMLFrameOwnerElement* owner = DeprecatedLocalOwner();
+  DCHECK(owner);
+  DCHECK(owner->GetDocument().GetFrame());
+  return owner->GetDocument().GetFrame()->GetMainFrameViewportSize();
 }
 
 IntPoint RemoteFrame::GetMainFrameScrollOffset() const {
-  if (!IsMainFrame())
-    return Tree().Top().GetMainFrameScrollOffset();
-  return main_frame_scroll_offset_;
+  HTMLFrameOwnerElement* owner = DeprecatedLocalOwner();
+  DCHECK(owner);
+  DCHECK(owner->GetDocument().GetFrame());
+  return owner->GetDocument().GetFrame()->GetMainFrameScrollOffset();
 }
 
 bool RemoteFrame::IsIgnoredForHitTest() const {

third_party/blink/renderer/core/frame/remote_frame.h

@@ -140,9 +140,8 @@ class CORE_EXPORT RemoteFrame final : public Frame,
   // the next navigation.
   void DidUpdateFramePolicy(const FramePolicy& frame_policy) override;
 
-  void SetMainFrameViewportSize(const IntSize&) override;
+  // Called only when this frame has a local frame owner.
   IntSize GetMainFrameViewportSize() const override;
-  void SetMainFrameScrollOffset(const IntPoint&) override;
   IntPoint GetMainFrameScrollOffset() const override;
 
  private:
@@ -167,8 +166,6 @@ class CORE_EXPORT RemoteFrame final : public Frame,
   bool prevent_contents_opaque_changes_ = false;
   bool is_surface_layer_ = false;
   ParsedFeaturePolicy feature_policy_header_;
-  IntSize main_frame_viewport_size_;
-  IntPoint main_frame_scroll_offset_;
 
   mojo::AssociatedRemote<mojom::blink::RemoteFrameHost>
       remote_frame_host_remote_;