Chromium Code Reviews| Index: chrome/browser/extensions/context_menu_matcher.cc | 
| diff --git a/chrome/browser/extensions/context_menu_matcher.cc b/chrome/browser/extensions/context_menu_matcher.cc | 
| index 0e7adbddbdfc61208f56cfd0c7d2a7e67421d05e..451fb0dab7b157909252c2e2c2a0efb544b361b3 100644 | 
| --- a/chrome/browser/extensions/context_menu_matcher.cc | 
| +++ b/chrome/browser/extensions/context_menu_matcher.cc | 
| @@ -14,6 +14,13 @@ | 
| #include "ui/gfx/image/image.h" | 
| namespace extensions { | 
| +namespace api { | 
| +namespace context_menus { | 
| + | 
| +extern const int ACTION_MENU_TOP_LEVEL_LIMIT; | 
| 
 
Yoyo Zhou
2014/07/22 00:44:32
Why not include the generated chrome/common/extens
 
gpdavis
2014/07/22 18:52:52
Ah, that is more convenient.  I wasn't sure exactl
 
 | 
| + | 
| +} // namespace context_menus | 
| +} // namespace api | 
| // static | 
| const size_t ContextMenuMatcher::kMaxExtensionItemTitleLength = 75; | 
| @@ -30,7 +37,8 @@ ContextMenuMatcher::ContextMenuMatcher( | 
| void ContextMenuMatcher::AppendExtensionItems( | 
| const MenuItem::ExtensionKey& extension_key, | 
| const base::string16& selection_text, | 
| - int* index) { | 
| + int* index, | 
| + bool is_action_menu) { | 
| DCHECK_GE(*index, 0); | 
| int max_index = | 
| IDC_EXTENSIONS_CONTEXT_CUSTOM_LAST - IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST; | 
| @@ -54,11 +62,18 @@ void ContextMenuMatcher::AppendExtensionItems( | 
| // 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 | 
| - // extension icon next to it). | 
| - // If they have more than that, we automatically push them into a submenu. | 
| - if (extension->is_platform_app()) { | 
| - RecursivelyAppendExtensionItems(items, can_cross_incognito, selection_text, | 
| - menu_model_, index); | 
| + // extension icon next to it), unless the context menu is an an action menu. | 
| + // Action menus only include items from one extension, and do not include the | 
| 
 
Yoyo Zhou
2014/07/22 00:44:32
Rewrite this: Action menus do not include the exte
 
gpdavis
2014/07/22 18:52:52
Done.
 
 | 
| + // extension icon, so they are not placed within a submenu. 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) { | 
| + RecursivelyAppendExtensionItems(items, | 
| + can_cross_incognito, | 
| + selection_text, | 
| + menu_model_, | 
| + index, | 
| + is_action_menu); | 
| } else { | 
| int menu_id = IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST + (*index)++; | 
| base::string16 title; | 
| @@ -83,10 +98,15 @@ void ContextMenuMatcher::AppendExtensionItems( | 
| ui::SimpleMenuModel* submenu = new ui::SimpleMenuModel(delegate_); | 
| extension_menu_models_.push_back(submenu); | 
| menu_model_->AddSubMenu(menu_id, title, submenu); | 
| - RecursivelyAppendExtensionItems(submenu_items, can_cross_incognito, | 
| - selection_text, submenu, index); | 
| + RecursivelyAppendExtensionItems(submenu_items, | 
| + can_cross_incognito, | 
| + selection_text, | 
| + submenu, | 
| + index, | 
| + false); | 
| } | 
| - SetExtensionIcon(extension_key.extension_id); | 
| + if (!is_action_menu) | 
| + SetExtensionIcon(extension_key.extension_id); | 
| } | 
| } | 
| @@ -191,10 +211,11 @@ void ContextMenuMatcher::RecursivelyAppendExtensionItems( | 
| bool can_cross_incognito, | 
| const base::string16& selection_text, | 
| ui::SimpleMenuModel* menu_model, | 
| - int* index) | 
| -{ | 
| + int* index, | 
| + bool is_action_menu_top_level) { | 
| MenuItem::Type last_type = MenuItem::NORMAL; | 
| int radio_group_id = 1; | 
| + int num_items = 0; | 
| for (MenuItem::List::const_iterator i = items.begin(); | 
| i != items.end(); ++i) { | 
| @@ -208,9 +229,16 @@ void ContextMenuMatcher::RecursivelyAppendExtensionItems( | 
| last_type = MenuItem::SEPARATOR; | 
| } | 
| - int menu_id = IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST + (*index)++; | 
| - if (menu_id >= IDC_EXTENSIONS_CONTEXT_CUSTOM_LAST) | 
| + int menu_id = IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST + *index; | 
| + num_items++; | 
| + // Action context menus have a limit for top level extension items to | 
| + // prevent control items from being pushed off the screen, since extension | 
| + // items will not be placed in a submenu. | 
| + if (menu_id >= IDC_EXTENSIONS_CONTEXT_CUSTOM_LAST || | 
| + (is_action_menu_top_level && | 
| + num_items > api::context_menus::ACTION_MENU_TOP_LEVEL_LIMIT)) | 
| return; | 
| + (*index)++; | 
| extension_item_map_[menu_id] = item->id(); | 
| base::string16 title = item->TitleWithReplacement(selection_text, | 
| kMaxExtensionItemTitleLength); | 
| @@ -223,8 +251,12 @@ void ContextMenuMatcher::RecursivelyAppendExtensionItems( | 
| ui::SimpleMenuModel* submenu = new ui::SimpleMenuModel(delegate_); | 
| extension_menu_models_.push_back(submenu); | 
| menu_model->AddSubMenu(menu_id, title, submenu); | 
| - RecursivelyAppendExtensionItems(children, can_cross_incognito, | 
| - selection_text, submenu, index); | 
| + RecursivelyAppendExtensionItems(children, | 
| + can_cross_incognito, | 
| + selection_text, | 
| + submenu, | 
| + index, | 
| + false); | 
| } | 
| } else if (item->type() == MenuItem::CHECKBOX) { | 
| menu_model->AddCheckItem(menu_id, title); |