diff options
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 |