Extend contextMenus API to support browser/page actions

chrome/browser/renderer_context_menu/render_view_context_menu.cc

Index: chrome/browser/renderer_context_menu/render_view_context_menu.cc
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 8b1f54547a36a5301418e68b6e521c204cc1aece..20cfd5d83045657a39aec6940a6dc7c8c0bfcfde 100644
--- a/chrome/browser/renderer_context_menu/render_view_context_menu.cc
+++ b/chrome/browser/renderer_context_menu/render_view_context_menu.cc
@@ -18,7 +18,6 @@
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/time/time.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/app_mode/app_mode_utils.h"
 #include "chrome/browser/autocomplete/autocomplete_classifier.h"
@@ -563,17 +562,17 @@ void RenderViewContextMenu::AppendAllExtensionItems() {
   std::set<MenuItem::ExtensionKey> ids = menu_manager->ExtensionIds();
   std::vector<base::string16> sorted_menu_titles;
   std::map<base::string16, std::string> map_ids;
-  for (std::set<MenuItem::ExtensionKey>::iterator i = ids.begin();
-       i != ids.end();
-       ++i) {
+  for (std::set<MenuItem::ExtensionKey>::iterator iter = ids.begin();
+       iter != ids.end();
+       ++iter) {
     const Extension* extension =
-        service->GetExtensionById(i->extension_id, false);
+        service->GetExtensionById(iter->extension_id, false);
     // Platform apps have their context menus created directly in
     // AppendPlatformAppItems.
     if (extension && !extension->is_platform_app()) {
       base::string16 menu_title = extension_items_.GetTopLevelContextMenuTitle(
-          *i, printable_selection_text);
-      map_ids[menu_title] = i->extension_id;
+          *iter, printable_selection_text);
+      map_ids[menu_title] = iter->extension_id;
       sorted_menu_titles.push_back(menu_title);
     }
   }
@@ -584,17 +583,15 @@ void RenderViewContextMenu::AppendAllExtensionItems() {
   l10n_util::SortStrings16(app_locale, &sorted_menu_titles);
 
   int index = 0;
-  base::TimeTicks begin = base::TimeTicks::Now();
-  for (size_t i = 0; i < sorted_menu_titles.size(); ++i) {
-    const std::string& id = map_ids[sorted_menu_titles[i]];
-    const MenuItem::ExtensionKey extension_key(id);
-    extension_items_.AppendExtensionItems(
-        extension_key, printable_selection_text, &index);
+  for (std::vector<base::string16>::iterator iter = sorted_menu_titles.begin();

[brettw, 2014/08/04 20:17:47]

It's not clear to me why you rewrote this loop. To me, the new code seems
strictly worse than the old code, and even if it wasn't, this isn't the type of
thing that's worth doing.

[Devlin, 2014/08/04 20:23:02]

Greg probably did this based off my comments on similar code in extensions/ (I
prefer this way because it's marginally faster to loop through the vector with
an iter than to dereference all the different indices).  But we don't need to
change old code to match, and OWNERS preference takes priority.

[gpdavis, 2014/08/04 20:52:32]

Yep, that was it.  Since Devlin suggested these changes to code I borrowed from
here, I thought I'd change it here as well (made a note of it in an earlier
patch set).

I'll go ahead and put this back to the way it was.

+       iter != sorted_menu_titles.end();
+       ++iter) {
+    const std::string& id = map_ids[*iter];
+    extension_items_.AppendExtensionItems(MenuItem::ExtensionKey(id),
+                                          printable_selection_text,
+                                          &index,
+                                          false);  // is_action_menu
   }
-
-  UMA_HISTOGRAM_TIMES("Extensions.ContextMenus_BuildTime",
-                      base::TimeTicks::Now() - begin);
-  UMA_HISTOGRAM_COUNTS("Extensions.ContextMenus_ItemCount", index);
 }
 
 void RenderViewContextMenu::AppendCurrentExtensionItems() {
@@ -607,8 +604,10 @@ void RenderViewContextMenu::AppendCurrentExtensionItems() {
     int index = 0;
     const MenuItem::ExtensionKey key(
         extension->id(), WebViewGuest::GetViewInstanceId(source_web_contents_));
-    extension_items_.AppendExtensionItems(
-        key, PrintableSelectionText(), &index);
+    extension_items_.AppendExtensionItems(key,
+                                          PrintableSelectionText(),
+                                          &index,
+                                          false);  // is_action_menu
   }
 }