Fixing the Position of Context Menu for BrowserPlugin (<webview>)

With current implementation of the BrowserPlugin, all WebMouseEvents are
routed, throught the browser (RenderWidgetHostViewGuest), to the
renderer process of the guest. The guest uses the coordinates, as passed, for creating the ContextMenu event.
Therefore, the guest renderer process is oblivious to any CSS transforms
applied on the BrowserPlugin and as a result, the reported position of
the context menu is potentially incorrect.

This patch applies a hacky solution to resolve this issue. The work
around is to store the location of the context menu from a MouseDown
event. The position is measured form the global coordinates of the mouse
event and the boundaries of the owner's view.

BUG=470087

Review URL: https://codereview.chromium.org/1293963002

Cr-Commit-Position: refs/heads/master@{#347195}

16 files changed, 206 insertions, 5 deletions

diff --git a/chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc b/chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc
index ac03c12..64dbd62 100644
--- a/chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc
+++ b/chrome/browser/apps/guest_view/web_view_interactive_browsertest.cc
@@ -287,7 +287,10 @@ class WebViewInteractiveTest
     mouse_event.x = mouse_event.windowX = x;
     mouse_event.y = mouse_event.windowY = y;
     mouse_event.modifiers = 0;
-
+    // Needed for the WebViewTest.ContextMenuPositionAfterCSSTransforms
+    gfx::Rect rect = rwh->GetView()->GetViewBounds();
+    mouse_event.globalX = x + rect.x();
+    mouse_event.globalY = y + rect.y();
     mouse_event.type = blink::WebInputEvent::MouseDown;
     rwh->ForwardMouseEvent(mouse_event);
     mouse_event.type = blink::WebInputEvent::MouseUp;
@@ -849,6 +852,55 @@ IN_PROC_BROWSER_TEST_F(WebViewInteractiveTest, ContextMenuParamCoordinates) {
   ASSERT_EQ(20, menu_observer.params().y);
 }
 
+// Tests whether <webview> context menu sees <webview> local coordinates in its
+// RenderViewContextMenu params, when it is subject to CSS transforms.
+IN_PROC_BROWSER_TEST_F(WebViewInteractiveTest,
+                       ContextMenuParamsAfterCSSTransforms) {
+  LoadAndLaunchPlatformApp("web_view/context_menus/coordinates_with_transforms",
+                           "Launched");
+
+  if (!embedder_web_contents_)
+    embedder_web_contents_ = GetFirstAppWindowWebContents();
+  EXPECT_TRUE(embedder_web_contents());
+
+  if (!guest_web_contents_)
+    guest_web_contents_ = GetGuestViewManager()->WaitForSingleGuestCreated();
+  EXPECT_TRUE(guest_web_contents());
+
+  // We will send the input event to the embedder rather than the guest; which
+  // is more realistic. We need to do this to make sure that the MouseDown event
+  // is received forwarded by the BrowserPlugin to the RWHVG and eventually back
+  // to the guest. The RWHVG will in turn notify the ChromeWVGDelegate of the
+  // newly observed mouse down (potentially a context menu).
+  const std::string transforms[] = {"rotate(20deg)", "scale(1.5, 2.0)",
+                                    "translate(20px, 30px)", "NONE"};
+  for (size_t index = 0; index < 4; ++index) {
+    std::string command =
+        base::StringPrintf("setTransform('%s')", transforms[index].c_str());
+    ExtensionTestMessageListener transform_set_listener("TRANSFORM_SET", false);
+    EXPECT_TRUE(content::ExecuteScript(embedder_web_contents(), command));
+    ASSERT_TRUE(transform_set_listener.WaitUntilSatisfied());
+
+    gfx::Rect embedder_view_bounds =
+        embedder_web_contents()->GetRenderWidgetHostView()->GetViewBounds();
+    gfx::Rect guest_view_bounds =
+        guest_web_contents()->GetRenderWidgetHostView()->GetViewBounds();
+    ContextMenuWaiter menu_observer(content::NotificationService::AllSources());
+    gfx::Point guest_window_point(150, 150);
+    gfx::Point embedder_window_point = guest_window_point;
+    embedder_window_point += guest_view_bounds.OffsetFromOrigin();
+    embedder_window_point -= embedder_view_bounds.OffsetFromOrigin();
+    SimulateRWHMouseClick(embedder_web_contents()->GetRenderViewHost(),
+                          blink::WebMouseEvent::ButtonRight,
+                          /* Using window coordinates for the embedder */
+                          embedder_window_point.x(), embedder_window_point.y());
+
+    menu_observer.WaitForMenuOpenAndClose();
+    EXPECT_EQ(menu_observer.params().x, guest_window_point.x());
+    EXPECT_EQ(menu_observer.params().y, guest_window_point.y());
+  }
+}
+
 IN_PROC_BROWSER_TEST_F(WebViewInteractiveTest, ExecuteCode) {
   ASSERT_TRUE(RunPlatformAppTestWithArg(
       "platform_apps/web_view/common", "execute_code")) << message_;
diff --git a/chrome/browser/guest_view/web_view/chrome_web_view_guest_delegate.cc b/chrome/browser/guest_view/web_view/chrome_web_view_guest_delegate.cc
index c6d910b..7b1dda5 100644
--- a/chrome/browser/guest_view/web_view/chrome_web_view_guest_delegate.cc
+++ b/chrome/browser/guest_view/web_view/chrome_web_view_guest_delegate.cc
@@ -39,7 +39,18 @@ bool ChromeWebViewGuestDelegate::HandleContextMenu(
       ContextMenuDelegate::FromWebContents(guest_web_contents());
   DCHECK(menu_delegate);
 
-  pending_menu_ = menu_delegate->BuildMenu(guest_web_contents(), params);
+  content::ContextMenuParams new_params = params;
+  // The only case where |context_menu_position_| is not initialized is the case
+  // where the input event is directly sent to the guest WebContents without
+  // ever going throught the embedder and BrowserPlugin's
+  // RenderWidgetHostViewGuest. This only happens in some tests, e.g.,
+  // WebViewInteractiveTest.ContextMenuParamCoordinates.
+  if (context_menu_position_) {
+    new_params.x = context_menu_position_->x();
+    new_params.y = context_menu_position_->y();
+  }
+
+  pending_menu_ = menu_delegate->BuildMenu(guest_web_contents(), new_params);
   // It's possible for the returned menu to be null, so early out to avoid
   // a crash. TODO(wjmaclean): find out why it's possible for this to happen
   // in the first place, and if it's an error.
@@ -139,4 +150,12 @@ void ChromeWebViewGuestDelegate::OnAccessibilityStatusChanged(
 }
 #endif
 
+void ChromeWebViewGuestDelegate::SetContextMenuPosition(
+    const gfx::Point& position) {
+  if (context_menu_position_ == nullptr)
+    context_menu_position_.reset(new gfx::Point());
+
+  *context_menu_position_ = position;
+}
+
 }  // namespace extensions
diff --git a/chrome/browser/guest_view/web_view/chrome_web_view_guest_delegate.h b/chrome/browser/guest_view/web_view/chrome_web_view_guest_delegate.h
index 7b4c145..d6101e7 100644
--- a/chrome/browser/guest_view/web_view/chrome_web_view_guest_delegate.h
+++ b/chrome/browser/guest_view/web_view/chrome_web_view_guest_delegate.h
@@ -39,6 +39,8 @@ class ChromeWebViewGuestDelegate : public WebViewGuestDelegate {
     return web_view_guest()->web_contents();
   }
 
+  void SetContextMenuPosition(const gfx::Point& position) override;
+
   // Returns the top level items (ignoring submenus) as Value.
   static scoped_ptr<base::ListValue> MenuModelToValue(
       const ui::SimpleMenuModel& menu_model);
@@ -70,6 +72,8 @@ class ChromeWebViewGuestDelegate : public WebViewGuestDelegate {
 
   WebViewGuest* const web_view_guest_;
 
+  scoped_ptr<gfx::Point> context_menu_position_;
+
   // This is used to ensure pending tasks will not fire after this object is
   // destroyed.
   base::WeakPtrFactory<ChromeWebViewGuestDelegate> weak_ptr_factory_;
diff --git a/chrome/test/data/extensions/platform_apps/web_view/context_menus/coordinates_with_transforms/embedder.html b/chrome/test/data/extensions/platform_apps/web_view/context_menus/coordinates_with_transforms/embedder.html
new file mode 100644
index 0000000..904e496
--- /dev/null
+++ b/chrome/test/data/extensions/platform_apps/web_view/context_menus/coordinates_with_transforms/embedder.html
@@ -0,0 +1,11 @@
+<!--
+ * Copyright 2015 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.
+-->
+<html>
+<body>
+  <div id="webview-tag-container" style="width: 300px; heigth: 300px;"></div>
+  <script src="embedder.js"></script>
+</body>
+</html>
diff --git a/chrome/test/data/extensions/platform_apps/web_view/context_menus/coordinates_with_transforms/embedder.js b/chrome/test/data/extensions/platform_apps/web_view/context_menus/coordinates_with_transforms/embedder.js
new file mode 100644
index 0000000..ce62b9e
--- /dev/null
+++ b/chrome/test/data/extensions/platform_apps/web_view/context_menus/coordinates_with_transforms/embedder.js
@@ -0,0 +1,39 @@
+// Copyright 2015 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.
+
+var LOG = function(msg) { window.console.log(msg); };
+
+// window.* exported functions begin.
+window.setTransform = function(tr) {
+  LOG('Setting transform to "' + tr + '".');
+  var webview = document.body.querySelector('webview');
+  webview.style.transform = (tr !== 'NONE') ? tr : '';
+  webview.onloadstop = function() {
+    LOG('Transform "' + tr + '" is set.');
+    chrome.test.sendMessage('TRANSFORM_SET');
+  };
+  webview.reload();
+};
+// window.* exported functions end.
+
+function setUpTest(messageCallback) {
+  var guestUrl = 'data:text/html,<html><body>guest</body></html>';
+  var webview = document.createElement('webview');
+  var onLoadStop = function(e) {
+    LOG('webview has loaded.');
+    messageCallback(webview);
+  };
+  webview.addEventListener('loadstop', onLoadStop);
+
+  webview.setAttribute('src', guestUrl);
+  document.body.appendChild(webview);
+}
+
+onload = function() {
+  chrome.test.getConfig(function(config) {
+    setUpTest(function(webview) {
+      chrome.test.sendMessage('Launched');
+    });
+  });
+};
diff --git a/chrome/test/data/extensions/platform_apps/web_view/context_menus/coordinates_with_transforms/manifest.json b/chrome/test/data/extensions/platform_apps/web_view/context_menus/coordinates_with_transforms/manifest.json
new file mode 100644
index 0000000..3a78588
--- /dev/null
+++ b/chrome/test/data/extensions/platform_apps/web_view/context_menus/coordinates_with_transforms/manifest.json
@@ -0,0 +1,13 @@
+{
+  "name": "Packaged App Test: <webview> context menu param coordinates",
+  "description": "Helper app to verify <webview> context menu param's coordinates",
+  "version": "1",
+  "permissions": [
+    "webview"
+  ],
+  "app": {
+    "background": {
+      "scripts": ["test.js"]
+    }
+  }
+}
diff --git a/chrome/test/data/extensions/platform_apps/web_view/context_menus/coordinates_with_transforms/test.js b/chrome/test/data/extensions/platform_apps/web_view/context_menus/coordinates_with_transforms/test.js
new file mode 100644
index 0000000..f7d6ae1
--- /dev/null
+++ b/chrome/test/data/extensions/platform_apps/web_view/context_menus/coordinates_with_transforms/test.js
@@ -0,0 +1,12 @@
+// Copyright 2015 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.
+
+chrome.app.runtime.onLaunched.addListener(function() {
+  chrome.app.window.create('embedder.html', {
+    outerBounds: {
+      width: 500,
+      height: 500
+    }
+  }, function () {});
+});
diff --git a/components/guest_view/browser/guest_view_base.cc b/components/guest_view/browser/guest_view_base.cc
index 45c872b..d720355 100644
--- a/components/guest_view/browser/guest_view_base.cc
+++ b/components/guest_view/browser/guest_view_base.cc
@@ -368,6 +368,8 @@ bool GuestViewBase::ZoomPropagatesFromEmbedderToGuest() const {
   return true;
 }
 
+void GuestViewBase::SetContextMenuPosition(const gfx::Point& position) {}
+
 WebContents* GuestViewBase::CreateNewGuestWindow(
     const WebContents::CreateParams& create_params) {
   auto guest_manager = GuestViewManager::FromBrowserContext(browser_context());
diff --git a/components/guest_view/browser/guest_view_base.h b/components/guest_view/browser/guest_view_base.h
index 3ea4265..6c78645 100644
--- a/components/guest_view/browser/guest_view_base.h
+++ b/components/guest_view/browser/guest_view_base.h
@@ -271,7 +271,6 @@ class GuestViewBase : public content::BrowserPluginGuestDelegate,
   void SetAttachParams(const base::DictionaryValue& params);
   void SetOpener(GuestViewBase* opener);
 
-  // BrowserPluginGuestDelegate implementation.
   content::WebContents* CreateNewGuestWindow(
       const content::WebContents::CreateParams& create_params) final;
   void DidAttach(int guest_proxy_routing_id) final;
@@ -369,6 +368,9 @@ class GuestViewBase : public content::BrowserPluginGuestDelegate,
   // entirety of the embedder's content.
   virtual bool ShouldHandleFindRequestsForEmbedder() const;
 
+  // BrowserPluginGuestDelegate implementation.
+  void SetContextMenuPosition(const gfx::Point& position) override;
+
  private:
   class OwnerContentsObserver;
 
diff --git a/content/browser/browser_plugin/browser_plugin_guest.cc b/content/browser/browser_plugin/browser_plugin_guest.cc
index 6c8cfd7..7942c4f 100644
--- a/content/browser/browser_plugin/browser_plugin_guest.cc
+++ b/content/browser/browser_plugin/browser_plugin_guest.cc
@@ -1001,4 +1001,9 @@ void BrowserPluginGuest::OnImeCompositionRangeChanged(
 }
 #endif
 
+void BrowserPluginGuest::SetContextMenuPosition(const gfx::Point& position) {
+  if (delegate_)
+    delegate_->SetContextMenuPosition(position);
+}
+
 }  // namespace content
diff --git a/content/browser/browser_plugin/browser_plugin_guest.h b/content/browser/browser_plugin/browser_plugin_guest.h
index 1d975f4..bb48799 100644
--- a/content/browser/browser_plugin/browser_plugin_guest.h
+++ b/content/browser/browser_plugin/browser_plugin_guest.h
@@ -187,6 +187,17 @@ class CONTENT_EXPORT BrowserPluginGuest : public GuestHost,
 
   gfx::Point GetScreenCoordinates(const gfx::Point& relative_position) const;
 
+  // This method is called by the RenderWidgetHostViewGuest to inform the
+  // BrowserPlugin of the potential location of the context menu event (to
+  // come). The need for this (hack) is that the input events when passed on to
+  // the BrowserPlugin are modified by any CSS transforms applied on the plugin.
+  // Therefore, the coordinates of the context menu event with respect to the
+  // container window are modifed with the guest renderer process beiung unaware
+  // of the change. Then eventually, when the context menu event arrives at the
+  // browser, it contains the wrong coordinates (BUG=470087).
+  // TODO(ekaramad): Find a more fundamental solution and remove this later.
+  void SetContextMenuPosition(const gfx::Point& position);
+
   // Helper to send messages to embedder. If this guest is not yet attached,
   // then IPCs will be queued until attachment.
   void SendMessageToEmbedder(IPC::Message* msg);
diff --git a/content/browser/frame_host/render_widget_host_view_guest.cc b/content/browser/frame_host/render_widget_host_view_guest.cc
index 9c4503e..3de9530 100644
--- a/content/browser/frame_host/render_widget_host_view_guest.cc
+++ b/content/browser/frame_host/render_widget_host_view_guest.cc
@@ -637,8 +637,24 @@ void RenderWidgetHostViewGuest::OnHandleInputEvent(
     const gfx::Rect& guest_window_rect,
     const blink::WebInputEvent* event) {
   if (blink::WebInputEvent::isMouseEventType(event->type)) {
-    host_->ForwardMouseEvent(
-        *static_cast<const blink::WebMouseEvent*>(event));
+    // The mouse events for BrowserPlugin are modified by all
+    // the CSS transforms applied on the <object> and embedder. As a result of
+    // this, the coordinates passed on to the guest renderer are potentially
+    // incorrect to determine the position of the context menu(they are not the
+    // actual X, Y of the window). As a hack, we report the last location of a
+    // right mouse up to the BrowserPluginGuest to inform it of the next
+    // potential location for context menu (BUG=470087).
+    // TODO(ekaramad): Find a better and more fundamental solution. Could the
+    // ContextMenuParams be based on global X, Y?
+    const blink::WebMouseEvent& mouse_event =
+        static_cast<const blink::WebMouseEvent&>(*event);
+    // A MouseDown on the ButtonRight could suggest a ContextMenu.
+    if (guest_ && mouse_event.type == blink::WebInputEvent::MouseDown &&
+        mouse_event.button == blink::WebPointerProperties::ButtonRight)
+      guest_->SetContextMenuPosition(
+          gfx::Point(mouse_event.globalX - GetViewBounds().x(),
+                     mouse_event.globalY - GetViewBounds().y()));
+    host_->ForwardMouseEvent(mouse_event);
     return;
   }
 
diff --git a/content/public/browser/browser_plugin_guest_delegate.h b/content/public/browser/browser_plugin_guest_delegate.h
index 000e3b6..aabe27b 100644
--- a/content/public/browser/browser_plugin_guest_delegate.h
+++ b/content/public/browser/browser_plugin_guest_delegate.h
@@ -86,6 +86,12 @@ class CONTENT_EXPORT BrowserPluginGuestDelegate {
   // Provides the delegate with an interface with which to communicate with the
   // content module.
   virtual void SetGuestHost(GuestHost* guest_host) {}
+
+  // Sets the position of the context menu for the guest contents. The value
+  // reported from the guest renderer should be ignored. The reported value
+  // fromt he guest renderer is incorrect in situations where BrowserPlugin is
+  // subject to CSS transforms.
+  virtual void SetContextMenuPosition(const gfx::Point& position) {}
 };
 
 }  // namespace content
diff --git a/extensions/browser/guest_view/web_view/web_view_guest.cc b/extensions/browser/guest_view/web_view/web_view_guest.cc
index 70dca3f..bab38da 100644
--- a/extensions/browser/guest_view/web_view/web_view_guest.cc
+++ b/extensions/browser/guest_view/web_view/web_view_guest.cc
@@ -567,6 +567,11 @@ void WebViewGuest::LoadAbort(bool is_top_level,
       new GuestViewEvent(webview::kEventLoadAbort, args.Pass()));
 }
 
+void WebViewGuest::SetContextMenuPosition(const gfx::Point& position) {
+  if (web_view_guest_delegate_)
+    web_view_guest_delegate_->SetContextMenuPosition(position);
+}
+
 void WebViewGuest::CreateNewGuestWebViewWindow(
     const content::OpenURLParams& params) {
   GuestViewManager* guest_manager =
diff --git a/extensions/browser/guest_view/web_view/web_view_guest.h b/extensions/browser/guest_view/web_view/web_view_guest.h
index 9605ad2..9144c68 100644
--- a/extensions/browser/guest_view/web_view/web_view_guest.h
+++ b/extensions/browser/guest_view/web_view/web_view_guest.h
@@ -354,6 +354,8 @@ class WebViewGuest : public guest_view::GuestView<WebViewGuest>,
 
   void ApplyAttributes(const base::DictionaryValue& params);
 
+  void SetContextMenuPosition(const gfx::Point& position) override;
+
   // Identifies the set of rules registries belonging to this guest.
   int rules_registry_id_;
 
diff --git a/extensions/browser/guest_view/web_view/web_view_guest_delegate.h b/extensions/browser/guest_view/web_view/web_view_guest_delegate.h
index 84a06f5..afee259 100644
--- a/extensions/browser/guest_view/web_view/web_view_guest_delegate.h
+++ b/extensions/browser/guest_view/web_view/web_view_guest_delegate.h
@@ -49,6 +49,8 @@ class WebViewGuestDelegate {
   // Returns true if the WebViewGuest should handle find requests for its
   // embedder.
   virtual bool ShouldHandleFindRequestsForEmbedder() const = 0;
+
+  virtual void SetContextMenuPosition(const gfx::Point& position) = 0;
 };
 
 }  // namespace extensions