Re-trigger beforeinstallprompt when install prompt is dismissed.

Bug: 770016
Change-Id: I3375293694ca5e124ae3a37224a150f495d76179
Reviewed-on: https://chromium-review.googlesource.com/718059
Commit-Queue: Michael McGreevy <mcgreevy@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509678}

chrome/browser/banners/app_banner_manager.cc

@@ -102,10 +102,9 @@ class TrackingStatusReporter
   // Records code via an UMA histogram.
   void ReportStatus(content::WebContents* web_contents,
                     InstallableStatusCode code) override {
-    // Ensure that we haven't yet logged a status code for this page.
-    DCHECK(!done_);
-
-    if (code != NO_ERROR_DETECTED)
+    // We only increment the histogram once per page load (and only if the
+    // banner pipeline is triggered).
+    if (!done_ && code != NO_ERROR_DETECTED)
       banners::TrackInstallableStatusCode(code);
 
     done_ = true;
@@ -137,6 +136,7 @@ void AppBannerManager::RequestAppBanner(const GURL& validated_url,
   // if it's been triggered from devtools.
   if (state_ != State::INACTIVE) {
     DCHECK(is_debug_mode);
+    weak_factory_.InvalidateWeakPtrs();
     ResetBindings();
   }
 
@@ -182,6 +182,11 @@ void AppBannerManager::SendBannerAccepted() {
 void AppBannerManager::SendBannerDismissed() {
   if (event_.is_bound())
     event_->BannerDismissed();
+
+  if (IsExperimentalAppBannersEnabled()) {
+    ResetBindings();
+    SendBannerPromptRequest();  // Reprompt.
+  }
 }
 
 base::WeakPtr<AppBannerManager> AppBannerManager::GetWeakPtr() {
@@ -369,6 +374,7 @@ InstallableStatusCode AppBannerManager::TerminationCode() const {
 void AppBannerManager::Stop(InstallableStatusCode code) {
   ReportStatus(web_contents(), code);
 
+  weak_factory_.InvalidateWeakPtrs();
   ResetBindings();
   UpdateState(State::COMPLETE);
   status_reporter_ = std::make_unique<NullStatusReporter>(),
@@ -507,7 +513,6 @@ bool AppBannerManager::IsExperimentalAppBannersEnabled() {
 }
 
 void AppBannerManager::ResetBindings() {
-  weak_factory_.InvalidateWeakPtrs();
   binding_.Close();
   controller_.reset();
   event_.reset();
@@ -608,8 +613,15 @@ void AppBannerManager::ShowBanner() {
         BEFORE_INSTALL_EVENT_PROMPT_CALLED_AFTER_PREVENT_DEFAULT);
   }
 
-  AppBannerSettingsHelper::RecordMinutesFromFirstVisitToShow(
-      web_contents(), validated_url_, GetAppIdentifier(), GetCurrentTime());
+  // If this is the first time that we are showing the banner for this site,
+  // record how long it's been since the first visit.
+  if (AppBannerSettingsHelper::GetSingleBannerEvent(
+          web_contents(), validated_url_, GetAppIdentifier(),
+          AppBannerSettingsHelper::APP_BANNER_EVENT_DID_SHOW)
+          .is_null()) {
+    AppBannerSettingsHelper::RecordMinutesFromFirstVisitToShow(
+        web_contents(), validated_url_, GetAppIdentifier(), GetCurrentTime());
+  }
 
   DCHECK(!manifest_url_.is_empty());
   DCHECK(!manifest_.IsEmpty());

chrome/browser/banners/app_banner_manager.h

@@ -262,7 +262,7 @@ class AppBannerManager : public content::WebContentsObserver,
   // Returns whether the new experimental flow and UI is enabled.
   static bool IsExperimentalAppBannersEnabled();
 
-  // Voids all outstanding weak pointers and service pointers.
+  // Voids all outstanding service pointers.
   void ResetBindings();
 
   // Record that the banner could be shown at this point, if the triggering
@@ -276,8 +276,8 @@ class AppBannerManager : public content::WebContentsObserver,
   // Called after the manager sends a message to the renderer regarding its
   // intention to show a prompt. The renderer will send a message back with the
   // opportunity to cancel.
-  void OnBannerPromptReply(blink::mojom::AppBannerPromptReply reply,
-                           const std::string& referrer);
+  virtual void OnBannerPromptReply(blink::mojom::AppBannerPromptReply reply,
+                                   const std::string& referrer);
 
   // Does the non-platform specific parts of showing the app banner.
   void ShowBanner();

chrome/browser/banners/app_banner_settings_helper.cc

@@ -19,6 +19,7 @@
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/installable/installable_logging.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/common/chrome_switches.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/common/content_settings_pattern.h"
@@ -256,23 +257,29 @@ InstallableStatusCode AppBannerSettingsHelper::ShouldShowBanner(
   if (!added_time.is_null())
     return ALREADY_INSTALLED;
 
-  base::Time blocked_time =
-      GetSingleBannerEvent(web_contents, origin_url, package_name_or_start_url,
-                           APP_BANNER_EVENT_DID_BLOCK);
+  // Showing of experimental app banners is under developer control, and
+  // requires a user gesture. In contrast, showing of traditional app banners
+  // is automatic, so we throttle it if the user has recently ignored or
+  // blocked the banner.
+  if (!base::FeatureList::IsEnabled(features::kExperimentalAppBanners)) {
+    base::Time blocked_time = GetSingleBannerEvent(web_contents, origin_url,
+                                                   package_name_or_start_url,
+                                                   APP_BANNER_EVENT_DID_BLOCK);
+
+    // Null times are in the distant past, so the delta between real times and
+    // null events will always be greater than the limits.
+    if (time - blocked_time <
+        base::TimeDelta::FromDays(gDaysAfterDismissedToShow)) {
+      return PREVIOUSLY_BLOCKED;
+    }
 
-  // Null times are in the distant past, so the delta between real times and
-  // null events will always be greater than the limits.
-  if (time - blocked_time <
-      base::TimeDelta::FromDays(gDaysAfterDismissedToShow)) {
-    return PREVIOUSLY_BLOCKED;
+    base::Time shown_time = GetSingleBannerEvent(web_contents, origin_url,
+                                                 package_name_or_start_url,
+                                                 APP_BANNER_EVENT_DID_SHOW);
+    if (time - shown_time < base::TimeDelta::FromDays(gDaysAfterIgnoredToShow))
+      return PREVIOUSLY_IGNORED;
   }
 
-  base::Time shown_time =
-      GetSingleBannerEvent(web_contents, origin_url, package_name_or_start_url,
-                           APP_BANNER_EVENT_DID_SHOW);
-  if (time - shown_time < base::TimeDelta::FromDays(gDaysAfterIgnoredToShow))
-    return PREVIOUSLY_IGNORED;
-
   return NO_ERROR_DETECTED;
 }