Fix top-level context menus sorting by name

chrome/browser/extensions/extension_context_menu_browsertest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/extensions/extension_test_message_listener.h"
@@ skipping 313 matching lines @@
   // truncated.
   GURL url("http://foo.com/");
   scoped_ptr<TestRenderViewContextMenu> menu(
       CreateMenu(browser(), url, GURL(), GURL()));
 
   string16 label;
   ASSERT_TRUE(GetItemLabel(menu.get(), item->id(), &label));
   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,
                                MenuModel::ItemType expected_type,
                                const MenuModel& menu,
                                int index) {
   EXPECT_EQ(expected_type, menu.GetTypeAt(index));
   EXPECT_EQ(UTF8ToUTF16(expected_label), menu.GetLabelAt(index));
 }
 
@@ skipping 236 matching lines @@
   ASSERT_TRUE(LoadContextMenuExtensionIncognito("incognito"));
 
   // Wait for the extension's processes to tell us they've created an item.
   ASSERT_TRUE(created.WaitUntilSatisfied());
   ASSERT_TRUE(created_incognito.WaitUntilSatisfied());
   ASSERT_EQ(2u, GetItems().size());
 
   browser()->profile()->DestroyOffTheRecordProfile();
   ASSERT_EQ(1u, GetItems().size());
 }