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..38312a3ac49ba2261b680a7030108e788160785e 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());
   }
 }
@@ -622,51 +626,57 @@ IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest, CheckWebappInIframe) {
             InstallabilityCheckStatus::COMPLETE_NON_PROGRESSIVE_WEB_APP);
 }
 
-IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest,
-                       CheckPageWithManifestAndNoServiceWorker) {

[benwells, 2017/06/14 11:39:06]

Is the no service worker case tested at all?

[dominickn, 2017/06/15 04:16:07]

I can restore the no service worker test with the waiting disabled. Otherwise,
the manager waits indefinitely so it's really difficult to properly test the
no-worker but wait forever version.

Now we have tests for:

 - lazy worker, don't wait
 - lazy worker, wait
 - no worker, don't wait

-  // Just fetch the manifest. This should have no error.
+IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest, CheckLazyServiceWorker) {
   {
     base::RunLoop run_loop;
     std::unique_ptr<CallbackTester> tester(
         new CallbackTester(run_loop.QuitClosure()));
 
     NavigateAndRunInstallableManager(
-        tester.get(), GetManifestParams(),
-        "/banners/manifest_no_service_worker.html");
+        tester.get(), GetWebAppParams(),
+        "/banners/lazy_service_worker_test_page.html");
     run_loop.Run();
 
     EXPECT_FALSE(tester->manifest().IsEmpty());
     EXPECT_FALSE(tester->manifest_url().is_empty());
-
-    EXPECT_TRUE(tester->primary_icon_url().is_empty());
-    EXPECT_EQ(nullptr, tester->primary_icon());
-    EXPECT_FALSE(tester->is_installable());
+    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::NOT_COMPLETED);
-  }
+    EXPECT_EQ(GetStatus(),
+              InstallabilityCheckStatus::COMPLETE_PROGRESSIVE_WEB_APP);
 
-  // Fetch the full criteria should fail.
-  {
-    base::RunLoop run_loop;
-    std::unique_ptr<CallbackTester> tester(
-        new CallbackTester(run_loop.QuitClosure()));
+    // Verify that the returned state matches manager internal state.
+    InstallableManager* manager = GetManager();
 
-    RunInstallableManager(tester.get(), GetWebAppParams());
-    run_loop.Run();
+    EXPECT_FALSE(manager->manifest().IsEmpty());
+    EXPECT_FALSE(manager->manifest_url().is_empty());
+    EXPECT_TRUE(manager->is_installable());
+    EXPECT_EQ(1u, manager->icons_.size());
+    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->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());
+  }
 
-    EXPECT_FALSE(tester->manifest().IsEmpty());
-    EXPECT_FALSE(tester->manifest_url().is_empty());
+  {
+    // Check that a subsequent navigation resets state.
+    ui_test_utils::NavigateToURL(browser(), GURL("about:blank"));
+    InstallableManager* manager = GetManager();
 
-    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);
+    EXPECT_TRUE(manager->manifest().IsEmpty());
+    EXPECT_TRUE(manager->manifest_url().is_empty());
+    EXPECT_FALSE(manager->is_installable());
+    EXPECT_TRUE(manager->icons_.empty());
+    EXPECT_EQ(NO_ERROR_DETECTED, manager->manifest_error());
+    EXPECT_EQ(NO_ERROR_DETECTED, manager->valid_manifest_error());
+    EXPECT_EQ(NO_ERROR_DETECTED, manager->worker_error());
+    EXPECT_TRUE(manager->tasks_.empty());
   }
 }