Allow banners to trigger on sites which don't register a service worker onload.

chrome/browser/installable/installable_manager_browsertest.cc

Index: chrome/browser/installable/installable_manager_browsertest.cc
diff --git a/chrome/browser/installable/installable_manager_browsertest.cc b/chrome/browser/installable/installable_manager_browsertest.cc
index 58cabb85b4996b4dd1ff2f2960ffff70113aa860..65b89f449d0f4c4a93970345886b2acc32006234 100644
--- a/chrome/browser/installable/installable_manager_browsertest.cc
+++ b/chrome/browser/installable/installable_manager_browsertest.cc
@@ -214,7 +214,8 @@ IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest,
   EXPECT_FALSE(manager->is_installable());
 
   EXPECT_EQ(NO_ERROR_DETECTED, manager->manifest_error());
-  EXPECT_EQ(NO_ERROR_DETECTED, manager->installable_error());
+  EXPECT_EQ(NO_ERROR_DETECTED, manager->valid_manifest_error());
+  EXPECT_EQ(NO_ERROR_DETECTED, manager->worker_error());
   EXPECT_TRUE(manager->tasks_.empty());
 }
 
@@ -543,7 +544,8 @@ IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest, CheckWebapp) {
     EXPECT_FALSE((manager->icon_url(kPrimaryIconParams).is_empty()));
     EXPECT_NE(nullptr, (manager->icon(kPrimaryIconParams)));
     EXPECT_EQ(NO_ERROR_DETECTED, manager->manifest_error());
-    EXPECT_EQ(NO_ERROR_DETECTED, manager->installable_error());
+    EXPECT_EQ(NO_ERROR_DETECTED, manager->valid_manifest_error());
+    EXPECT_EQ(NO_ERROR_DETECTED, manager->worker_error());
     EXPECT_EQ(NO_ERROR_DETECTED, (manager->icon_error(kPrimaryIconParams)));
     EXPECT_TRUE(manager->tasks_.empty());
   }
@@ -579,7 +581,8 @@ IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest, CheckWebapp) {
     EXPECT_FALSE((manager->icon_url(kPrimaryIconParams).is_empty()));
     EXPECT_NE(nullptr, (manager->icon(kPrimaryIconParams)));
     EXPECT_EQ(NO_ERROR_DETECTED, manager->manifest_error());
-    EXPECT_EQ(NO_ERROR_DETECTED, manager->installable_error());
+    EXPECT_EQ(NO_ERROR_DETECTED, manager->valid_manifest_error());
+    EXPECT_EQ(NO_ERROR_DETECTED, manager->worker_error());
     EXPECT_EQ(NO_ERROR_DETECTED, (manager->icon_error(kPrimaryIconParams)));
     EXPECT_TRUE(manager->tasks_.empty());
   }
@@ -594,7 +597,8 @@ IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest, CheckWebapp) {
     EXPECT_FALSE(manager->is_installable());
     EXPECT_TRUE(manager->icons_.empty());
     EXPECT_EQ(NO_ERROR_DETECTED, manager->manifest_error());
-    EXPECT_EQ(NO_ERROR_DETECTED, manager->installable_error());
+    EXPECT_EQ(NO_ERROR_DETECTED, manager->valid_manifest_error());
+    EXPECT_EQ(NO_ERROR_DETECTED, manager->worker_error());
     EXPECT_TRUE(manager->tasks_.empty());
   }
 }
@@ -647,13 +651,15 @@ IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest,
     EXPECT_EQ(GetStatus(), InstallabilityCheckStatus::NOT_COMPLETED);
   }
 
-  // Fetch the full criteria should fail.
+  // Fetching the full criteria should fail if we don't wait for the worker.
   {
     base::RunLoop run_loop;
     std::unique_ptr<CallbackTester> tester(
         new CallbackTester(run_loop.QuitClosure()));
 
-    RunInstallableManager(tester.get(), GetWebAppParams());
+    InstallableParams params = GetWebAppParams();
+    params.wait_for_worker = false;
+    RunInstallableManager(tester.get(), params);
     run_loop.Run();
 
     EXPECT_FALSE(tester->manifest().IsEmpty());
@@ -670,6 +676,59 @@ IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest,
   }
 }
 
+IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest,
+                       CheckLazyServiceWorkerFailsWithoutWaiting) {
+  base::RunLoop run_loop;
+  std::unique_ptr<CallbackTester> tester(
+      new CallbackTester(run_loop.QuitClosure()));
+
+  // This page registers its worker after a short delay. Verify the check
+  // deterministically fails if we don't wait for the worker.
+  InstallableParams params = GetWebAppParams();
+  params.wait_for_worker = false;
+  NavigateAndRunInstallableManager(
+      tester.get(), params,
+      "/banners/lazy_service_worker_test_page.html?swdelay=500");

[benwells, 2017/06/15 04:42:43]

Is the 500 important here? Just asking as 300 changed to 500 in this version, am
hoping we don't have the test results depend on this value.

[dominickn, 2017/06/15 04:50:12]

In my empirical testing, 300ms was originally long enough for both Android and
desktop. But it started flaking on desktop when I re-added this test (it would
pass the check when it should have failed), hence the extension to 500.

There's not really any other sane way to have a test for this. I could do a lot
more mocking to have the manager quit when it gets to the waiting for SW step,
then restart it, but that's getting to be a lot of test-only code that could be
quite fragile. :(

+  run_loop.Run();
+
+  EXPECT_FALSE(tester->manifest().IsEmpty());
+  EXPECT_FALSE(tester->manifest_url().is_empty());
+
+  EXPECT_FALSE(tester->primary_icon_url().is_empty());
+  EXPECT_NE(nullptr, tester->primary_icon());
+  EXPECT_FALSE(tester->is_installable());
+  EXPECT_TRUE(tester->badge_icon_url().is_empty());
+  EXPECT_EQ(nullptr, tester->badge_icon());
+  EXPECT_EQ(NO_MATCHING_SERVICE_WORKER, tester->error_code());
+  EXPECT_EQ(GetStatus(),
+            InstallabilityCheckStatus::COMPLETE_NON_PROGRESSIVE_WEB_APP);
+}
+
+IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest,
+                       CheckLazyServiceWorkerPassesWhenWaiting) {
+  base::RunLoop run_loop;
+  std::unique_ptr<CallbackTester> tester(
+      new CallbackTester(run_loop.QuitClosure()));
+
+  // This page registers its worker after a short delay. Verify the check
+  // deterministically passes if we do wait for the worker.
+  NavigateAndRunInstallableManager(
+      tester.get(), GetWebAppParams(),
+      "/banners/lazy_service_worker_test_page.html?swdelay=500");
+  run_loop.Run();
+
+  EXPECT_FALSE(tester->manifest().IsEmpty());
+  EXPECT_FALSE(tester->manifest_url().is_empty());
+  EXPECT_FALSE(tester->primary_icon_url().is_empty());
+  EXPECT_NE(nullptr, tester->primary_icon());
+  EXPECT_TRUE(tester->is_installable());
+  EXPECT_TRUE(tester->badge_icon_url().is_empty());
+  EXPECT_EQ(nullptr, tester->badge_icon());
+  EXPECT_EQ(NO_ERROR_DETECTED, tester->error_code());
+  EXPECT_EQ(GetStatus(),
+            InstallabilityCheckStatus::COMPLETE_PROGRESSIVE_WEB_APP);
+}
+
 IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest,
                        CheckPageWithNoServiceWorkerFetchHandler) {
   base::RunLoop run_loop;