[Extensions] Re-enable test CommandsApiTest.PageAction

chrome/browser/extensions/extension_keybinding_apitest.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/command_line.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/extensions/active_tab_permission_granter.h"
 #include "chrome/browser/extensions/api/commands/command_service.h"
 #include "chrome/browser/extensions/browser_action_test_util.h"
 #include "chrome/browser/extensions/extension_action.h"
 #include "chrome/browser/extensions/extension_action_manager.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/sessions/session_tab_helper.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_command_controller.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/interactive_test_utils.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test_utils.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/feature_switch.h"
 #include "extensions/common/manifest_constants.h"
 #include "extensions/common/permissions/permissions_data.h"
+#include "extensions/test/extension_test_message_listener.h"
 #include "extensions/test/result_catcher.h"
 
 using content::WebContents;
 
 namespace extensions {
 
 namespace {
 // This extension ID is used for tests require a stable ID over multiple
 // extension installs.
 const char kId[] = "pgoakhfeplldmjheffidklpoklkppipp";
@@ skipping 123 matching lines @@
   result = false;
   ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
       tab,
       "setInterval(function(){"
       "  if(document.body.bgColor == 'blue'){"
       "    window.domAutomationController.send(true)}}, 100)",
       &result));
   ASSERT_TRUE(result);
 }
 
-// Flaky on linux and chromeos, http://crbug.com/165825
-#if defined(OS_MACOSX) || defined(OS_WIN)
-#define MAYBE_PageAction PageAction
-#else
-#define MAYBE_PageAction DISABLED_PageAction
-#endif
-IN_PROC_BROWSER_TEST_F(CommandsApiTest, MAYBE_PageAction) {
+IN_PROC_BROWSER_TEST_F(CommandsApiTest, PageAction) {
   ASSERT_TRUE(test_server()->Start());
   ASSERT_TRUE(RunExtensionTest("keybinding/page_action")) << message_;
   const Extension* extension = GetSingleLoadedExtension();
   ASSERT_TRUE(extension) << message_;
 
   {
     // Load a page, the extension will detect the navigation and request to show
     // the page action icon.
     ResultCatcher catcher;
     ui_test_utils::NavigateToURL(
         browser(), test_server()->GetURL("files/extensions/test_file.txt"));
     ASSERT_TRUE(catcher.GetNextResult());
   }
 
   // Make sure it appears and is the right one.
   ASSERT_TRUE(WaitForPageActionVisibilityChangeTo(1));
   int tab_id = SessionTabHelper::FromWebContents(
       browser()->tab_strip_model()->GetActiveWebContents())->session_id().id();
   ExtensionAction* action =
       ExtensionActionManager::Get(browser()->profile())->
       GetPageAction(*extension);
   ASSERT_TRUE(action);
   EXPECT_EQ("Make this page red", action->GetTitle(tab_id));
 
+  ExtensionTestMessageListener test_listener(false);  // Won't reply.
+  test_listener.set_extension_id(extension->id());
+
   // Activate the shortcut (Alt+Shift+F).
   ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
       browser(), ui::VKEY_F, false, true, true, false));
 
-  // Verify the command worked (the page action turns the page red).
-  WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
-  bool result = false;
-  ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
-      tab,
-      "setInterval(function(){"
-      "  if(document.body.bgColor == 'red'){"
-      "    window.domAutomationController.send(true)}}, 100)",
-      &result));
-  ASSERT_TRUE(result);
+  test_listener.WaitUntilSatisfied();
+  EXPECT_EQ("clicked", test_listener.message());

[Finnur, 2015/01/12 10:21:40]

Love this change. So much so, in fact, that I'd like to see the background
changing go away completely. For example, there are other occurrences in this
file. Would you be willing to convert those also? If so, feel free to do so as a
follow-up.

[Devlin, 2015/01/12 23:06:23]

Happy to do so, but will do in a follow-up so that if the test happens to still
be flaky, reverting doesn't undo all of it. :)

 }
 
 IN_PROC_BROWSER_TEST_F(CommandsApiTest, PageActionKeyUpdated) {
   ASSERT_TRUE(test_server()->Start());
   ASSERT_TRUE(RunExtensionTest("keybinding/page_action")) << message_;
   const Extension* extension = GetSingleLoadedExtension();
   ASSERT_TRUE(extension) << message_;
 
   CommandService* command_service = CommandService::Get(browser()->profile());
   // Simulate the user setting the keybinding to Alt+Shift+G.
   command_service->UpdateKeybindingPrefs(
       extension->id(), manifest_values::kPageActionCommandEvent, kAltShiftG);
 
   {
     // Load a page. The extension will detect the navigation and request to show
     // the page action icon.
     ResultCatcher catcher;
     ui_test_utils::NavigateToURL(
         browser(), test_server()->GetURL("files/extensions/test_file.txt"));
     ASSERT_TRUE(catcher.GetNextResult());
   }
 
-  // Activate the shortcut to make the page red.
+  ExtensionTestMessageListener test_listener(false);  // Won't reply.
+  test_listener.set_extension_id(extension->id());
+
+  // Activate the shortcut.
   ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
       browser(), ui::VKEY_G, false, true, true, false));
 
-  // Verify the command worked (the page action turns the page red).
-  WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
-  bool result = false;
-  ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
-      tab,
-      std::string("setInterval(function() {") +
-      "  if (document.body.bgColor == 'red') {" +
-      "    window.domAutomationController.send(true)}}, 100)",
-      &result));
-  ASSERT_TRUE(result);
+  test_listener.WaitUntilSatisfied();
+  EXPECT_EQ("clicked", test_listener.message());
 }
 
 // This test validates that the getAll query API function returns registered
 // commands as well as synthesized ones and that inactive commands (like the
 // synthesized ones are in nature) have no shortcuts.
 IN_PROC_BROWSER_TEST_F(CommandsApiTest, SynthesizedCommand) {
   ASSERT_TRUE(test_server()->Start());
   ASSERT_TRUE(RunExtensionTest("keybinding/synthesized")) << message_;
 }
 
@@ skipping 694 matching lines @@
 #define MAYBE_ChromeOSConversions DISABLED_ChromeOSConversions
 #else
 #define MAYBE_ChromeOSConversions ChromeOSConversions
 #endif
 IN_PROC_BROWSER_TEST_F(CommandsApiTest, MAYBE_ChromeOSConversions) {
   RunChromeOSConversionTest("keybinding/chromeos_conversions");
 }
 #endif  // OS_CHROMEOS
 
 }  // namespace extensions