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

chrome/browser/banners/app_banner_manager.cc

Index: chrome/browser/banners/app_banner_manager.cc
diff --git a/chrome/browser/banners/app_banner_manager.cc b/chrome/browser/banners/app_banner_manager.cc
index 9524f7eff2093ecb1489a53631f279852a4d1392..976db92ce6a877047bac558b1e39f1fa499e5a9f 100644
--- a/chrome/browser/banners/app_banner_manager.cc
+++ b/chrome/browser/banners/app_banner_manager.cc
@@ -110,6 +110,7 @@ void AppBannerManager::RequestAppBanner(const GURL& validated_url,
   if (binding_.is_bound())
     binding_.Close();
 
+  UpdateState(State::PENDING_MANIFEST);
   manager_->GetData(
       ParamsToGetManifest(),
       base::Bind(&AppBannerManager::OnDidGetManifest, GetWeakPtr()));
@@ -206,6 +207,7 @@ bool AppBannerManager::IsWebAppInstalled(
 }
 
 void AppBannerManager::OnDidGetManifest(const InstallableData& data) {
+  UpdateState(State::ACTIVE);
   if (data.error_code != NO_ERROR_DETECTED) {
     ReportStatus(web_contents(), data.error_code);
     Stop();
@@ -237,6 +239,11 @@ InstallableParams AppBannerManager::ParamsToPerformInstallableCheck() {
   params.check_installable = true;
   params.fetch_valid_primary_icon = true;
 
+  // Don't wait for the worker if we're in debug mode. However, we want to
+  // ignore whether the bypass flag is on for this purpose (otherwise we won't
+  // wait for the worker in that instance).

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

Maybe it's just late but this confuses me. 

Edit: After seeing the rest of the patch I understand now. I think just renaming
is_debug_mode_ to from_devools_ will make it pretty obvious why you just want to
check that and not the full IsDebugMode.

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

Done.

+  params.wait_for_worker = !is_debug_mode_;
+
   return params;
 }
 
@@ -245,6 +252,7 @@ void AppBannerManager::PerformInstallableCheck() {
     return;
 
   // Fetch and verify the other required information.
+  UpdateState(State::PENDING_INSTALLABLE_CHECK);
   manager_->GetData(ParamsToPerformInstallableCheck(),
                     base::Bind(&AppBannerManager::OnDidPerformInstallableCheck,
                                GetWeakPtr()));
@@ -252,6 +260,7 @@ void AppBannerManager::PerformInstallableCheck() {
 
 void AppBannerManager::OnDidPerformInstallableCheck(
     const InstallableData& data) {
+  UpdateState(State::ACTIVE);
   if (data.is_installable)
     TrackDisplayEvent(DISPLAY_EVENT_WEB_APP_BANNER_REQUESTED);
 
@@ -327,14 +336,15 @@ void AppBannerManager::Stop() {
              !has_sufficient_engagement_) {
     TrackDisplayEvent(DISPLAY_EVENT_NOT_VISITED_ENOUGH);
     ReportStatus(web_contents(), INSUFFICIENT_ENGAGEMENT);
+  } else if (state_ == State::PENDING_MANIFEST) {
+    ReportStatus(web_contents(), WAITING_FOR_MANIFEST);
+  } else if (state_ == State::PENDING_INSTALLABLE_CHECK) {
+    ReportStatus(web_contents(), WAITING_FOR_INSTALLABLE_CHECK);
   }
 
   // In every non-debug run through the banner pipeline, we should have called
-  // ReportStatus() and set need_to_log_status_ to false. The only case where
-  // we don't is if we're still active and waiting for a callback from the
-  // InstallableManager (e.g. the renderer crashes or the browser is shutting
-  // down). These situations are explicitly not logged.
-  DCHECK(!need_to_log_status_ || is_active());
+  // ReportStatus() and set need_to_log_status_ to false.
+  DCHECK(!need_to_log_status_);
 
   weak_factory_.InvalidateWeakPtrs();
   binding_.Close();
@@ -342,7 +352,6 @@ void AppBannerManager::Stop() {
   event_.reset();
 
   UpdateState(State::COMPLETE);
-  need_to_log_status_ = false;
   has_sufficient_engagement_ = false;
 }