AppShell: Suppress context menu in webviews

AppShell doesn't support context menus, but still needs to provide a
WebViewGuestDelegate to handle the context menu. Otherwise, we reach a
NOTREACHED() in WebContentsViewChildFrame::ShowContextMenu().

Also removes a WebViewGuestDelegate function that is unused (see
https://codereview.chromium.org/2836973002).

Bug: 820604
Change-Id: I0760d397384908a490704f23388e11c4eef113a5
Reviewed-on: https://chromium-review.googlesource.com/957596
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Lucas Gadani <lfg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544136}

chrome/browser/guest_view/web_view/chrome_web_view_guest_delegate.cc

@@ -15,7 +15,6 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/renderer_context_menu/render_view_context_menu.h"
 #include "chrome/browser/ui/pdf/chrome_pdf_web_contents_helper_client.h"
-#include "chrome/common/url_constants.h"
 #include "components/guest_view/browser/guest_view_event.h"
 #include "components/renderer_context_menu/context_menu_delegate.h"
 #include "content/public/browser/render_process_host.h"
@@ -107,11 +106,4 @@ void ChromeWebViewGuestDelegate::OnShowContextMenu(int request_id) {
   menu_delegate->ShowMenu(std::move(pending_menu_));
 }
 
-bool ChromeWebViewGuestDelegate::ShouldHandleFindRequestsForEmbedder() const {
-  // Find requests will be handled by the guest for the Chrome signin page.
-  return web_view_guest_->owner_web_contents()->GetWebUI() != nullptr &&
-         web_view_guest_->GetOwnerSiteURL().GetOrigin().spec() ==
-             chrome::kChromeUIChromeSigninURL;
-}
-
 }  // namespace extensions

chrome/browser/guest_view/web_view/chrome_web_view_guest_delegate.h

@@ -27,7 +27,6 @@ class ChromeWebViewGuestDelegate : public WebViewGuestDelegate {
   // WebViewGuestDelegate implementation.
   bool HandleContextMenu(const content::ContextMenuParams& params) override;
   void OnShowContextMenu(int request_id) override;
-  bool ShouldHandleFindRequestsForEmbedder() const override;
 
   WebViewGuest* web_view_guest() const { return web_view_guest_; }
 

extensions/browser/guest_view/web_view/web_view_apitest.cc

@@ -10,6 +10,7 @@
 
 #include "base/command_line.h"
 #include "base/macros.h"
+#include "base/memory/scoped_refptr.h"
 #include "base/path_service.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
@@ -21,6 +22,7 @@
 #include "components/guest_view/browser/test_guest_view_manager.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
+#include "content/public/browser/render_view_host.h"
 #include "content/public/common/content_features.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/browser_test_utils.h"
@@ -43,6 +45,7 @@
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/test/embedded_test_server/http_request.h"
 #include "net/test/embedded_test_server/http_response.h"
+#include "third_party/WebKit/public/platform/WebMouseEvent.h"
 #include "ui/display/display_switches.h"
 
 using guest_view::GuestViewManager;
@@ -471,6 +474,30 @@ IN_PROC_BROWSER_TEST_F(WebViewAPITest, TestContentLoadEvent) {
   RunTest("testContentLoadEvent", "web_view/apitest");
 }
 
+// Verifies that trying to show the context menu doesn't crash
+// (https://crbug.com/820604).
+IN_PROC_BROWSER_TEST_F(WebViewAPITest, TestContextMenu) {
+  // Launch some test app that displays a webview.
+  LaunchApp("web_view/visibility_changed");
+
+  // Ensure the webview's surface is ready for hit testing.
+  content::WebContents* guest_web_contents = GetGuestWebContents();
+  content::WaitForGuestSurfaceReady(guest_web_contents);
+
+  // Register a ContextMenuFilter to wait for the context menu event to be sent.
+  content::RenderProcessHost* guest_process_host =
+      guest_web_contents->GetMainFrame()->GetProcess();
+  auto context_menu_filter = base::MakeRefCounted<content::ContextMenuFilter>();
+  guest_process_host->AddFilter(context_menu_filter.get());
+
+  // Trigger the context menu. AppShell doesn't show a context menu; this is
+  // just a sanity check that nothing breaks.
+  content::SimulateRoutedMouseClickAt(
+      guest_web_contents, blink::WebInputEvent::kNoModifiers,
+      blink::WebMouseEvent::Button::kRight, gfx::Point(10, 10));
+  context_menu_filter->Wait();
+}
+
 IN_PROC_BROWSER_TEST_F(WebViewAPITest, TestDeclarativeWebRequestAPI) {
   std::string app_location = "web_view/apitest";
   StartTestServer(app_location);

extensions/browser/guest_view/web_view/web_view_guest_delegate.h

@@ -20,10 +20,6 @@ class WebViewGuestDelegate {
 
   // Shows the context menu for the guest.
   virtual void OnShowContextMenu(int request_id) = 0;
-
-  // Returns true if the WebViewGuest should handle find requests for its
-  // embedder.
-  virtual bool ShouldHandleFindRequestsForEmbedder() const = 0;
 };
 
 }  // namespace extensions

extensions/shell/BUILD.gn

@@ -174,6 +174,8 @@ source_set("app_shell_lib") {
     "browser/shell_url_request_context_getter.h",
     "browser/shell_virtual_keyboard_delegate.cc",
     "browser/shell_virtual_keyboard_delegate.h",
+    "browser/shell_web_view_guest_delegate.cc",
+    "browser/shell_web_view_guest_delegate.h",
     "common/shell_content_client.cc",
     "common/shell_content_client.h",
     "common/shell_extensions_client.cc",

extensions/shell/browser/shell_app_view_guest_delegate.cc

@@ -17,7 +17,8 @@ ShellAppViewGuestDelegate::~ShellAppViewGuestDelegate() {
 bool ShellAppViewGuestDelegate::HandleContextMenu(
     content::WebContents* web_contents,
     const content::ContextMenuParams& params) {
-  return false;
+  // Eat the context menu request, as AppShell doesn't show context menus.
+  return true;
 }
 
 AppDelegate* ShellAppViewGuestDelegate::CreateAppDelegate() {

extensions/shell/browser/shell_extensions_api_client.cc

@@ -14,6 +14,7 @@
 #include "extensions/shell/browser/shell_app_view_guest_delegate.h"
 #include "extensions/shell/browser/shell_extension_web_contents_observer.h"
 #include "extensions/shell/browser/shell_virtual_keyboard_delegate.h"
+#include "extensions/shell/browser/shell_web_view_guest_delegate.h"
 
 #if defined(OS_LINUX) && !defined(OS_CHROMEOS)
 #include "extensions/shell/browser/api/file_system/shell_file_system_delegate.h"
@@ -35,6 +36,11 @@ AppViewGuestDelegate* ShellExtensionsAPIClient::CreateAppViewGuestDelegate()
   return new ShellAppViewGuestDelegate();
 }
 
+WebViewGuestDelegate* ShellExtensionsAPIClient::CreateWebViewGuestDelegate(
+    WebViewGuest* web_view_guest) const {
+  return new ShellWebViewGuestDelegate();
+}
+
 std::unique_ptr<VirtualKeyboardDelegate>
 ShellExtensionsAPIClient::CreateVirtualKeyboardDelegate(
     content::BrowserContext* browser_context) const {

extensions/shell/browser/shell_extensions_api_client.h

@@ -26,6 +26,8 @@ class ShellExtensionsAPIClient : public ExtensionsAPIClient {
   void AttachWebContentsHelpers(content::WebContents* web_contents) const
       override;
   AppViewGuestDelegate* CreateAppViewGuestDelegate() const override;
+  WebViewGuestDelegate* CreateWebViewGuestDelegate(
+      WebViewGuest* web_view_guest) const override;
   std::unique_ptr<VirtualKeyboardDelegate> CreateVirtualKeyboardDelegate(
       content::BrowserContext* browser_context) const override;
 #if defined(OS_LINUX) && !defined(OS_CHROMEOS)

extensions/shell/browser/shell_web_view_guest_delegate.cc

+// Copyright 2018 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 "extensions/shell/browser/shell_web_view_guest_delegate.h"
+
+namespace extensions {
+
+ShellWebViewGuestDelegate::ShellWebViewGuestDelegate() = default;
+ShellWebViewGuestDelegate::~ShellWebViewGuestDelegate() = default;
+
+bool ShellWebViewGuestDelegate::HandleContextMenu(
+    const content::ContextMenuParams& params) {
+  // Eat the context menu request, as AppShell doesn't show context menus.
+  return true;
+}
+
+void ShellWebViewGuestDelegate::OnShowContextMenu(int request_id) {}
+
+}  // namespace extensions

extensions/shell/browser/shell_web_view_guest_delegate.h

+// Copyright 2018 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.
+
+#ifndef EXTENSIONS_SHELL_BROWSER_SHELL_WEB_VIEW_GUEST_DELEGATE_H_
+#define EXTENSIONS_SHELL_BROWSER_SHELL_WEB_VIEW_GUEST_DELEGATE_H_
+
+#include "base/macros.h"
+#include "extensions/browser/guest_view/web_view/web_view_guest_delegate.h"
+
+namespace extensions {
+
+class ShellWebViewGuestDelegate : public WebViewGuestDelegate {
+ public:
+  ShellWebViewGuestDelegate();
+  ~ShellWebViewGuestDelegate() override;
+
+  // WebViewGuestDelegate:
+  bool HandleContextMenu(const content::ContextMenuParams& params) override;
+  void OnShowContextMenu(int request_id) override;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ShellWebViewGuestDelegate);
+};
+
+}  // namespace extensions
+
+#endif  // EXTENSIONS_SHELL_BROWSER_SHELL_WEB_VIEW_GUEST_DELEGATE_H_