Add new app banner metrics using InstallableStatusCode.

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 2757f5cb5ac99beb1a0576bea768ebea3d80836d..4f08c961aad168505f4330a08e7cd5e52681dc17 100644
--- a/chrome/browser/banners/app_banner_manager.cc
+++ b/chrome/browser/banners/app_banner_manager.cc
@@ -217,6 +217,7 @@ void AppBannerManager::PerformInstallableCheck() {
   if (IsWebAppInstalled(web_contents()->GetBrowserContext(),
                         manifest_.start_url) &&
       !IsDebugMode()) {
+    TrackInstallableErrorCode(ALREADY_INSTALLED);
     Stop();
   }
 
@@ -273,12 +274,15 @@ void AppBannerManager::ReportError(content::WebContents* web_contents,
                                    InstallableErrorCode code) {
   if (IsDebugMode())
     LogErrorToConsole(web_contents, code, GetErrorParam(code));
+  else
+    TrackInstallableErrorCode(code);
 }
 
 void AppBannerManager::Stop() {
   if (was_canceled_by_page_ && !page_requested_prompt_) {
     TrackBeforeInstallEvent(
         BEFORE_INSTALL_EVENT_PROMPT_NOT_CALLED_AFTER_PREVENT_DEFAULT);
+    ReportError(web_contents(), RENDERER_CANCELLED);
   }
 
   is_active_ = false;
@@ -367,19 +371,24 @@ void AppBannerManager::OnEngagementIncreased(content::WebContents* contents,
   //  1. engagement increased for the web contents which we are attached to; and
   //  2. there are no currently active media players; and
   //  3. we have accumulated sufficient engagement.
-  if (web_contents() == contents && active_media_players_.empty() &&
-      AppBannerSettingsHelper::HasSufficientEngagement(score)) {
-    // Stop observing so we don't double-trigger the banner.
-    SiteEngagementObserver::Observe(nullptr);
-
-    if (!load_finished_) {
-      // Wait until the main frame finishes loading before requesting a banner.
-      banner_request_queued_ = true;
+  if (web_contents() == contents && active_media_players_.empty()) {
+    if (AppBannerSettingsHelper::HasSufficientEngagement(score)) {
+      // Stop observing so we don't double-trigger the banner.
+      SiteEngagementObserver::Observe(nullptr);
+
+      if (!load_finished_) {
+        // Queue the banner request until the main frame finishes loading.
+        banner_request_queued_ = true;
+      } else {
+        // A banner request performs some simple tests, creates a data fetcher,
+        // and starts some asynchronous checks to test installability. It should
+        // be safe to start this in response to user input.
+        RequestAppBanner(url, false /* is_debug_mode */);
+      }
     } else {
-      // Requesting a banner performs some simple tests, creates a data fetcher,
-      // and starts some asynchronous checks to test installability. It should
-      // be safe to start this in response to user input.
-      RequestAppBanner(url, false /* is_debug_mode */);
+      // If IsDebugMode() returns true, we must have taken the other branch, so
+      // there is no need to recheck it here to avoid skew.

[gone, 2016/08/10 21:02:51]

Seems like something that should be DCHECKED, as well.

[dominickn, 2016/08/11 07:00:19]

Done.

+      TrackInstallableErrorCode(INSUFFICIENT_ENGAGEMENT);
     }
   }
 }
@@ -397,6 +406,7 @@ bool AppBannerManager::CheckIfShouldShowBanner() {
   content::WebContents* contents = web_contents();
   DCHECK(contents);
 
+  // ShouldShowBanner will call TrackInstallableErrorCode as appropriate.
   return AppBannerSettingsHelper::ShouldShowBanner(
       contents, validated_url_, GetAppIdentifier(), GetCurrentTime());
 }
@@ -439,7 +449,6 @@ void AppBannerManager::OnBannerPromptReply(
       !page_requested_prompt_) {
     TrackBeforeInstallEvent(BEFORE_INSTALL_EVENT_PREVENT_DEFAULT_CALLED);
     was_canceled_by_page_ = true;
-    ReportError(contents, RENDERER_CANCELLED);
     return;
   }
 
@@ -461,6 +470,7 @@ void AppBannerManager::OnBannerPromptReply(
   DCHECK(!manifest_.IsEmpty());
   DCHECK(!icon_url_.is_empty());
   DCHECK(icon_.get());
+
   TrackBeforeInstallEvent(BEFORE_INSTALL_EVENT_COMPLETE);
   ShowBanner();
   is_active_ = false;