Revert "[Extensions] Added visibility property to chrome.contextMenu items API"
This reverts commit 44248851c5f19227c3fc9bcc8c60285175064a3a.
Reason for revert: <INSERT REASONING HERE>
FindIt claims that this CL is causing a build failure (with 100% certainty). More info:
https://findit-for-me.appspot.com/waterfall/build-failure?url=https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Win7%20%28dbg%29/builds/10594
https://chromium-swarm.appspot.com/task?id=3822bb5befc13310&refresh=10&show_raw=1
Original change's description:
> [Extensions] Added visibility property to chrome.contextMenu items API
>
> Users of the chrome.contextMenu API should be able to programmatically
> set the visiblility (shown, hidden) of context menu items on item
> creation and update.
>
> This CL introduces the |visible| property to createProperties
> (chrome.contextMenus.create) and updateProperties
> (chrome.contextMenus.udpate). Note, this change ensures that hidden
> items are not counted against
> chrome.contextMenus.ACTION_MENU_TOP_LEVEL_LIMIT.
>
> Bug: 463476
> Change-Id: I315bcf070516fe8667e51fbec7c8f539230afa23
> Reviewed-on: https://chromium-review.googlesource.com/553578
> Commit-Queue: catmullings <[email protected]>
> Reviewed-by: Istiaque Ahmed <[email protected]>
> Reviewed-by: Devlin <[email protected]>
> Cr-Commit-Position: refs/heads/master@{#496210}
[email protected],[email protected],[email protected]
Change-Id: I25265b68f8881f1bce650a395067ce05bab4cadd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 463476
Reviewed-on: https://chromium-review.googlesource.com/626622
Reviewed-by: Dave Schuyler <[email protected]>
Commit-Queue: Dave Schuyler <[email protected]>
Cr-Commit-Position: refs/heads/master@{#496386}
diff --git a/chrome/browser/extensions/context_menu_matcher.cc b/chrome/browser/extensions/context_menu_matcher.cc
index 7b6a229c..fdda6a9 100644
--- a/chrome/browser/extensions/context_menu_matcher.cc
+++ b/chrome/browser/extensions/context_menu_matcher.cc
@@ -74,7 +74,8 @@
// If this is the first extension-provided menu item, and there are other
// items in the menu, and the last item is not a separator add a separator.
- bool prepend_separator = *index == 0 && menu_model_->GetItemCount();
+ if (*index == 0 && menu_model_->GetItemCount())
+ menu_model_->AddSeparator(ui::NORMAL_SEPARATOR);
// Extensions (other than platform apps) are only allowed one top-level slot
// (and it can't be a radio or checkbox item because we are going to put the
@@ -84,8 +85,6 @@
// Otherwise, we automatically push them into a submenu if there is more than
// one top-level item.
if (extension->is_platform_app() || is_action_menu) {
- if (prepend_separator)
- menu_model_->AddSeparator(ui::NORMAL_SEPARATOR);
RecursivelyAppendExtensionItems(items,
can_cross_incognito,
selection_text,
@@ -99,19 +98,9 @@
MenuItem::List submenu_items;
if (items.size() > 1 || items[0]->type() != MenuItem::NORMAL) {
- if (prepend_separator)
- menu_model_->AddSeparator(ui::NORMAL_SEPARATOR);
title = base::UTF8ToUTF16(extension->name());
submenu_items = items;
} else {
- // The top-level menu item, |item[0]|, is sandwiched between two menu
- // separators. If the top-level menu item is visible, its preceding
- // separator should be included in the UI model, so that both separators
- // are shown. Otherwise if the top-level menu item is hidden, the
- // preceding separator should be excluded, so that only one of the two
- // separators remain.
- if (prepend_separator && items[0]->visible())
- menu_model_->AddSeparator(ui::NORMAL_SEPARATOR);
MenuItem* item = items[0];
extension_item_map_[menu_id] = item->id();
title = item->TitleWithReplacement(selection_text,
@@ -171,20 +160,6 @@
return item->checked();
}
-bool ContextMenuMatcher::IsCommandIdVisible(int command_id) const {
- MenuItem* item = GetExtensionMenuItem(command_id);
- // The context menu code creates a top-level menu item, labeled with the
- // extension's name, that is a container of an extension's menu items. This
- // top-level menu item is not added to the context menu, so checking its
- // visibility is a special case handled below. This top-level menu item should
- // always be displayed.
- if (command_id == IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST && !item)
- return true;
- if (!item)
- return false;
- return item->visible();
-}
-
bool ContextMenuMatcher::IsCommandIdEnabled(int command_id) const {
MenuItem* item = GetExtensionMenuItem(command_id);
if (!item)
@@ -255,7 +230,7 @@
bool is_action_menu_top_level) {
MenuItem::Type last_type = MenuItem::NORMAL;
int radio_group_id = 1;
- int num_visible_items = 0;
+ int num_items = 0;
for (auto i = items.begin(); i != items.end(); ++i) {
MenuItem* item = *i;
@@ -274,12 +249,11 @@
// items will not be placed in a submenu.
const int top_level_limit = api::context_menus::ACTION_MENU_TOP_LEVEL_LIMIT;
if (menu_id >= extensions_context_custom_last ||
- (is_action_menu_top_level && num_visible_items >= top_level_limit))
+ (is_action_menu_top_level && num_items >= top_level_limit))
return;
++(*index);
- if (item->visible())
- ++num_visible_items;
+ ++num_items;
extension_item_map_[menu_id] = item->id();
base::string16 title = item->TitleWithReplacement(selection_text,