Compare commits

..

2 Commits

Author SHA1 Message Date
Marshall Greenblatt 4ed13d8c47 views: Add missing patched files (see #3711) 2024-06-13 18:07:59 -04:00
Marshall Greenblatt fe24ce3c71 chrome: Add cleanup when context menu isn't running (fixes #3711)
The menu may not be running in the following cases:

- If the menu is empty (e.g. cleared in OnBeforeContextMenu).
- If the menu is disabled (see e.g. RenderViewContextMenuViews::Show).
- When the run call blocks until the menu is dismissed (macOS behavior).

We explicitly clean up in these cases instead of waiting for OnMenuClosed
which will otherwise never be called for the first 2 cases.

Menu run status is exposed via new ContextMenuDelegate and
RenderViewContextMenuBase methods.
2024-06-13 13:38:41 -04:00
5 changed files with 243 additions and 13 deletions

View File

@ -152,6 +152,12 @@ class CefContextMenuObserver : public RenderViewContextMenuObserver,
}
void OnMenuClosed() override {
// May be called multiple times. For example, if the menu runs and is
// additionally reset via MaybeResetContextMenu.
if (!handler_) {
return;
}
handler_->OnContextMenuDismissed(browser_, GetFrame());
model_->Detach();
@ -203,17 +209,25 @@ class CefContextMenuObserver : public RenderViewContextMenuObserver,
base::BindOnce(&CefContextMenuObserver::ExecuteCommandCallback,
weak_ptr_factory_.GetWeakPtr())));
bool handled = handler_->RunContextMenu(browser_, GetFrame(), params_,
is_handled_ = handler_->RunContextMenu(browser_, GetFrame(), params_,
model_, callbackImpl.get());
if (!handled && callbackImpl->IsDisconnected()) {
if (!is_handled_ && callbackImpl->IsDisconnected()) {
LOG(ERROR) << "Should return true from RunContextMenu when executing the "
"callback";
handled = true;
is_handled_ = true;
}
if (!handled) {
if (!is_handled_) {
callbackImpl->Disconnect();
}
return handled;
return is_handled_;
}
void MaybeResetContextMenu() {
// Don't reset the menu when the client is using custom handling. It will be
// reset via ExecuteCommandCallback instead.
if (!is_handled_) {
OnMenuClosed();
}
}
private:
@ -282,6 +296,8 @@ class CefContextMenuObserver : public RenderViewContextMenuObserver,
using ItemInfoMap = std::map<int, ItemInfo>;
ItemInfoMap iteminfomap_;
bool is_handled_ = false;
base::WeakPtrFactory<CefContextMenuObserver> weak_ptr_factory_{this};
};
@ -336,4 +352,13 @@ bool HandleContextMenu(content::WebContents* opener,
return false;
}
void MaybeResetContextMenu(content::WebContents* opener) {
auto browser = CefBrowserHostBase::GetBrowserForContents(opener);
if (browser && browser->context_menu_observer()) {
return static_cast<CefContextMenuObserver*>(
browser->context_menu_observer())
->MaybeResetContextMenu();
}
}
} // namespace context_menu

View File

@ -21,6 +21,8 @@ void RegisterCallbacks();
bool HandleContextMenu(content::WebContents* opener,
const content::ContextMenuParams& params);
void MaybeResetContextMenu(content::WebContents* opener);
} // namespace context_menu
#endif // CEF_LIBCEF_BROWSER_CHROME_CHROME_CONTEXT_MENU_HANDLER_H_

View File

@ -19,4 +19,14 @@ void ChromeWebContentsViewDelegateCef::ShowContextMenu(
}
ChromeWebContentsViewDelegateBase::ShowContextMenu(render_frame_host, params);
// The menu may not be running in the following cases:
// - If the menu is empty (e.g. cleared in OnBeforeContextMenu).
// - If the menu is disabled (see e.g. RenderViewContextMenuViews::Show).
// - When the above call blocks until the menu is dismissed (macOS behavior).
// We explicitly clean up in these cases instead of waiting for OnMenuClosed
// which will otherwise never be called for the first 2 cases.
if (!IsMenuRunning()) {
context_menu::MaybeResetContextMenu(web_contents_);
}
}

View File

@ -8,6 +8,8 @@
#include "cef/libcef/browser/browser_host_base.h"
#include "cef/libcef/browser/chrome/chrome_context_menu_handler.h"
#include "cef/libcef/browser/osr/web_contents_view_osr.h"
#include "components/renderer_context_menu/context_menu_delegate.h"
#include "content/public/browser/web_contents.h"
#include "extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.h"
namespace extensions {
@ -42,8 +44,27 @@ bool ChromeMimeHandlerViewGuestDelegateCef::HandleContextMenu(
return true;
}
return ChromeMimeHandlerViewGuestDelegate::HandleContextMenu(
render_frame_host, params);
[[maybe_unused]] bool result =
ChromeMimeHandlerViewGuestDelegate::HandleContextMenu(render_frame_host,
params);
DCHECK(result);
content::WebContents* web_contents =
content::WebContents::FromRenderFrameHost(&render_frame_host);
ContextMenuDelegate* menu_delegate =
ContextMenuDelegate::FromWebContents(web_contents);
// The menu may not be running in the following cases:
// - If the menu is empty (e.g. cleared in OnBeforeContextMenu).
// - If the menu is disabled (see e.g. RenderViewContextMenuViews::Show).
// - When the above call blocks until the menu is dismissed (macOS behavior).
// We explicitly clean up in these cases instead of waiting for OnMenuClosed
// which will otherwise never be called for the first 2 cases.
if (!menu_delegate->IsMenuRunning()) {
context_menu::MaybeResetContextMenu(owner_web_contents_);
}
return true;
}
} // namespace extensions

View File

@ -123,8 +123,20 @@ index 6c59d4ccaf3d5..21c959aea9c21 100644
// An observer that handles spelling suggestions, "Add to dictionary", and
// "Use enhanced spell check" items.
std::unique_ptr<SpellingMenuObserver> spelling_suggestions_menu_observer_;
diff --git chrome/browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac_cocoa.h chrome/browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac_cocoa.h
index cb51224f9892c..b5a3946999d8f 100644
--- chrome/browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac_cocoa.h
+++ chrome/browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac_cocoa.h
@@ -29,6 +29,7 @@ class RenderViewContextMenuMacCocoa : public RenderViewContextMenuMac {
// RenderViewContextMenu:
void Show() override;
+ bool IsRunning() override;
private:
// RenderViewContextMenuViewsMac:
diff --git chrome/browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac_cocoa.mm chrome/browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac_cocoa.mm
index b130e9768c2d6..049c5fb235d87 100644
index b130e9768c2d6..3e46b2fe18d0a 100644
--- chrome/browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac_cocoa.mm
+++ chrome/browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac_cocoa.mm
@@ -68,6 +68,10 @@ RenderViewContextMenuMacCocoa::~RenderViewContextMenuMacCocoa() {
@ -138,8 +150,46 @@ index b130e9768c2d6..049c5fb235d87 100644
views::Widget* widget = views::Widget::GetTopLevelWidgetForNativeView(
source_web_contents_->GetNativeView());
@@ -93,6 +97,10 @@ void RenderViewContextMenuMacCocoa::Show() {
views::ElementTrackerViews::GetContextForWidget(widget));
}
+bool RenderViewContextMenuMacCocoa::IsRunning() {
+ return menu_controller_ && [menu_controller_ isMenuOpen];
+}
+
void RenderViewContextMenuMacCocoa::CancelToolkitMenu() {
[menu_controller_ cancel];
}
diff --git chrome/browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac_remote_cocoa.h chrome/browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac_remote_cocoa.h
index fb86992cee93f..fe674fe471cc4 100644
--- chrome/browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac_remote_cocoa.h
+++ chrome/browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac_remote_cocoa.h
@@ -27,6 +27,7 @@ class RenderViewContextMenuMacRemoteCocoa : public RenderViewContextMenuMac {
// RenderViewContextMenu:
void Show() override;
+ bool IsRunning() override;
private:
// RenderViewContextMenuViewsMac:
diff --git chrome/browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac_remote_cocoa.mm chrome/browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac_remote_cocoa.mm
index 9f6c5fd44f206..dc50bc909897f 100644
--- chrome/browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac_remote_cocoa.mm
+++ chrome/browser/ui/cocoa/renderer_context_menu/render_view_context_menu_mac_remote_cocoa.mm
@@ -42,6 +42,10 @@ void RenderViewContextMenuMacRemoteCocoa::Show() {
target_view_id_);
}
+bool RenderViewContextMenuMacRemoteCocoa::IsRunning() {
+ return runner_ && runner_->IsRunning();
+}
+
void RenderViewContextMenuMacRemoteCocoa::CancelToolkitMenu() {
runner_->Cancel();
}
diff --git chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc
index c88a77a0b49e2..d1af9a85c4ec6 100644
index c88a77a0b49e2..31b7224a36ae8 100644
--- chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc
+++ chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc
@@ -149,6 +149,9 @@ void RenderViewContextMenuViews::RunMenuAt(views::Widget* parent,
@ -163,6 +213,97 @@ index c88a77a0b49e2..d1af9a85c4ec6 100644
if (base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kKioskMode))
return;
@@ -427,6 +434,11 @@ void RenderViewContextMenuViews::Show() {
}
}
+bool RenderViewContextMenuViews::IsRunning() {
+ return static_cast<ToolkitDelegateViews*>(toolkit_delegate())
+ ->IsMenuRunning();
+}
+
views::Widget* RenderViewContextMenuViews::GetTopLevelWidget() {
return views::Widget::GetTopLevelWidgetForNativeView(GetActiveNativeView());
}
diff --git chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.h chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.h
index 55ba34c417e4a..e92f668868e74 100644
--- chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.h
+++ chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.h
@@ -45,6 +45,7 @@ class RenderViewContextMenuViews : public RenderViewContextMenu {
// RenderViewContextMenuBase implementation.
void Show() override;
+ bool IsRunning() override;
protected:
RenderViewContextMenuViews(content::RenderFrameHost& render_frame_host,
diff --git chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views.cc chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views.cc
index eb855deeb6040..3ff97b28fa4c6 100644
--- chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views.cc
+++ chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views.cc
@@ -91,6 +91,10 @@ void ChromeWebContentsViewDelegateViews::ShowMenu(
context_menu_->Show();
}
+bool ChromeWebContentsViewDelegateViews::IsMenuRunning() {
+ return context_menu_ && context_menu_->IsRunning();
+}
+
void ChromeWebContentsViewDelegateViews::ShowContextMenu(
content::RenderFrameHost& render_frame_host,
const content::ContextMenuParams& params) {
diff --git chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views.h chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views.h
index 97c98c8738965..96c142806d7b2 100644
--- chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views.h
+++ chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views.h
@@ -55,6 +55,7 @@ class ChromeWebContentsViewDelegateViews
content::RenderFrameHost& render_frame_host,
const content::ContextMenuParams& params) override;
void ShowMenu(std::unique_ptr<RenderViewContextMenuBase> menu) override;
+ bool IsMenuRunning() override;
private:
// The context menu is reset every time we show it, but we keep a pointer to
diff --git chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views_mac.h chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views_mac.h
index 07e5b3613a2df..293282690bd5a 100644
--- chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views_mac.h
+++ chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views_mac.h
@@ -55,6 +55,7 @@ class ChromeWebContentsViewDelegateViewsMac
content::RenderFrameHost& render_frame_host,
const content::ContextMenuParams& params) override;
void ShowMenu(std::unique_ptr<RenderViewContextMenuBase> menu) override;
+ bool IsMenuRunning() override;
private:
content::RenderWidgetHostView* GetActiveRenderWidgetHostView() const;
diff --git chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views_mac.mm chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views_mac.mm
index 0e2cf71973741..e8d9c38b49b6c 100644
--- chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views_mac.mm
+++ chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views_mac.mm
@@ -131,6 +131,10 @@ void ChromeWebContentsViewDelegateViewsMac::ShowMenu(
context_menu_->Show();
}
+bool ChromeWebContentsViewDelegateViewsMac::IsMenuRunning() {
+ return context_menu_ && context_menu_->IsRunning();
+}
+
content::RenderWidgetHostView*
ChromeWebContentsViewDelegateViewsMac::GetActiveRenderWidgetHostView() const {
return web_contents_->GetTopLevelRenderWidgetHostView();
diff --git components/renderer_context_menu/context_menu_delegate.h components/renderer_context_menu/context_menu_delegate.h
index 042428f77f4ad..e4efd98ca45d5 100644
--- components/renderer_context_menu/context_menu_delegate.h
+++ components/renderer_context_menu/context_menu_delegate.h
@@ -45,6 +45,8 @@ class ContextMenuDelegate {
// Displays the context menu.
virtual void ShowMenu(std::unique_ptr<RenderViewContextMenuBase> menu) = 0;
+ virtual bool IsMenuRunning() = 0;
+
private:
raw_ptr<content::WebContents> web_contents_ = nullptr;
};
diff --git components/renderer_context_menu/render_view_context_menu_base.cc components/renderer_context_menu/render_view_context_menu_base.cc
index 8e45cecb17039..e40115e23ee82 100644
--- components/renderer_context_menu/render_view_context_menu_base.cc
@ -186,11 +327,15 @@ index 8e45cecb17039..e40115e23ee82 100644
command_executed_ = true;
RecordUsedItem(id);
diff --git components/renderer_context_menu/render_view_context_menu_base.h components/renderer_context_menu/render_view_context_menu_base.h
index 57b288bc885e6..112990e3a9ad3 100644
index 57b288bc885e6..9909899e3b5fe 100644
--- components/renderer_context_menu/render_view_context_menu_base.h
+++ components/renderer_context_menu/render_view_context_menu_base.h
@@ -87,6 +87,9 @@ class RenderViewContextMenuBase : public ui::SimpleMenuModel::Delegate,
@@ -85,8 +85,13 @@ class RenderViewContextMenuBase : public ui::SimpleMenuModel::Delegate,
// Programmatically closes the context menu.
void Cancel();
+ virtual bool IsRunning() = 0;
+
const ui::SimpleMenuModel& menu_model() const { return menu_model_; }
const content::ContextMenuParams& params() const { return params_; }
+ content::WebContents* source_web_contents() const {
@ -199,7 +344,7 @@ index 57b288bc885e6..112990e3a9ad3 100644
// Returns true if the specified command id is known and valid for
// this menu. If the command is known |enabled| is set to indicate
@@ -95,6 +98,9 @@ class RenderViewContextMenuBase : public ui::SimpleMenuModel::Delegate,
@@ -95,6 +100,9 @@ class RenderViewContextMenuBase : public ui::SimpleMenuModel::Delegate,
// SimpleMenuModel::Delegate implementation.
bool IsCommandIdChecked(int command_id) const override;
@ -246,3 +391,30 @@ index 0527c57abd51c..70bebcbb50461 100644
// Called when a user selects the specified context-menu item. This is
// only called when the observer returns true for IsCommandIdSupported()
// for that |command_id|.
diff --git components/renderer_context_menu/views/toolkit_delegate_views.cc components/renderer_context_menu/views/toolkit_delegate_views.cc
index 823f044316527..85439bf7ad2eb 100644
--- components/renderer_context_menu/views/toolkit_delegate_views.cc
+++ components/renderer_context_menu/views/toolkit_delegate_views.cc
@@ -30,6 +30,10 @@ void ToolkitDelegateViews::RunMenuAt(views::Widget* parent,
anchor_position, type);
}
+bool ToolkitDelegateViews::IsMenuRunning() const {
+ return menu_runner_ && menu_runner_->IsRunning();
+}
+
void ToolkitDelegateViews::Init(ui::SimpleMenuModel* menu_model) {
menu_adapter_ = std::make_unique<views::MenuModelAdapter>(menu_model);
std::unique_ptr<views::MenuItemView> menu_view = menu_adapter_->CreateMenu();
diff --git components/renderer_context_menu/views/toolkit_delegate_views.h components/renderer_context_menu/views/toolkit_delegate_views.h
index eacdc12813204..9084effdaa121 100644
--- components/renderer_context_menu/views/toolkit_delegate_views.h
+++ components/renderer_context_menu/views/toolkit_delegate_views.h
@@ -39,6 +39,7 @@ class ToolkitDelegateViews : public RenderViewContextMenuBase::ToolkitDelegate {
const gfx::Point& point,
ui::MenuSourceType type);
views::MenuItemView* menu_view() { return menu_view_; }
+ bool IsMenuRunning() const;
protected:
// ToolkitDelegate: