Update nested OOPIF's screen rects when parent is repositioned in page.

content/browser/site_per_process_browsertest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/site_per_process_browsertest.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 5129 matching lines @@
   void Wait() {
     if (!message_received_) {
       initial_rect_ = gfx::Rect();
       message_loop_runner_->Run();
     }
   }
 
   void Reset() {
     message_received_ = false;
     initial_rect_ = gfx::Rect();
+    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::Bind(&ShowWidgetMessageFilter::OnShowWidgetOnUI, this, route_id,
                    initial_rect));
@@ skipping 62 matching lines @@
   rwhv_child->ProcessMouseEvent(click_event);
 
   filter->Wait();
 
   gfx::Rect popup_rect = filter->last_initial_rect();
 
   EXPECT_EQ(popup_rect.x() - rwhv_root->GetViewBounds().x(), 354);
   EXPECT_EQ(popup_rect.y() - rwhv_root->GetViewBounds().y(), 94);
 }
 
+// Test that clicking a select element in a nested out-of-process iframe creates
+// a popup menu in the correct position, even if the top-level page repositions
+// its out-of-process iframe. This verifies that screen positioning information
+// is propagating down the frame tree correctly.
+#if defined(OS_ANDROID) || defined(OS_MACOSX)
+// Page Popups work differently on Aura than on Android and Mac. This tests
+// only the Aura mechanism.
+#define MAYBE_NestedPagePopupMenuTest DISABLED_NestedPagePopupMenuTest
+#else
+#define MAYBE_NestedPagePopupMenuTest NestedPagePopupMenuTest
+#endif
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
+                       MAYBE_NestedPagePopupMenuTest) {

[nasko, 2016/03/22 13:56:54]

nit: NestedFrames? MultipleNestedFrames? We've been avoiding Page usage in
general.

[kenrb, 2016/03/22 15:40:20]

That's fine. PagePopup is used throughout Blink to refer to these kinds of
popups, but in content PopupMenu is probably better. Done.

+  GURL main_url(embedded_test_server()->GetURL(
+      "/cross_site_iframe_factory.html?a(b(c))"));
+  NavigateToURL(shell(), main_url);
+
+  FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
+                            ->GetFrameTree()
+                            ->root();
+
+  // Position main window to ensure consistent screen coordinates.
+  RenderWidgetHostViewBase* rwhv_root = static_cast<RenderWidgetHostViewBase*>(
+      root->current_frame_host()->GetRenderWidgetHost()->GetView());
+  rwhv_root->SetBounds(gfx::Rect(100, 100, 500, 500));
+  static_cast<WebContentsImpl*>(shell()->web_contents())->SendScreenRects();
+
+  // For clarity, we are labeling the frame tree nodes as:
+  //  - root_node
+  //   \-> b_node (out-of-process)

[nasko, 2016/03/22 13:56:54]

nit: "out-of-process from root and c_node"? It will make it clearer also on the
next line that each of them are out of process from each other.

[kenrb, 2016/03/22 15:40:20]

Done.

+  //     \-> c_node (out-of-process)
+
+  content::TestNavigationObserver navigation_observer(shell()->web_contents());
+  FrameTreeNode* b_node = root->child_at(0);
+  FrameTreeNode* c_node = b_node->child_at(0);
+  GURL site_url(embedded_test_server()->GetURL(
+      "baz.com", "/site_isolation/page-with-select.html"));
+  NavigateFrameToURL(c_node, site_url);
+  navigation_observer.Wait();

[nasko, 2016/03/22 13:56:54]

No need to wait any longer. I've landed the CL to make it wait for document
load, so this observer can be removed.

[kenrb, 2016/03/22 15:40:20]

Done.

+
+  RenderWidgetHostViewBase* rwhv_c_node =
+      static_cast<RenderWidgetHostViewBase*>(
+          c_node->current_frame_host()->GetRenderWidgetHost()->GetView());
+
+  EXPECT_NE(shell()->web_contents()->GetSiteInstance(),
+            c_node->current_frame_host()->GetSiteInstance());
+
+  scoped_refptr<ShowWidgetMessageFilter> filter = new ShowWidgetMessageFilter();
+  c_node->current_frame_host()->GetProcess()->AddFilter(filter.get());
+
+  // Target left-click event to child frame.
+  blink::WebMouseEvent click_event;
+  click_event.type = blink::WebInputEvent::MouseDown;
+  click_event.button = blink::WebPointerProperties::ButtonLeft;
+  click_event.x = 15;
+  click_event.y = 15;
+  click_event.clickCount = 1;
+  rwhv_c_node->ProcessMouseEvent(click_event);
+
+  filter->Wait();
+
+  gfx::Rect popup_rect = filter->last_initial_rect();
+
+  EXPECT_EQ(popup_rect.x() - rwhv_root->GetViewBounds().x(), 354);
+  EXPECT_EQ(popup_rect.y() - rwhv_root->GetViewBounds().y(), 154);
+
+  // Prompt the WebContents to dismiss the popup by clicking elsewhere.
+  click_event.button = blink::WebPointerProperties::ButtonLeft;
+  click_event.x = 1;
+  click_event.y = 1;
+  click_event.clickCount = 1;
+  rwhv_c_node->ProcessMouseEvent(click_event);
+
+  // Save the screen rect for b_node. Since it updates asynchronously from
+  // the script command that changes it, we need to wait for it to change
+  // before attempting to create the popup widget again.
+  gfx::Rect last_b_node_bounds_rect =
+      b_node->current_frame_host()->GetView()->GetViewBounds();
+
+  std::string script =
+      "var iframe = document.querySelector('iframe');"
+      "iframe.style.position = 'absolute';"
+      "iframe.style.left = 150;"
+      "iframe.style.top = 150;";
+  EXPECT_TRUE(ExecuteScript(root->current_frame_host(), script));
+
+  filter->Reset();
+
+  // Busy loop to wait for b_node's screen rect to get updated. There
+  // doesn't seem to be any better way to find out when this happens.

[nasko, 2016/03/22 13:56:54]

Can't be done with another message filter?

[kenrb, 2016/03/22 15:40:20]

Not easily. We need to wait until after the message has been processed and
passed on, so hooking the incoming message will execute too soon.

[nasko, 2016/03/22 16:18:00]

Acknowledged.

+  while (last_b_node_bounds_rect.x() ==
+             b_node->current_frame_host()->GetView()->GetViewBounds().x() &&
+         last_b_node_bounds_rect.y() ==
+             b_node->current_frame_host()->GetView()->GetViewBounds().y()) {
+    base::RunLoop run_loop;
+    base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+        FROM_HERE, run_loop.QuitClosure(), TestTimeouts::tiny_timeout());
+    run_loop.Run();
+  }
+
+  click_event.button = blink::WebPointerProperties::ButtonLeft;
+  click_event.x = 15;
+  click_event.y = 15;
+  click_event.clickCount = 1;
+  rwhv_c_node->ProcessMouseEvent(click_event);
+
+  filter->Wait();
+
+  popup_rect = filter->last_initial_rect();

[nasko, 2016/03/22 13:56:54]

minor nit: if the rect can be updated, why call it "initial"?

[kenrb, 2016/03/22 15:40:20]

It's the name of the IPC parameter whose value we are verifying. I felt it made
it clearer to keep with consistent naming.

[nasko, 2016/03/22 16:18:00]

Acknowledged.

+
+  EXPECT_EQ(popup_rect.x() - rwhv_root->GetViewBounds().x(), 203);
+  EXPECT_EQ(popup_rect.y() - rwhv_root->GetViewBounds().y(), 248);
+}
+
 // Test for https://crbug.com/526304, where a parent frame executes a
 // remote-to-local navigation on a child frame and immediately removes the same
 // child frame.  This test exercises the path where the detach happens before
 // the provisional local frame is created.
 IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
                        NavigateProxyAndDetachBeforeProvisionalFrameCreation) {
   GURL main_url(embedded_test_server()->GetURL(
       "a.com", "/cross_site_iframe_factory.html?a(b,b)"));
   EXPECT_TRUE(NavigateToURL(shell(), main_url));
 
@@ skipping 895 matching lines @@
   EXPECT_EQ(b_url, root->current_url());
 
   // Verify that the same RenderViewHost is preserved and that it is no longer
   // in swapped out state.
   EXPECT_EQ(rvh, contents->GetFrameTree()->GetRenderViewHost(
                      root->current_frame_host()->GetSiteInstance()));
   EXPECT_FALSE(rvh->is_swapped_out_);
 }
 
 }  // namespace content