Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix testing methods for renderer context menu so tests can run. #4673

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,24 @@
// Make it clear which class we mean here.
#undef RenderViewContextMenu

base::OnceCallback<void(BraveRenderViewContextMenu*)>* GetMenuShownCallback() {
static base::NoDestructor<base::OnceCallback<void(BraveRenderViewContextMenu*)>>
callback;
return callback.get();
}

BraveRenderViewContextMenu::BraveRenderViewContextMenu(
content::RenderFrameHost* render_frame_host,
const content::ContextMenuParams& params)
: RenderViewContextMenu_Chromium(render_frame_host, params) {
}

void BraveRenderViewContextMenu::NotifyMenuShown() {
auto* cb = GetMenuShownCallback();
if (!cb->is_null())
std::move(*cb).Run(this);
}

bool BraveRenderViewContextMenu::IsCommandIdEnabled(int id) const {
switch (id) {
case IDC_CONTENT_CONTEXT_OPENLINKTOR:
Expand Down Expand Up @@ -66,6 +78,12 @@ void BraveRenderViewContextMenu::ExecuteCommand(int id, int event_flags) {
}
}

// static
void BraveRenderViewContextMenu::RegisterMenuShownCallbackForTesting(
base::OnceCallback<void(BraveRenderViewContextMenu*)> cb) {
*GetMenuShownCallback() = std::move(cb);
}

void BraveRenderViewContextMenu::AddSpellCheckServiceItem(bool is_checked) {
// Call our implementation, not the one in the base class.
// Assumption:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ class BraveRenderViewContextMenu : public RenderViewContextMenu_Chromium {
// Hide base class implementation.
static void AddSpellCheckServiceItem(ui::SimpleMenuModel* menu,
bool is_checked);
void NotifyMenuShown() override;
// Registers a one-time callback that will be called the next time a context
// menu is shown.
static void RegisterMenuShownCallbackForTesting(
base::OnceCallback<void(BraveRenderViewContextMenu*)> cb);

private:
// RenderViewContextMenuBase:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
diff --git a/chrome/browser/renderer_context_menu/render_view_context_menu.cc b/chrome/browser/renderer_context_menu/render_view_context_menu.cc
index 491e36bcdfbaff0c35c5d6ca6c740df35eb35ad2..746f5402368f07b4d01290d68b9312777de95491 100644
--- a/chrome/browser/renderer_context_menu/render_view_context_menu.cc
+++ b/chrome/browser/renderer_context_menu/render_view_context_menu.cc
@@ -226,12 +226,6 @@ using extensions::MenuManager;

namespace {

-base::OnceCallback<void(RenderViewContextMenu*)>* GetMenuShownCallback() {
- static base::NoDestructor<base::OnceCallback<void(RenderViewContextMenu*)>>
- callback;
- return callback.get();
-}
-
// State of the profile that is activated via "Open Link as User".
enum UmaEnumOpenLinkAsUser {
OPEN_LINK_AS_USER_ACTIVE_PROFILE_ENUM_ID,
@@ -2475,12 +2469,6 @@ void RenderViewContextMenu::AddAccessibilityLabelsServiceItem(bool is_checked) {
}
}

-// static
-void RenderViewContextMenu::RegisterMenuShownCallbackForTesting(
- base::OnceCallback<void(RenderViewContextMenu*)> cb) {
- *GetMenuShownCallback() = std::move(cb);
-}
-
ProtocolHandlerRegistry::ProtocolHandlerList
RenderViewContextMenu::GetHandlersForLinkUrl() {
ProtocolHandlerRegistry::ProtocolHandlerList handlers =
@@ -2490,9 +2478,9 @@ ProtocolHandlerRegistry::ProtocolHandlerList
}

void RenderViewContextMenu::NotifyMenuShown() {
- auto* cb = GetMenuShownCallback();
- if (!cb->is_null())
- std::move(*cb).Run(this);
+ // Ensure that if this is called something breaks.
+ // If this method is updated in Chromium, update the BraveRenderViewContextMenu method.
+ DCHECK(false);
}

base::string16 RenderViewContextMenu::PrintableSelectionText() {
Original file line number Diff line number Diff line change
@@ -1,8 +1,20 @@
diff --git a/chrome/browser/renderer_context_menu/render_view_context_menu.h b/chrome/browser/renderer_context_menu/render_view_context_menu.h
index d5195d4b0f6c8aae36728942278a75d5ac7bed4e..eac7504bc11aca503235379c52c04c432ab5c128 100644
index d5195d4b0f6c8aae36728942278a75d5ac7bed4e..a9697e02b008c2faee9c369c57f56b87c92985f6 100644
--- a/chrome/browser/renderer_context_menu/render_view_context_menu.h
+++ b/chrome/browser/renderer_context_menu/render_view_context_menu.h
@@ -113,6 +113,7 @@ class RenderViewContextMenu : public RenderViewContextMenuBase {
@@ -83,11 +83,6 @@ class RenderViewContextMenu : public RenderViewContextMenuBase {
void AddSpellCheckServiceItem(bool is_checked) override;
void AddAccessibilityLabelsServiceItem(bool is_checked) override;

- // Registers a one-time callback that will be called the next time a context
- // menu is shown.
- static void RegisterMenuShownCallbackForTesting(
- base::OnceCallback<void(RenderViewContextMenu*)> cb);
-
protected:
Profile* GetProfile() const;

@@ -113,6 +108,7 @@ class RenderViewContextMenu : public RenderViewContextMenuBase {
// Returns true if keyboard lock is active and requires the user to press and
// hold escape to exit exclusive access mode.
bool IsPressAndHoldEscRequiredToExitFullscreen() const;
Expand Down