Fix top-level context menus sorting by name

chrome/browser/extensions/extension_context_menu_browsertest.cc

Index: chrome/browser/extensions/extension_context_menu_browsertest.cc
===================================================================
--- chrome/browser/extensions/extension_context_menu_browsertest.cc	(revision 188662)
+++ chrome/browser/extensions/extension_context_menu_browsertest.cc	(working copy)
@@ -331,6 +331,76 @@
   ASSERT_TRUE(label.size() <= limit);
 }
 
+// Checks that Context Menus are ordered alphabetically by their name when
+// extensions have only one single Context Menu item and by the extension name
+// when multiples Context Menu items are created.
+IN_PROC_BROWSER_TEST_F(ExtensionContextMenuBrowserTest, TopLevel) {
+  // We expect to see the following items in the menu:

[Yoyo Zhou, 2013/03/20 21:54:13]

nit: This would be clearer with 2-space indenting.

[François Beaufort, 2013/05/07 15:51:53]

Done.

+  //  An Extension with multiple Context Menus
+  //   Context Menu #1
+  //   Context Menu #2
+  //  Context Menu #1 - Extension #2
+  //  Context Menu #2 - Extension #3
+  //  Context Menu #3 - Extension #1
+  //  Ze Extension with multiple Context Menus
+  //   Context Menu #1
+  //   Context Menu #2
+
+  // Load extensions and wait until it's created a single menu item.
+  ExtensionTestMessageListener listener1("created item", false);
+  ASSERT_TRUE(LoadContextMenuExtension("top_level_single1"));

[Yoyo Zhou, 2013/03/20 21:54:13]

You can write another helper function like LoadContextMenuExtension for this
test which does

test_data_dir_.AppendASCII("context_menus").AppendASCII("top_level").AppendASCII(subdirectory)

[François Beaufort, 2013/05/07 15:51:53]

Done.

+  ASSERT_TRUE(listener1.WaitUntilSatisfied());
+
+  ExtensionTestMessageListener listener2("created item", false);
+  ASSERT_TRUE(LoadContextMenuExtension("top_level_single2"));
+  ASSERT_TRUE(listener2.WaitUntilSatisfied());
+
+  ExtensionTestMessageListener listener3("created item", false);
+  ASSERT_TRUE(LoadContextMenuExtension("top_level_single3"));
+  ASSERT_TRUE(listener3.WaitUntilSatisfied());
+
+  // Load extensions and wait until it's created two menu items.
+  ExtensionTestMessageListener listener4("created items", false);
+  ASSERT_TRUE(LoadContextMenuExtension("top_level_multi4"));
+  ASSERT_TRUE(listener4.WaitUntilSatisfied());
+
+  ExtensionTestMessageListener listener5("created items", false);
+  ASSERT_TRUE(LoadContextMenuExtension("top_level_multi5"));
+  ASSERT_TRUE(listener5.WaitUntilSatisfied());
+
+  GURL url("http://foo.com/");
+  scoped_ptr<TestRenderViewContextMenu> menu(
+      CreateMenu(browser(), url, GURL(), GURL()));
+
+  int index = 0;
+  MenuModel* model = NULL;
+
+  ASSERT_TRUE(menu->GetMenuModelAndItemIndex(
+      IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST, &model, &index));
+  EXPECT_EQ(UTF8ToUTF16("An Extension with multiple Context Menus"),
+      model->GetLabelAt(index));

[Yoyo Zhou, 2013/03/20 21:54:13]

You should just be able to use
model = menu->menu_model();
model->GetLabelAt(0);
etc.

Also, please align indentation to the open paren.

[François Beaufort, 2013/05/07 15:51:53]

model->GetLabelAt(0) actually returns &Back and so on ;)

Hope you'll like this better.

Indentation is fixed.


On 2013/03/20 21:54:13, Yoyo Zhou wrote:

+
+  ASSERT_TRUE(menu->GetMenuModelAndItemIndex(
+      IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST + 3, &model, &index));
+  EXPECT_EQ(UTF8ToUTF16("Context Menu #1 - Extension #2"),
+      model->GetLabelAt(index));
+
+  ASSERT_TRUE(menu->GetMenuModelAndItemIndex(
+      IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST + 4, &model, &index));
+  EXPECT_EQ(UTF8ToUTF16("Context Menu #2 - Extension #3"),
+      model->GetLabelAt(index));
+
+  ASSERT_TRUE(menu->GetMenuModelAndItemIndex(
+      IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST + 5, &model, &index));
+  EXPECT_EQ(UTF8ToUTF16("Context Menu #3 - Extension #1"),
+      model->GetLabelAt(index));
+
+  ASSERT_TRUE(menu->GetMenuModelAndItemIndex(
+      IDC_EXTENSIONS_CONTEXT_CUSTOM_FIRST + 6, &model, &index));
+  EXPECT_EQ(UTF8ToUTF16("Ze Extension with multiple Context Menus"),
+      model->GetLabelAt(index));
+}
+
 // Checks that in |menu|, the item at |index| has type |expected_type| and a
 // label of |expected_label|.
 static void ExpectLabelAndType(const char* expected_label,