Commit 7a1e3f91 authored by Devlin Cronin's avatar Devlin Cronin Committed by Commit Bot

[Extensions] Use ActionInfo::Type directly in ExtensionContextMenuModel

The ExtensionContextMenuModel looks at the action type in order to
determine if context menu items match. Instead of using a bespoke enum,
use ActionInfo::Type directly (with a base::Optional<> to indicate no
action).

This also:
- Removes a ternary that treated TYPE_ACTION the same as TYPE_BROWSER
- Removes the action type as a member variable, since its only used in
  one place.

Finally, this helps pave the way for introducing an "action" context
for menus.

Bug: 893373
Change-Id: Ibef23864c8a849f7248cd6b73ef1dfe728962a58
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2132531Reviewed-by: default avatarIstiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757595}
parent ba2ec763
...@@ -56,9 +56,9 @@ namespace extensions { ...@@ -56,9 +56,9 @@ namespace extensions {
namespace { namespace {
// Returns true if the given |item| is of the given |type|. // Returns true if the given |item| is of the given |type|.
bool MenuItemMatchesAction(ExtensionContextMenuModel::ActionType type, bool MenuItemMatchesAction(const base::Optional<ActionInfo::Type> action_type,
const MenuItem* item) { const MenuItem* item) {
if (type == ExtensionContextMenuModel::NO_ACTION) if (!action_type)
return false; return false;
const MenuItem::ContextList& contexts = item->contexts(); const MenuItem::ContextList& contexts = item->contexts();
...@@ -66,11 +66,15 @@ bool MenuItemMatchesAction(ExtensionContextMenuModel::ActionType type, ...@@ -66,11 +66,15 @@ bool MenuItemMatchesAction(ExtensionContextMenuModel::ActionType type,
if (contexts.Contains(MenuItem::ALL)) if (contexts.Contains(MenuItem::ALL))
return true; return true;
if (contexts.Contains(MenuItem::PAGE_ACTION) && if (contexts.Contains(MenuItem::PAGE_ACTION) &&
(type == ExtensionContextMenuModel::PAGE_ACTION)) (*action_type == ActionInfo::TYPE_PAGE)) {
return true; return true;
}
if (contexts.Contains(MenuItem::BROWSER_ACTION) && if (contexts.Contains(MenuItem::BROWSER_ACTION) &&
(type == ExtensionContextMenuModel::BROWSER_ACTION)) (*action_type == ActionInfo::TYPE_BROWSER)) {
return true; return true;
}
// TODO(devlin): Add support for ActionInfo::TYPE_ACTION here.
return false; return false;
} }
...@@ -200,7 +204,6 @@ ExtensionContextMenuModel::ExtensionContextMenuModel( ...@@ -200,7 +204,6 @@ ExtensionContextMenuModel::ExtensionContextMenuModel(
browser_(browser), browser_(browser),
profile_(browser->profile()), profile_(browser->profile()),
delegate_(delegate), delegate_(delegate),
action_type_(NO_ACTION),
button_visibility_(button_visibility), button_visibility_(button_visibility),
can_show_icon_in_toolbar_(can_show_icon_in_toolbar) { can_show_icon_in_toolbar_(can_show_icon_in_toolbar) {
InitMenu(extension, button_visibility); InitMenu(extension, button_visibility);
...@@ -377,16 +380,15 @@ void ExtensionContextMenuModel::InitMenu(const Extension* extension, ...@@ -377,16 +380,15 @@ void ExtensionContextMenuModel::InitMenu(const Extension* extension,
ButtonVisibility button_visibility) { ButtonVisibility button_visibility) {
DCHECK(extension); DCHECK(extension);
base::Optional<ActionInfo::Type> action_type;
extension_action_ = extension_action_ =
ExtensionActionManager::Get(profile_)->GetExtensionAction(*extension); ExtensionActionManager::Get(profile_)->GetExtensionAction(*extension);
if (extension_action_) { if (extension_action_)
action_type_ = extension_action_->action_type() == ActionInfo::TYPE_PAGE action_type = extension_action_->action_type();
? PAGE_ACTION
: BROWSER_ACTION;
}
extension_items_.reset(new ContextMenuMatcher( extension_items_.reset(new ContextMenuMatcher(
profile_, this, this, base::Bind(MenuItemMatchesAction, action_type_))); profile_, this, this,
base::BindRepeating(MenuItemMatchesAction, action_type)));
std::string extension_name = extension->name(); std::string extension_name = extension->name();
// Ampersands need to be escaped to avoid being treated like // Ampersands need to be escaped to avoid being treated like
......
...@@ -69,9 +69,6 @@ class ExtensionContextMenuModel : public ui::SimpleMenuModel, ...@@ -69,9 +69,6 @@ class ExtensionContextMenuModel : public ui::SimpleMenuModel,
kMaxValue = kPageAccessLearnMore, kMaxValue = kPageAccessLearnMore,
}; };
// Type of action the extension icon represents.
enum ActionType { NO_ACTION = 0, BROWSER_ACTION, PAGE_ACTION };
// The current visibility of the button; this can affect the "hide"/"show" // The current visibility of the button; this can affect the "hide"/"show"
// strings in the menu. // strings in the menu.
enum ButtonVisibility { enum ButtonVisibility {
...@@ -161,9 +158,6 @@ class ExtensionContextMenuModel : public ui::SimpleMenuModel, ...@@ -161,9 +158,6 @@ class ExtensionContextMenuModel : public ui::SimpleMenuModel,
// The delegate which handles the 'inspect popup' menu command (or NULL). // The delegate which handles the 'inspect popup' menu command (or NULL).
PopupDelegate* delegate_; PopupDelegate* delegate_;
// The type of extension action to which this context menu is attached.
ActionType action_type_;
// The visibility of the button at the time the menu opened. // The visibility of the button at the time the menu opened.
ButtonVisibility button_visibility_; ButtonVisibility button_visibility_;
......
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