[Extensions] Fixed a broken and flaky test.

The ExtensionApiTest.ChromeRuntimeReloadDisable test was broken due
to underlying changes in the code that terminates extensions that
reload themselves too frequently. It was also flaky.

The new version is fixed and is also no longer flaky.

Bug: 366181
Change-Id: I8b11cb5b2db1ee17885e3270cf41037faebb0923
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2150164
Commit-Queue: David Bertoni <dbertoni@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#762108}

chrome/browser/extensions/api/runtime/chrome_runtime_api_delegate.cc

@@ -56,7 +56,7 @@ const char kUpdateThrottled[] = "throttled";
 const char kUpdateNotFound[] = "no_update";
 const char kUpdateFound[] = "update_available";
 
-// If an extension reloads itself within this many miliseconds of reloading
+// If an extension reloads itself within this many milliseconds of reloading
 // itself, the reload is considered suspiciously fast.
 const int kFastReloadTime = 10000;
 
@@ -64,13 +64,6 @@ const int kFastReloadTime = 10000;
 // extensions for ease of testing.
 const int kUnpackedFastReloadTime = 1000;
 
-// After this many suspiciously fast consecutive reloads, an extension will get
-// disabled.
-const int kFastReloadCount = 5;
-
-// Same as above, but we increase the fast reload count for unpacked extensions.
-const int kUnpackedFastReloadCount = 30;
-
 // A holder class for the policy we use for exponential backoff of update check
 // requests.
 class BackoffPolicy {
@@ -184,14 +177,14 @@ void ChromeRuntimeAPIDelegate::ReloadExtension(
       extensions::ExtensionRegistry::Get(browser_context_)
           ->GetInstalledExtension(extension_id);
   int fast_reload_time = kFastReloadTime;
-  int fast_reload_count = kFastReloadCount;
+  int fast_reload_count = extensions::RuntimeAPI::kFastReloadCount;
 
   // If an extension is unpacked, we allow for a faster reload interval
   // and more fast reload attempts before terminating the extension.
   // This is intended to facilitate extension testing for developers.
   if (extensions::Manifest::IsUnpackedLocation(extension->location())) {
     fast_reload_time = kUnpackedFastReloadTime;
-    fast_reload_count = kUnpackedFastReloadCount;
+    fast_reload_count = extensions::RuntimeAPI::kUnpackedFastReloadCount;
   }
 
   std::pair<base::TimeTicks, int>& reload_info =

chrome/browser/extensions/api/runtime/runtime_apitest.cc

@@ -147,34 +147,38 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest,
 }
 
 // Tests that an extension calling chrome.runtime.reload() repeatedly
-// will eventually be disabled.
-// This test is flaky: crbug.com/366181
-IN_PROC_BROWSER_TEST_F(ExtensionApiTest, DISABLED_ChromeRuntimeReloadDisable) {
+// will eventually be terminated.
+IN_PROC_BROWSER_TEST_F(ExtensionApiTest, ExtensionTerminatedForRapidReloads) {
   ExtensionRegistry* registry = ExtensionRegistry::Get(profile());
-  const char kManifest[] =
-      "{"
-      "  \"name\": \"reload\","
-      "  \"version\": \"1.0\","
-      "  \"background\": {"
-      "    \"scripts\": [\"background.js\"]"
-      "  },"
-      "  \"manifest_version\": 2"
-      "}";
+  static constexpr char kManifest[] = R"(
+      {
+        "name": "reload",
+        "version": "1.0",
+        "background": {
+          "scripts": ["background.js"]
+        },
+        "manifest_version": 2
+      })";
 
   TestExtensionDir dir;
   dir.WriteManifest(kManifest);
-  dir.WriteFile(FILE_PATH_LITERAL("background.js"), "console.log('loaded');");
+  dir.WriteFile(FILE_PATH_LITERAL("background.js"),
+                "chrome.test.sendMessage('ready');");
 
-  const Extension* extension = LoadExtension(dir.UnpackedPath());
+  // Use a packed extension, since this is the scenario we are interested in
+  // testing. Unpacked extensions are allowed more reloads within the allotted
+  // time, to avoid interfering with the developer work flow.
+  const Extension* extension = LoadExtension(dir.Pack());
   ASSERT_TRUE(extension);
   const std::string extension_id = extension->id();
 
-  // Somewhat arbitrary upper limit of 30 iterations. If the extension manages
-  // to reload itself that often without being terminated, the test fails
+  // The current limit for fast reload is 5, so the loop limit of 10
+  // be enough to trigger termination. If the extension manages to
+  // reload itself that often without being terminated, the test fails
   // anyway.
-  for (int i = 0; i < 30; i++) {
+  for (int i = 0; i < RuntimeAPI::kFastReloadCount + 1; i++) {
+    ExtensionTestMessageListener ready_listener_reload("ready", false);
     TestExtensionRegistryObserver unload_observer(registry, extension_id);
-    TestExtensionRegistryObserver load_observer(registry, extension_id);
     ASSERT_TRUE(ExecuteScriptInBackgroundPageNoWait(
         extension_id, "chrome.runtime.reload();"));
     unload_observer.WaitForExtensionUnloaded();
@@ -184,11 +188,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, DISABLED_ChromeRuntimeReloadDisable) {
                                    ExtensionRegistry::TERMINATED)) {
       break;
     } else {
-      load_observer.WaitForExtensionLoaded();
-      // We need to let other registry observers handle the notification to
-      // finish initialization
-      base::RunLoop().RunUntilIdle();
-      WaitForExtensionViewsToLoad();
+      EXPECT_TRUE(ready_listener_reload.WaitUntilSatisfied());
     }
   }
   ASSERT_TRUE(

extensions/browser/api/runtime/runtime_api.cc

@@ -164,6 +164,9 @@ BrowserContextKeyedAPIFactory<RuntimeAPI>* RuntimeAPI::GetFactoryInstance() {
   return g_factory.Pointer();
 }
 
+constexpr int RuntimeAPI::kFastReloadCount;
+constexpr int RuntimeAPI::kUnpackedFastReloadCount;
+
 // static
 void RuntimeAPI::RegisterPrefs(PrefRegistrySimple* registry) {
   registry->RegisterBooleanPref(kPrefLastRestartWasDueToDelayedRestartApi,

extensions/browser/api/runtime/runtime_api.h

@@ -72,6 +72,14 @@ class RuntimeAPI : public BrowserContextKeyedAPI,
     SUCCESS_RESTART_SCHEDULED,
   };
 
+  // After this many suspiciously fast consecutive reloads, an extension will
+  // get disabled.
+  static constexpr int kFastReloadCount = 5;
+
+  // Same as above, but we increase the fast reload count for unpacked
+  // extensions.
+  static constexpr int kUnpackedFastReloadCount = 30;
+
   static BrowserContextKeyedAPIFactory<RuntimeAPI>* GetFactoryInstance();
 
   static void RegisterPrefs(PrefRegistrySimple* registry);