Index: chrome/browser/tab_contents/render_view_context_menu.cc |
=================================================================== |
--- chrome/browser/tab_contents/render_view_context_menu.cc (revision 182231) |
+++ chrome/browser/tab_contents/render_view_context_menu.cc (working copy) |
@@ -364,17 +364,27 @@ |
return; // In unit-tests, we may not have an ExtensionService. |
MenuManager* menu_manager = service->menu_manager(); |
+ string16 printable_selection_text = PrintableSelectionText(); |
+ // Escape "&" as "&&". |
+ for (size_t position = printable_selection_text.find('&'); |
Yoyo Zhou
2013/02/19 19:00:50
This is more complex than necessary. Use ReplaceCh
François Beaufort
2013/02/20 11:32:06
I added a TODO to commit this when the other code
|
+ position != string16::npos; |
+ position = printable_selection_text.find('&', position + 2)) |
+ printable_selection_text.insert(position, 1, '&'); |
+ |
// Get a list of extension id's that have context menu items, and sort it by |
Yoyo Zhou
2013/02/19 19:00:50
delete 'it'
François Beaufort
2013/02/20 11:32:06
Done.
|
- // the extension's name. |
+ // the top level context menu title of the extension. |
std::set<std::string> ids = menu_manager->ExtensionIds(); |
std::vector<std::pair<std::string, std::string> > sorted_ids; |
for (std::set<std::string>::iterator i = ids.begin(); i != ids.end(); ++i) { |
const Extension* extension = service->GetExtensionById(*i, false); |
// Platform apps have their context menus created directly in |
// AppendPlatformAppItems. |
Yoyo Zhou
2013/02/19 19:00:50
Does AppendPlatformAppItems have the same sorting
François Beaufort
2013/02/20 11:32:06
From what I can read, there is no sorting involved
|
- if (extension && !extension->is_platform_app()) |
+ if (extension && !extension->is_platform_app()) { |
+ std::string menu_title = extension_items_.GetTopLevelContextMenuTitle(*i, |
Yoyo Zhou
2013/02/19 19:00:50
nit: put *i on the next line. You can put it on th
François Beaufort
2013/02/20 11:32:06
Done.
|
+ printable_selection_text); |
sorted_ids.push_back( |
- std::pair<std::string, std::string>(extension->name(), *i)); |
+ std::pair<std::string, std::string>(menu_title, *i)); |
+ } |
} |
// TODO(asargent) - See if this works properly for i18n names (bug 32363). |
std::sort(sorted_ids.begin(), sorted_ids.end()); |
@@ -383,20 +393,12 @@ |
return; |
int index = 0; |
+ // TODO(fbeaufort): Where to move this UMA now? |
Yoyo Zhou
2013/02/19 19:00:50
I think it's okay to leave it here.
François Beaufort
2013/02/20 11:32:06
Done.
|
base::TimeTicks begin = base::TimeTicks::Now(); |
std::vector<std::pair<std::string, std::string> >::const_iterator i; |
- for (i = sorted_ids.begin(); |
- i != sorted_ids.end(); ++i) { |
- string16 printable_selection_text = PrintableSelectionText(); |
- // Escape "&" as "&&". |
- for (size_t position = printable_selection_text.find('&'); |
- position != string16::npos; |
- position = printable_selection_text.find('&', position + 2)) |
- printable_selection_text.insert(position, 1, '&'); |
- |
+ for (i = sorted_ids.begin(); i != sorted_ids.end(); ++i) |
extension_items_.AppendExtensionItems(i->second, printable_selection_text, |
Yoyo Zhou
2013/02/19 19:00:50
I don't see how this can behave equivalently to th
François Beaufort
2013/02/20 11:32:06
I'm not sure about what you mean here. Before, pri
Yoyo Zhou
2013/03/06 20:18:58
No. I'm not sure of this, but I believe PrintableS
|
&index); |
- } |
UMA_HISTOGRAM_TIMES("Extensions.ContextMenus_BuildTime", |
base::TimeTicks::Now() - begin); |
UMA_HISTOGRAM_COUNTS("Extensions.ContextMenus_ItemCount", index); |