The AllUrlsApiTest.WhitelistedExtension test should no longer flake.

chrome/browser/extensions/all_urls_apitest.cc

 // Copyright (c) 2011 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/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/test/base/test_switches.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/crx_file/id_util.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extensions_client.h"
 #include "extensions/test/extension_test_message_listener.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 
 namespace extensions {
 
 namespace {
 const std::string kAllUrlsTarget = "/extensions/api_test/all_urls/index.html";
 }
 
-class AllUrlsApiTest : public ExtensionApiTest {
+class AllUrlsApiTest : public ExtensionApiTest,
+                       public ExtensionRegistryObserver {
  protected:
-  AllUrlsApiTest() {}
+  AllUrlsApiTest() : wait_until_reload_(false),
+                     content_script_is_reloaded_(false),
+                     execute_script_is_reloaded_(false) {}
   ~AllUrlsApiTest() override {}
 
   const Extension* content_script() const { return content_script_.get(); }
   const Extension* execute_script() const { return execute_script_.get(); }
 
+  // ExtensionRegistryObserver implementation
+  void OnExtensionLoaded(
+      content::BrowserContext* /*browser_context*/,

[Finnur, 2015/11/12 15:27:19]

Nit: convert to:

   content::BrowserContext*,  // |browser_context|,

Although as a comment this isn't useful. I'd just remove it.

+      const Extension* extension) override {
+    if (!wait_until_reload_)
+      return;
+
+    if (extension->id() == content_script_->id())
+      content_script_is_reloaded_ = true;
+    else if (extension->id() == execute_script_->id())
+      execute_script_is_reloaded_ = true;
+    if (content_script_is_reloaded_ && execute_script_is_reloaded_) {
+      base::MessageLoop::current()->QuitWhenIdle();
+      wait_until_reload_ = false;
+    }
+  }
+
   void WhitelistExtensions() {
+    ScopedObserver<ExtensionRegistry, ExtensionRegistryObserver> observer(this);
+    observer.Add(ExtensionRegistry::Get(browser()->profile()));
+
     ExtensionsClient::ScriptingWhitelist whitelist;
     whitelist.push_back(content_script_->id());
     whitelist.push_back(execute_script_->id());
     ExtensionsClient::Get()->SetScriptingWhitelist(whitelist);
     // Extensions will have certain permissions withheld at initialization if
     // they aren't whitelisted, so we need to reload them.
+    content_script_is_reloaded_ = false;
+    execute_script_is_reloaded_ = false;
+    wait_until_reload_ = true;
     extension_service()->ReloadExtension(content_script_->id());
     extension_service()->ReloadExtension(execute_script_->id());
+    base::MessageLoop::current()->Run();
   }
 
  private:
   void SetUpOnMainThread() override {
     ExtensionApiTest::SetUpOnMainThread();
     base::FilePath data_dir = test_data_dir_.AppendASCII("all_urls");
     content_script_ = LoadExtension(data_dir.AppendASCII("content_script"));
     ASSERT_TRUE(content_script_);
     execute_script_ = LoadExtension(data_dir.AppendASCII("execute_script"));
     ASSERT_TRUE(execute_script_);
   }
 
   scoped_refptr<const Extension> content_script_;
   scoped_refptr<const Extension> execute_script_;
 
+  bool wait_until_reload_;
+  bool content_script_is_reloaded_;
+  bool execute_script_is_reloaded_;
+
   DISALLOW_COPY_AND_ASSIGN(AllUrlsApiTest);
 };
 
-#if (defined(OS_WIN) && !defined(NDEBUG)) || defined(OS_CHROMEOS) || \
-    (defined(OS_MACOSX) && defined(ADDRESS_SANITIZER))
-// http://crbug.com/174341
-#define MAYBE_WhitelistedExtension DISABLED_WhitelistedExtension
-#else
-#define MAYBE_WhitelistedExtension WhitelistedExtension
-#endif
-IN_PROC_BROWSER_TEST_F(AllUrlsApiTest, MAYBE_WhitelistedExtension) {
+IN_PROC_BROWSER_TEST_F(AllUrlsApiTest, WhitelistedExtension) {
 #if defined(OS_WIN) && defined(USE_ASH)
   // Disable this test in Metro+Ash for now (http://crbug.com/262796).
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kAshBrowserTests))
     return;
 #endif
 
   WhitelistExtensions();
 
   std::string url;
@@ skipping 49 matching lines @@
   GURL page_url = embedded_test_server()->GetURL(kAllUrlsTarget);
   ExtensionTestMessageListener listener1a("content script: " + page_url.spec(),
                                           false);
   ExtensionTestMessageListener listener1b("execute: " + page_url.spec(), false);
   ui_test_utils::NavigateToURL(browser(), page_url);
   ASSERT_TRUE(listener1a.WaitUntilSatisfied());
   ASSERT_TRUE(listener1b.WaitUntilSatisfied());
 }
 
 // Disabled because sometimes bystander doesn't load.
 // TODO(devlin): Why?

[Finnur, 2015/11/12 15:27:19]

Can it be enabled now?

 IN_PROC_BROWSER_TEST_F(AllUrlsApiTest,
                        DISABLED_WhitelistedExtensionRunsOnExtensionPages) {
   WhitelistExtensions();
   const Extension* bystander =
       LoadExtension(test_data_dir_.AppendASCII("all_urls")
                                   .AppendASCII("bystander"));
   ASSERT_TRUE(bystander);
 
   // TODO(devlin): This test should probably go in the WhitelistedExtension test
   // above, but that one has a bunch of disableds, so it wouldn't be very
   // useful.
   GURL url(bystander->GetResourceURL("page.html"));
   ExtensionTestMessageListener listenerA(
       "content script: " + url.spec(), false);
   ExtensionTestMessageListener listenerB("execute: " + url.spec(), false);
   ui_test_utils::NavigateToURL(browser(), GURL(url));
   ASSERT_TRUE(listenerA.WaitUntilSatisfied());
   ASSERT_TRUE(listenerB.WaitUntilSatisfied());
 }
 
 }  // namespace extensions