Desktop context menu reorganisation

chrome/browser/renderer_context_menu/spelling_menu_observer_browsertest.cc

Index: chrome/browser/renderer_context_menu/spelling_menu_observer_browsertest.cc
diff --git a/chrome/browser/renderer_context_menu/spelling_menu_observer_browsertest.cc b/chrome/browser/renderer_context_menu/spelling_menu_observer_browsertest.cc
index f5eadc55178f95048c18c70b599b07e3b2b70384..358861ee2714bf94fd82e51bd8e01cae1ee0746e 100644
--- a/chrome/browser/renderer_context_menu/spelling_menu_observer_browsertest.cc
+++ b/chrome/browser/renderer_context_menu/spelling_menu_observer_browsertest.cc
@@ -264,12 +264,11 @@ IN_PROC_BROWSER_TEST_F(SpellingMenuObserverTest, InitMenuWithCorrectWord) {
   EXPECT_EQ(static_cast<size_t>(0), menu()->GetMenuSize());
 }
 
-// Tests that right-clicking a misspelled word adds four items:
-// "No spelling suggestions", "Add to dictionary", "Ask Google for suggestions",
-// and a separator.
+// Tests that right-clicking a misspelled word adds three items:
+// "Add to dictionary", "Ask Google for suggestions",and a separator.

[Avi (use Gerrit), 2015/09/26 02:26:15]

space after a comma

[edwardjung, 2015/09/28 14:11:26]

Done.

 IN_PROC_BROWSER_TEST_F(SpellingMenuObserverTest, InitMenuWithMisspelledWord) {
   InitMenu("wiimode", NULL);
-  EXPECT_EQ(static_cast<size_t>(4), menu()->GetMenuSize());
+  EXPECT_EQ(static_cast<size_t>(3), menu()->GetMenuSize());

[Avi (use Gerrit), 2015/09/26 02:26:15]

er, just do 3u

[edwardjung, 2015/09/28 14:11:25]

Hadn't realised this. Done.

 
   // Read all the context-menu items added by this test and verify they are
   // expected ones. We do not check the item titles to prevent resource changes
@@ -277,19 +276,15 @@ IN_PROC_BROWSER_TEST_F(SpellingMenuObserverTest, InitMenuWithMisspelledWord) {
   // resources.)
   MockRenderViewContextMenu::MockMenuItem item;
   menu()->GetMenuItem(0, &item);
-  EXPECT_EQ(IDC_CONTENT_CONTEXT_NO_SPELLING_SUGGESTIONS, item.command_id);
-  EXPECT_FALSE(item.enabled);
-  EXPECT_FALSE(item.hidden);
-  menu()->GetMenuItem(1, &item);
   EXPECT_EQ(IDC_SPELLCHECK_ADD_TO_DICTIONARY, item.command_id);
   EXPECT_TRUE(item.enabled);
   EXPECT_FALSE(item.hidden);
-  menu()->GetMenuItem(2, &item);
+  menu()->GetMenuItem(1, &item);
   EXPECT_EQ(IDC_CONTENT_CONTEXT_SPELLING_TOGGLE, item.command_id);
   EXPECT_TRUE(item.enabled);
   EXPECT_FALSE(item.checked);
   EXPECT_FALSE(item.hidden);
-  menu()->GetMenuItem(3, &item);
+  menu()->GetMenuItem(2, &item);
   EXPECT_EQ(-1, item.command_id);
   EXPECT_FALSE(item.enabled);
   EXPECT_FALSE(item.hidden);
@@ -318,51 +313,18 @@ IN_PROC_BROWSER_TEST_F(SpellingMenuObserverTest, EnableSpellingService) {
   menu()->GetPrefs()->Set(prefs::kSpellCheckDictionaries, dictionary);
 
   InitMenu("wiimode", NULL);
-  EXPECT_EQ(static_cast<size_t>(4), menu()->GetMenuSize());
+  EXPECT_EQ(static_cast<size_t>(3), menu()->GetMenuSize());

[Avi (use Gerrit), 2015/09/26 02:26:15]

3u

[edwardjung, 2015/09/28 14:11:25]

Done.

 
   // To avoid duplicates, this test reads only the "Ask Google for suggestions"
   // item and verifies it is enabled and checked.
   MockRenderViewContextMenu::MockMenuItem item;
-  menu()->GetMenuItem(2, &item);
+  menu()->GetMenuItem(1, &item);
   EXPECT_EQ(IDC_CONTENT_CONTEXT_SPELLING_TOGGLE, item.command_id);
   EXPECT_TRUE(item.enabled);
   EXPECT_TRUE(item.checked);
   EXPECT_FALSE(item.hidden);
 }
 
-// Test that there will be a separator after "no suggestions" if
-// SpellingServiceClient::SUGGEST is on.
-IN_PROC_BROWSER_TEST_F(SpellingMenuObserverTest, SeparatorAfterSuggestions) {
-  ForceSuggestMode();
-  InitMenu("jhhj", NULL);
-
-  // The test should see a top separator, "No spelling suggestions",
-  // "No more Google suggestions" (from SpellingService) and a separator
-  // as the first four items, then possibly more (not relevant here).
-  EXPECT_LT(4U, menu()->GetMenuSize());
-
-  MockRenderViewContextMenu::MockMenuItem item;
-  menu()->GetMenuItem(0, &item);
-  EXPECT_EQ(-1, item.command_id);
-  EXPECT_FALSE(item.enabled);
-  EXPECT_FALSE(item.hidden);
-
-  menu()->GetMenuItem(1, &item);
-  EXPECT_EQ(IDC_CONTENT_CONTEXT_SPELLING_SUGGESTION, item.command_id);
-  EXPECT_FALSE(item.enabled);
-  EXPECT_FALSE(item.hidden);
-
-  menu()->GetMenuItem(2, &item);
-  EXPECT_EQ(IDC_CONTENT_CONTEXT_NO_SPELLING_SUGGESTIONS, item.command_id);
-  EXPECT_FALSE(item.enabled);
-  EXPECT_FALSE(item.hidden);
-
-  menu()->GetMenuItem(3, &item);
-  EXPECT_EQ(-1, item.command_id);
-  EXPECT_FALSE(item.enabled);
-  EXPECT_FALSE(item.hidden);
-}
-
 // Test that we don't show "No more suggestions from Google" if the spelling
 // service is enabled and that there is only one suggestion.
 IN_PROC_BROWSER_TEST_F(SpellingMenuObserverTest,
@@ -378,8 +340,9 @@ IN_PROC_BROWSER_TEST_F(SpellingMenuObserverTest,
       menu()->GetBrowserContext(), SpellingServiceClient::SPELLCHECK));
   InitMenu("asdfkj", "asdf");
 
-  // The test should see a separator, a suggestion and another separator
-  // as the first two items, then possibly more (not relevant here).
+  // The test should see a separator, suggestion, "Add to dictionary",
+  // "Ask Google for suggestions",and a separator.

[Avi (use Gerrit), 2015/09/26 02:26:15]

space after a comma

[edwardjung, 2015/09/28 14:11:25]

Done.

+  // Possibly more items (not relevant here).

[Avi (use Gerrit), 2015/09/26 02:26:15]

Combine this comment line with the previous one.

[edwardjung, 2015/09/28 14:11:26]

Done.

   EXPECT_LT(3U, menu()->GetMenuSize());
 
   MockRenderViewContextMenu::MockMenuItem item;
@@ -394,6 +357,17 @@ IN_PROC_BROWSER_TEST_F(SpellingMenuObserverTest,
   EXPECT_FALSE(item.hidden);
 
   menu()->GetMenuItem(2, &item);
+  EXPECT_EQ(IDC_SPELLCHECK_ADD_TO_DICTIONARY, item.command_id);
+  EXPECT_TRUE(item.enabled);
+  EXPECT_FALSE(item.hidden);
+
+  menu()->GetMenuItem(3, &item);
+  EXPECT_EQ(IDC_CONTENT_CONTEXT_SPELLING_TOGGLE, item.command_id);
+  EXPECT_TRUE(item.enabled);
+  EXPECT_TRUE(item.checked);
+  EXPECT_FALSE(item.hidden);
+
+  menu()->GetMenuItem(4, &item);
   EXPECT_EQ(-1, item.command_id);
   EXPECT_FALSE(item.enabled);
   EXPECT_FALSE(item.hidden);
@@ -425,25 +399,19 @@ IN_PROC_BROWSER_TEST_F(SpellingMenuObserverTest,
 
   InitMenu("sjxdjiiiiii", NULL);
 
-  // The test should see "No spelling suggestions" (from system checker).
   // They should not see "No more Google suggestions" (from SpellingService) or

[Avi (use Gerrit), 2015/09/26 02:26:15]

Who is "they"? Maybe "There should not be xxxx nor xxxx".

[edwardjung, 2015/09/28 14:11:25]

Done.

   // a separator. The next 2 items should be "Add to Dictionary" followed
   // by "Ask Google for suggestions" which should be disabled.
   // TODO(rlp): add autocorrect here when it is functional.
-  EXPECT_LT(3U, menu()->GetMenuSize());
+  EXPECT_LT(2U, menu()->GetMenuSize());
 
   MockRenderViewContextMenu::MockMenuItem item;
   menu()->GetMenuItem(0, &item);
-  EXPECT_EQ(IDC_CONTENT_CONTEXT_NO_SPELLING_SUGGESTIONS, item.command_id);
-  EXPECT_FALSE(item.enabled);
-  EXPECT_FALSE(item.hidden);
-
-  menu()->GetMenuItem(1, &item);
   EXPECT_EQ(IDC_SPELLCHECK_ADD_TO_DICTIONARY, item.command_id);
   EXPECT_TRUE(item.enabled);
   EXPECT_FALSE(item.hidden);
 
-  menu()->GetMenuItem(2, &item);
+  menu()->GetMenuItem(1, &item);
   EXPECT_EQ(IDC_CONTENT_CONTEXT_SPELLING_TOGGLE, item.command_id);
   EXPECT_FALSE(item.enabled);
   EXPECT_FALSE(item.hidden);
@@ -456,8 +424,8 @@ IN_PROC_BROWSER_TEST_F(SpellingMenuObserverTest, SuggestionsForceTopSeparator) {
 
   // First case: Misspelled word, no suggestions, no spellcheck service.
   InitMenu("asdfkj", NULL);
-  // See SpellingMenuObserverTest.InitMenuWithMisspelledWord on why 4 items.
-  EXPECT_EQ(static_cast<size_t>(4), menu()->GetMenuSize());
+  // See SpellingMenuObserverTest.InitMenuWithMisspelledWord on why 3 items.
+  EXPECT_EQ(static_cast<size_t>(3), menu()->GetMenuSize());

[Avi (use Gerrit), 2015/09/26 02:26:15]

3u

[edwardjung, 2015/09/28 14:11:25]

Done.

   MockRenderViewContextMenu::MockMenuItem item;
   menu()->GetMenuItem(0, &item);
   EXPECT_NE(-1, item.command_id);
@@ -468,7 +436,7 @@ IN_PROC_BROWSER_TEST_F(SpellingMenuObserverTest, SuggestionsForceTopSeparator) {
   InitMenu("asdfkj", "asdf");
 
   // Expect at least separator and 4 default entries.
-  EXPECT_LT(static_cast<size_t>(5), menu()->GetMenuSize());
+  EXPECT_LT(static_cast<size_t>(4), menu()->GetMenuSize());

[Avi (use Gerrit), 2015/09/26 02:26:15]

4u

[edwardjung, 2015/09/28 14:11:25]

Done.

   // This test only cares that the first one is a separator.
   menu()->GetMenuItem(0, &item);
   EXPECT_EQ(-1, item.command_id);