Fix flaky test ContentVerifierPolicyTest.PolicyCorruptedOnStartup

chrome/browser/extensions/content_verifier_browsertest.cc

 // Copyright 2014 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 <list>
 #include <set>
 #include <string>
 
 #include "base/callback_helpers.h"
 #include "base/macros.h"
@@ skipping 689 matching lines @@
 
  protected:
   // The id of the extension we want to have force-installed.
   std::string id_ = "npnbmohejbjohgpjnmjagbafnjhkmgko";
 
  private:
   policy::MockConfigurationPolicyProvider policy_provider_;
   DownloaderTestDelegate downloader_;
 };
 
-// Disabled due to flakiness (crbug.com/668067)
 // We want to test what happens at startup with a corroption-disabled policy
 // force installed extension. So we set that up in the PRE test here.
 IN_PROC_BROWSER_TEST_F(ContentVerifierPolicyTest,
-                       DISABLED_PRE_PolicyCorruptedOnStartup) {
+                       PRE_PolicyCorruptedOnStartup) {
   ExtensionRegistry* registry = ExtensionRegistry::Get(profile());
   RegistryObserver registry_observer(registry);
 
   // Wait for the extension to be installed by policy we set up in
   // SetUpInProcessBrowserTestFixture.
   if (!registry->GetInstalledExtension(id_)) {
     EXPECT_TRUE(registry_observer.WaitForInstall(id_));
   }
 
   // Simulate corruption of the extension so that we can test what happens
   // at startup in the non-PRE test.
   ExtensionSystem* system = ExtensionSystem::Get(profile());
   ContentVerifier* verifier = system->content_verifier();
   verifier->VerifyFailed(id_, ContentVerifyJob::HASH_MISMATCH);
   EXPECT_TRUE(registry_observer.WaitForUnload(id_));
   ExtensionPrefs* prefs = ExtensionPrefs::Get(profile());
   int reasons = prefs->GetDisableReasons(id_);
   EXPECT_TRUE(reasons & Extension::DISABLE_CORRUPTED);
 }
 
-// Disabled due to flakiness (crbug.com/668067)
 // Now actually test what happens on the next startup after the PRE test above.
-IN_PROC_BROWSER_TEST_F(ContentVerifierPolicyTest,
-                       DISABLED_PolicyCorruptedOnStartup) {
-  // Expect that the extension is still disabled for corruption.
+IN_PROC_BROWSER_TEST_F(ContentVerifierPolicyTest, PolicyCorruptedOnStartup) {
+  // Depdending on timing, the extension may have already been reinstalled
+  // between SetUpInProcessBrowserTestFixture and now (usually not during local
+  // testing on a developer machine, but sometimes on a heavily loaded system
+  // such as the build waterfall / trybots). If the reinstall didn't already
+  // happen, wait for it.
   ExtensionPrefs* prefs = ExtensionPrefs::Get(profile());
   int reasons = prefs->GetDisableReasons(id_);
-  EXPECT_TRUE(reasons & Extension::DISABLE_CORRUPTED);
-
-  // Now expect that it gets reinstalled (because a job should have been kicked
-  // off by the installed extension loading code).
-  ExtensionRegistry* registry = ExtensionRegistry::Get(profile());
-  RegistryObserver registry_observer(registry);
-  EXPECT_TRUE(registry_observer.WaitForInstall(id_));
-  reasons = prefs->GetDisableReasons(id_);
-  EXPECT_FALSE(reasons & Extension::DISABLE_CORRUPTED);
+  if (reasons & Extension::DISABLE_CORRUPTED) {
+    ExtensionRegistry* registry = ExtensionRegistry::Get(profile());
+    RegistryObserver registry_observer(registry);
+    EXPECT_TRUE(registry_observer.WaitForInstall(id_));
+    reasons = prefs->GetDisableReasons(id_);

[Devlin, 2016/11/29 22:46:38]

Could we end the test with checking the disable reasons in all cases, and maybe
checking that it's in the enabled_extensions set?  i.e., moving this line and
the one below outside the if and adding another check for the registry.

[asargent_no_longer_on_chrome, 2016/11/30 01:02:05]

Good suggestion - done. 

+    EXPECT_FALSE(reasons & Extension::DISABLE_CORRUPTED);
+  }
 }
 
 }  // namespace extensions