Commit c2a8ee53 authored by Shinya Kawanaka's avatar Shinya Kawanaka Committed by Commit Bot

Revert "AppShell: Suppress context menu in webviews"

This reverts commit 52e03fa8.

Reason for revert: Some mac builders are broken. e.g. https://ci.chromium.org/buildbot/chromium.fyi/Mac%20Builder%20%28dbg%29%20Goma%20Canary/3941

Original change's description:
> 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}

TBR=michaelpg@chromium.org,lfg@chromium.org

Change-Id: Ib3691d13fb6a787b487c0e11a73e9c0bdaad5b8b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 820604
Reviewed-on: https://chromium-review.googlesource.com/969723Reviewed-by: default avatarShinya Kawanaka <shinyak@chromium.org>
Commit-Queue: Shinya Kawanaka <shinyak@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544273}
parent 73f6eaf5
......@@ -15,6 +15,7 @@
#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"
......@@ -106,4 +107,11 @@ 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
......@@ -27,6 +27,7 @@ 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_; }
......
......@@ -10,7 +10,6 @@
#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"
......@@ -22,7 +21,6 @@
#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"
......@@ -45,7 +43,6 @@
#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;
......@@ -474,30 +471,6 @@ 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);
......
......@@ -20,6 +20,10 @@ 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
......
......@@ -174,8 +174,6 @@ 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",
......
......@@ -17,8 +17,7 @@ ShellAppViewGuestDelegate::~ShellAppViewGuestDelegate() {
bool ShellAppViewGuestDelegate::HandleContextMenu(
content::WebContents* web_contents,
const content::ContextMenuParams& params) {
// Eat the context menu request, as AppShell doesn't show context menus.
return true;
return false;
}
AppDelegate* ShellAppViewGuestDelegate::CreateAppDelegate() {
......
......@@ -14,7 +14,6 @@
#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"
......@@ -36,11 +35,6 @@ 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 {
......
......@@ -26,8 +26,6 @@ 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)
......
// 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
// 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_
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment