Allow select popups to check viewport position within OOPIFs

Select menu dropdowns use LocalFrameView::ContentsToViewport to
determine what part of the menu intersects with the window viewport.
This doesn't work in OOPIFs because LFV::ContentsToRootFrame only
converts coordinates into the local frame root's coordinates, not the
main frame. The consequence is that menus in large iframes that have
been scrolled into view will falsely report to be out of view.

This patch changes the intersection calculation to use
LayoutView::MapToVisualRectInAncestorSpace(), which determines the
viewport intersection while accounting for OOPIF containers.

Bug: 837639
Change-Id: If265e24ad4cbee967fdb97ae85002a81e75033a7
Reviewed-on: https://chromium-review.googlesource.com/1036232Reviewed-by: Steve Kobes <skobes@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Commit-Queue: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556355}

content/browser/site_per_process_browsertest.cc

@@ -11700,6 +11700,60 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
   EXPECT_LE(compositing_rect.y(), expected_offset + 2);
 }
 
+// Verify that OOPIF select element popup menu coordinates account for scroll
+// offset in containers embedding frame.
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, PopupMenuInTallIframeTest) {
+  GURL main_url(embedded_test_server()->GetURL(
+      "/frame_tree/page_with_tall_positioned_frame.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), main_url));
+
+  FrameTreeNode* root = web_contents()->GetFrameTree()->root();
+  FrameTreeNode* child_node = root->child_at(0);
+  GURL site_url(embedded_test_server()->GetURL(
+      "baz.com", "/site_isolation/page-with-select.html"));
+  NavigateFrameToURL(child_node, site_url);
+
+  scoped_refptr<UpdateViewportIntersectionMessageFilter> filter =
+      new UpdateViewportIntersectionMessageFilter();
+  root->current_frame_host()->GetProcess()->AddFilter(filter.get());
+
+  // Position the select element so that it is out of the viewport, then scroll
+  // it into view.
+  EXPECT_TRUE(ExecuteScript(
+      child_node, "document.querySelector('select').style.top='2000px';"));
+  EXPECT_TRUE(ExecuteScript(root, "window.scrollTo(0, 1900);"));
+
+  // Wait for a viewport intersection update to be dispatched to the child, and
+  // ensure it is processed by the browser before continuing.
+  filter->Wait();
+  {
+    // This yields the UI thread in order to ensure that the new viewport
+    // intersection is sent to the to child renderer before the mouse click
+    // below.
+    base::RunLoop loop;
+    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+                                                  loop.QuitClosure());
+    loop.Run();
+  }
+
+  scoped_refptr<ShowWidgetMessageFilter> show_widget_filter =
+      new ShowWidgetMessageFilter();
+  child_node->current_frame_host()->GetProcess()->AddFilter(
+      show_widget_filter.get());
+
+  SimulateMouseClick(child_node->current_frame_host()->GetRenderWidgetHost(),
+                     55, 2005);
+
+  // Dismiss the popup.
+  SimulateMouseClick(child_node->current_frame_host()->GetRenderWidgetHost(), 1,
+                     1);
+
+  // The test passes if this wait returns, indicating that the popup was
+  // scrolled into view and the OOPIF renderer displayed it. Other tests verify
+  // the correctness of popup menu coordinates.
+  show_widget_filter->Wait();
+}
+
 namespace {
 
 // Helper class to intercept DidCommitProvisionalLoad messages and inject a

content/browser/site_per_process_hit_test_browsertest.cc

@@ -3219,76 +3219,6 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessHighDPIHitTestBrowserTest,
   CreateContextMenuTestHelper(shell(), embedded_test_server());
 }
 
-class ShowWidgetMessageFilter : public content::BrowserMessageFilter {
- public:
-  ShowWidgetMessageFilter()
-#if defined(OS_MACOSX) || defined(OS_ANDROID)
-      : content::BrowserMessageFilter(FrameMsgStart),
-#else
-      : content::BrowserMessageFilter(ViewMsgStart),
-#endif
-        message_loop_runner_(new content::MessageLoopRunner) {
-  }
-
-  bool OnMessageReceived(const IPC::Message& message) override {
-    IPC_BEGIN_MESSAGE_MAP(ShowWidgetMessageFilter, message)
-#if defined(OS_MACOSX) || defined(OS_ANDROID)
-      IPC_MESSAGE_HANDLER(FrameHostMsg_ShowPopup, OnShowPopup)
-#else
-      IPC_MESSAGE_HANDLER(ViewHostMsg_ShowWidget, OnShowWidget)
-#endif
-    IPC_END_MESSAGE_MAP()
-    return false;
-  }
-
-  gfx::Rect last_initial_rect() const { return initial_rect_; }
-
-  int last_routing_id() const { return routing_id_; }
-
-  void Wait() {
-    initial_rect_ = gfx::Rect();
-    routing_id_ = MSG_ROUTING_NONE;
-    message_loop_runner_->Run();
-  }
-
-  void Reset() {
-    initial_rect_ = gfx::Rect();
-    routing_id_ = MSG_ROUTING_NONE;
-    message_loop_runner_ = new content::MessageLoopRunner;
-  }
-
- private:
-  ~ShowWidgetMessageFilter() override {}
-
-  void OnShowWidget(int route_id, const gfx::Rect& initial_rect) {
-    content::BrowserThread::PostTask(
-        content::BrowserThread::UI, FROM_HERE,
-        base::BindOnce(&ShowWidgetMessageFilter::OnShowWidgetOnUI, this,
-                       route_id, initial_rect));
-  }
-
-#if defined(OS_MACOSX) || defined(OS_ANDROID)
-  void OnShowPopup(const FrameHostMsg_ShowPopup_Params& params) {
-    content::BrowserThread::PostTask(
-        content::BrowserThread::UI, FROM_HERE,
-        base::Bind(&ShowWidgetMessageFilter::OnShowWidgetOnUI, this,
-                   MSG_ROUTING_NONE, params.bounds));
-  }
-#endif
-
-  void OnShowWidgetOnUI(int route_id, const gfx::Rect& initial_rect) {
-    initial_rect_ = initial_rect;
-    routing_id_ = route_id;
-    message_loop_runner_->Quit();
-  }
-
-  scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
-  gfx::Rect initial_rect_;
-  int routing_id_;
-
-  DISALLOW_COPY_AND_ASSIGN(ShowWidgetMessageFilter);
-};
-
 // Test that clicking a select element in an out-of-process iframe creates
 // a popup menu in the correct position.
 IN_PROC_BROWSER_TEST_P(SitePerProcessHitTestBrowserTest, PopupMenuTest) {

content/test/content_browser_test_utils_internal.cc

@@ -22,8 +22,8 @@
 #include "content/browser/frame_host/render_frame_host_delegate.h"
 #include "content/browser/frame_host/render_frame_proxy_host.h"
 #include "content/browser/renderer_host/delegated_frame_host.h"
-#include "content/common/frame_messages.h"
 #include "content/common/frame_visual_properties.h"
+#include "content/common/view_messages.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
@@ -397,4 +397,63 @@ RenderProcessHostKillWaiter::Wait() {
   return static_cast<bad_message::BadMessageReason>(bucket.min);
 }
 
+ShowWidgetMessageFilter::ShowWidgetMessageFilter()
+#if defined(OS_MACOSX) || defined(OS_ANDROID)
+    : content::BrowserMessageFilter(FrameMsgStart),
+#else
+    : content::BrowserMessageFilter(ViewMsgStart),
+#endif
+      message_loop_runner_(new content::MessageLoopRunner) {
+}
+
+ShowWidgetMessageFilter::~ShowWidgetMessageFilter() {}
+
+bool ShowWidgetMessageFilter::OnMessageReceived(const IPC::Message& message) {
+  IPC_BEGIN_MESSAGE_MAP(ShowWidgetMessageFilter, message)
+#if defined(OS_MACOSX) || defined(OS_ANDROID)
+    IPC_MESSAGE_HANDLER(FrameHostMsg_ShowPopup, OnShowPopup)
+#else
+    IPC_MESSAGE_HANDLER(ViewHostMsg_ShowWidget, OnShowWidget)
+#endif
+  IPC_END_MESSAGE_MAP()
+  return false;
+}
+
+void ShowWidgetMessageFilter::Wait() {
+  initial_rect_ = gfx::Rect();
+  routing_id_ = MSG_ROUTING_NONE;
+  message_loop_runner_->Run();
+}
+
+void ShowWidgetMessageFilter::Reset() {
+  initial_rect_ = gfx::Rect();
+  routing_id_ = MSG_ROUTING_NONE;
+  message_loop_runner_ = new content::MessageLoopRunner;
+}
+
+void ShowWidgetMessageFilter::OnShowWidget(int route_id,
+                                           const gfx::Rect& initial_rect) {
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::BindOnce(&ShowWidgetMessageFilter::OnShowWidgetOnUI, this, route_id,
+                     initial_rect));
+}
+
+#if defined(OS_MACOSX) || defined(OS_ANDROID)
+void ShowWidgetMessageFilter::OnShowPopup(
+    const FrameHostMsg_ShowPopup_Params& params) {
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(&ShowWidgetMessageFilter::OnShowWidgetOnUI, this,
+                 MSG_ROUTING_NONE, params.bounds));
+}
+#endif
+
+void ShowWidgetMessageFilter::OnShowWidgetOnUI(int route_id,
+                                               const gfx::Rect& initial_rect) {
+  initial_rect_ = initial_rect;
+  routing_id_ = route_id;
+  message_loop_runner_->Quit();
+}
+
 }  // namespace content

content/test/content_browser_test_utils_internal.h

@@ -21,11 +21,14 @@
 #include "base/optional.h"
 #include "base/run_loop.h"
 #include "base/test/histogram_tester.h"
+#include "build/build_config.h"
 #include "content/browser/bad_message.h"
+#include "content/common/frame_messages.h"
 #include "content/public/browser/resource_dispatcher_host_delegate.h"
 #include "content/public/browser/web_contents_delegate.h"
 #include "content/public/common/file_chooser_params.h"
 #include "content/public/test/browser_test_utils.h"
+#include "content/public/test/test_utils.h"
 #include "url/gurl.h"
 
 namespace content {
@@ -217,6 +220,38 @@ class RenderProcessHostKillWaiter {
   DISALLOW_COPY_AND_ASSIGN(RenderProcessHostKillWaiter);
 };
 
+class ShowWidgetMessageFilter : public content::BrowserMessageFilter {
+ public:
+  ShowWidgetMessageFilter();
+
+  bool OnMessageReceived(const IPC::Message& message) override;
+
+  gfx::Rect last_initial_rect() const { return initial_rect_; }
+
+  int last_routing_id() const { return routing_id_; }
+
+  void Wait();
+
+  void Reset();
+
+ private:
+  ~ShowWidgetMessageFilter() override;
+
+  void OnShowWidget(int route_id, const gfx::Rect& initial_rect);
+
+#if defined(OS_MACOSX) || defined(OS_ANDROID)
+  void OnShowPopup(const FrameHostMsg_ShowPopup_Params& params);
+#endif
+
+  void OnShowWidgetOnUI(int route_id, const gfx::Rect& initial_rect);
+
+  scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
+  gfx::Rect initial_rect_;
+  int routing_id_;
+
+  DISALLOW_COPY_AND_ASSIGN(ShowWidgetMessageFilter);
+};
+
 }  // namespace content
 
 #endif  // CONTENT_TEST_CONTENT_BROWSER_TEST_UTILS_INTERNAL_H_

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

@@ -1280,17 +1280,39 @@ IntRect Element::BoundsInViewport() const {
 }
 
 IntRect Element::VisibleBoundsInVisualViewport() const {
-  if (!GetLayoutObject() || !GetDocument().GetPage())
+  if (!GetLayoutObject() || !GetDocument().GetPage() ||
+      !GetDocument().GetFrame())
     return IntRect();
-  // TODO(tkent): Can we check invisibility by scrollable non-frame elements?
 
-  IntSize viewport_size = GetDocument().GetPage()->GetVisualViewport().Size();
-  IntRect rect(0, 0, viewport_size.Width(), viewport_size.Height());
   // We don't use absoluteBoundingBoxRect() because it can return an IntRect
   // larger the actual size by 1px. crbug.com/470503
-  rect.Intersect(GetDocument().View()->ContentsToViewport(
-      RoundedIntRect(GetLayoutObject()->AbsoluteBoundingBoxFloatRect())));
-  return rect;
+  LayoutRect rect(
+      RoundedIntRect(GetLayoutObject()->AbsoluteBoundingBoxFloatRect()));
+  LayoutRect frame_clip_rect =
+      GetDocument().View()->GetLayoutBox()->ClippingRect(LayoutPoint());
+  rect.Intersect(frame_clip_rect);
+
+  // MapToVisualRectInAncestorSpace, called with a null ancestor argument,
+  // returns the viewport-visible rect in the local frame root's coordinates,
+  // accounting for clips and transformed in embedding containers. This
+  // includes clips that might be applied by out-of-process frame ancestors.
+  GetDocument().View()->GetLayoutView()->MapToVisualRectInAncestorSpace(
+      nullptr, rect, kUseTransforms | kTraverseDocumentBoundaries,
+      kDefaultVisualRectFlags);
+
+  IntRect visible_rect = PixelSnappedIntRect(rect);
+  // If the rect is in the coordinates of the main frame, then it should
+  // also be clipped to the viewport to account for page scale. For OOPIFs,
+  // local frame root -> viewport coordinate conversion is done in the
+  // browser process.
+  if (GetDocument().GetFrame()->LocalFrameRoot().IsMainFrame()) {
+    IntSize viewport_size = GetDocument().GetPage()->GetVisualViewport().Size();
+    visible_rect =
+        GetDocument().GetPage()->GetVisualViewport().RootFrameToViewport(
+            visible_rect);
+    visible_rect.Intersect(IntRect(IntPoint(), viewport_size));
+  }
+  return visible_rect;
 }
 
 void Element::ClientQuads(Vector<FloatQuad>& quads) {

third_party/blink/renderer/core/dom/element.h

@@ -281,9 +281,12 @@ class CORE_EXPORT Element : public ContainerNode {
   virtual void scrollTo(const ScrollToOptions&);
 
   IntRect BoundsInViewport() const;
-  // Returns an intersection rectangle of the bounds rectangle and the
-  // viewport rectangle, in the visual viewport coordinate. This function is
-  // used to show popups beside this element.
+  // For elements that are not contained in any OOPIFs, this returns an
+  // intersection rectangle of the bounds rectangle and the viewport
+  // rectangle, in the visual viewport coordinate. For elements within OOPIFs,
+  // the returned rect is the intersection with the viewport but in the
+  // coordinate space of the local frame root.
+  // This function is used to show popups beside this element.
   IntRect VisibleBoundsInVisualViewport() const;
 
   DOMRectList* getClientRects();

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

@@ -1598,6 +1598,10 @@ TEST_P(VisualViewportTest, ElementBoundsInViewportSpaceAccountsForViewport) {
 }
 
 TEST_P(VisualViewportTest, ElementVisibleBoundsInVisualViewport) {
+  // VisibleBoundsInVisualViewport() assumes root layer scrolling is enabled.
+  if (!RuntimeEnabledFeatures::RootLayerScrollingEnabled())
+    return;
+
   InitializeWithAndroidSettings();
   WebView()->Resize(IntSize(640, 1080));
   RegisterMockedHttpURLLoad("viewport-select.html");