Fix flaky test ContentVerifierPolicyTest.PolicyCorruptedOnStartup

chrome/browser/extensions/content_verifier_browsertest.cc

Index: chrome/browser/extensions/content_verifier_browsertest.cc
diff --git a/chrome/browser/extensions/content_verifier_browsertest.cc b/chrome/browser/extensions/content_verifier_browsertest.cc
index 645875d85ea483c839fbfd1eef33e050467d10cb..82a54646df80b23d3072db33de1c3f4aeff99d1c 100644
--- a/chrome/browser/extensions/content_verifier_browsertest.cc
+++ b/chrome/browser/extensions/content_verifier_browsertest.cc
@@ -707,11 +707,10 @@ class ContentVerifierPolicyTest : public ContentVerifierTest {
   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);
 
@@ -732,22 +731,22 @@ IN_PROC_BROWSER_TEST_F(ContentVerifierPolicyTest,
   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