Check if a PWA is installed before checking the service worker and icon.

chrome/browser/banners/app_banner_manager.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/banners/app_banner_manager.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 222 matching lines @@
 
   manifest_url_ = data.manifest_url;
   manifest_ = data.manifest;
   app_title_ = (manifest_.name.is_null()) ? manifest_.short_name.string()
                                           : manifest_.name.string();
 
   PerformInstallableCheck();
 }
 
 void AppBannerManager::PerformInstallableCheck() {
-  if (IsWebAppInstalled(web_contents()->GetBrowserContext(),
-                        manifest_.start_url, manifest_url_) &&
-      !IsDebugMode()) {
-    ReportStatus(web_contents(), ALREADY_INSTALLED);
-    Stop();
-  }
-
-  if (!is_active_)
+  if (!CheckIfShouldShowBanner())
     return;
 
   // Fetch and verify the other required information.
   manager_->GetData(ParamsToPerformInstallableCheck(GetIdealIconSizeInPx(),
                                                     GetMinimumIconSizeInPx()),
                     base::Bind(&AppBannerManager::OnDidPerformInstallableCheck,
                                GetWeakPtr()));
 }
 
 void AppBannerManager::OnDidPerformInstallableCheck(
@@ skipping 70 matching lines @@
   was_canceled_by_page_ = false;
   page_requested_prompt_ = false;
   need_to_log_status_ = false;
   validated_url_ = GURL();
   referrer_.erase();
 }
 
 void AppBannerManager::SendBannerPromptRequest() {
   RecordCouldShowBanner();
 
-  // Given all of the other checks that have been made, the only possible reason
-  // for stopping now is that the app has been added to the homescreen.
-  if (!IsDebugMode() && !CheckIfShouldShowBanner()) {
-    Stop();
-    return;
-  }
-
   TrackBeforeInstallEvent(BEFORE_INSTALL_EVENT_CREATED);
   event_request_id_ = ++gCurrentRequestID;
 
   web_contents()->GetMainFrame()->GetRemoteInterfaces()->GetInterface(
       mojo::MakeRequest(&controller_));
 
   controller_->BannerPromptRequest(
       binding_.CreateInterfacePtrAndBind(), mojo::MakeRequest(&event_),
       {GetBannerType()},
       base::Bind(&AppBannerManager::OnBannerPromptReply, GetWeakPtr()));
@@ skipping 85 matching lines @@
 void AppBannerManager::RecordCouldShowBanner() {
   content::WebContents* contents = web_contents();
   DCHECK(contents);
 
   AppBannerSettingsHelper::RecordBannerEvent(
       contents, validated_url_, GetAppIdentifier(),
       AppBannerSettingsHelper::APP_BANNER_EVENT_COULD_SHOW, GetCurrentTime());
 }
 
 bool AppBannerManager::CheckIfShouldShowBanner() {
+  if (IsDebugMode())
+    return true;
+
+  // Check whether we are permitted to show the banner. If we have already
+  // added this site to homescreen, or if the banner has been shown too
+  // recently, prevent the banner from being shown.
   content::WebContents* contents = web_contents();
-  DCHECK(contents);
-
   InstallableStatusCode code = AppBannerSettingsHelper::ShouldShowBanner(
       contents, validated_url_, GetAppIdentifier(), GetCurrentTime());
-  if (code == NO_ERROR_DETECTED)
-    return true;
 
-  switch (code) {
-    case ALREADY_INSTALLED:
-      banners::TrackDisplayEvent(banners::DISPLAY_EVENT_INSTALLED_PREVIOUSLY);
-      break;
-    case PREVIOUSLY_BLOCKED:
-      banners::TrackDisplayEvent(banners::DISPLAY_EVENT_BLOCKED_PREVIOUSLY);
-      break;
-    case PREVIOUSLY_IGNORED:
-      banners::TrackDisplayEvent(banners::DISPLAY_EVENT_IGNORED_PREVIOUSLY);
-      break;
-    case INSUFFICIENT_ENGAGEMENT:

[dominickn, 2017/01/10 22:51:59]

ShouldShowBanner() will never return INSUFFICIENT_ENGAGEMENT any more.

-      banners::TrackDisplayEvent(banners::DISPLAY_EVENT_NOT_VISITED_ENOUGH);
-      break;
-    default:
-      break;
+  if (code == NO_ERROR_DETECTED &&
+      IsWebAppInstalled(contents->GetBrowserContext(), manifest_.start_url,
+                        manifest_url_)) {
+    code = ALREADY_INSTALLED;
   }
 
-  // If we are in debug mode, AppBannerSettingsHelper::ShouldShowBanner must
-  // return NO_ERROR_DETECTED (bypass flag is set) or we must not have entered
-  // this method.
-  DCHECK(!IsDebugMode());
-  ReportStatus(web_contents(), code);
-  return false;
+  if (code != NO_ERROR_DETECTED) {
+    switch (code) {
+      case ALREADY_INSTALLED:
+        banners::TrackDisplayEvent(banners::DISPLAY_EVENT_INSTALLED_PREVIOUSLY);
+        break;
+      case PREVIOUSLY_BLOCKED:
+        banners::TrackDisplayEvent(banners::DISPLAY_EVENT_BLOCKED_PREVIOUSLY);
+        break;
+      case PREVIOUSLY_IGNORED:
+        banners::TrackDisplayEvent(banners::DISPLAY_EVENT_IGNORED_PREVIOUSLY);
+        break;

[benwells, 2017/01/13 06:04:27]

Could this break be NOTREACHED()?

[dominickn, 2017/01/13 06:11:20]

Done.

+      default:
+        break;
+    }
+    ReportStatus(contents, code);
+    Stop();
+    return false;
+  }
+  return true;
 }
 
 void AppBannerManager::OnBannerPromptReply(
     blink::mojom::AppBannerPromptReply reply,
     const std::string& referrer) {
   // We don't need the controller any more, so reset it so the Blink-side object
   // is destroyed.
   controller_.reset();
   content::WebContents* contents = web_contents();
 
@@ skipping 42 matching lines @@
     // Simulate a non-canceled OnBannerPromptReply to show the delayed banner.
     // Don't reset |was_canceled_by_page_| yet for metrics purposes.
     OnBannerPromptReply(blink::mojom::AppBannerPromptReply::NONE, referrer_);
   } else {
     // Log that the prompt request was made for when we get the prompt reply.
     page_requested_prompt_ = true;
   }
 }
 
 }  // namespace banners