Fix top-level context menus sorting by name

chrome/browser/extensions/context_menu_matcher.cc

Index: chrome/browser/extensions/context_menu_matcher.cc
===================================================================
--- chrome/browser/extensions/context_menu_matcher.cc	(revision 182231)
+++ chrome/browser/extensions/context_menu_matcher.cc	(working copy)
@@ -98,6 +98,35 @@
   extension_menu_models_.clear();
 }
 
+std::string ContextMenuMatcher::GetTopLevelContextMenuTitle(

[Yoyo Zhou, 2013/02/19 19:00:50]

This duplicates code in AppendExtensionItems. Can you make that function use the
logic here, so the code isn't duplicated?

[François Beaufort, 2013/02/20 11:32:06]

I'm not sure about how to implement the private function and would like your
input on this.

Do you think of a function that would take 3 parameters "extension_id",
"extension*" and "items*" and sets "extension*" and "items*"  

OR something else?

On 2013/02/19 19:00:50, Yoyo Zhou wrote:

[Yoyo Zhou, 2013/03/06 20:18:58]

I was thinking it should take an extension_id as input and return the
MenuItem::List items. There will need to be some slight logic refactoring
(around the early returns if manager->MenuItems(extension_id) is empty).

You could call this GetRelevantExtensionTopLevelItems.

+    const std::string& extension_id,
+    const string16& selection_text) {
+  ExtensionService* service =
+      extensions::ExtensionSystem::Get(profile_)->extension_service();
+  MenuManager* manager = service->menu_manager();
+  const Extension* extension = service->GetExtensionById(extension_id, false);
+
+  // Find matching items.
+  const MenuItem::List* all_items = manager->MenuItems(extension_id);
+  if (!all_items || all_items->empty())
+    // Return extension name if there are no items

[Yoyo Zhou, 2013/02/19 19:00:50]

Shouldn't this function never be called for extensions that have no items? You
ought to be able to change this to
CHECK(all_items && !all_items->empty());

[François Beaufort, 2013/02/20 11:32:06]

Done.

+    return extension->name();
+  bool can_cross_incognito = service->CanCrossIncognito(extension);
+  MenuItem::List items = GetRelevantExtensionItems(*all_items,
+                                                   can_cross_incognito);
+
+  std::string title;
+
+  if (items.size() > 1 || items[0]->type() != MenuItem::NORMAL) {
+    title = extension->name();
+  } else {
+    MenuItem* item = items[0];
+    title = UTF16ToUTF8(item->TitleWithReplacement(selection_text,
+        kMaxExtensionItemTitleLength));

[Yoyo Zhou, 2013/02/19 19:00:50]

nit: if you continue function arguments onto the next line like this,
indentation should be to the open paren. Since it won't fit, start the first
argument on a new line (at +4 spaces).

[François Beaufort, 2013/02/20 11:32:06]

Done.

+  }
+  return title;
+}
+
 bool ContextMenuMatcher::IsCommandIdChecked(int command_id) const {
   MenuItem* item = GetExtensionMenuItem(command_id);
   if (!item)