Add new app banner metrics using InstallableStatusCode.

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 "base/bind.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/strings/string_number_conversions.h"
@@ skipping 199 matching lines @@
   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) &&
       !IsDebugMode()) {
+    TrackInstallableErrorCode(ALREADY_INSTALLED);
     Stop();
   }
 
   if (!is_active_)
     return;
 
   // Fetch and verify the other required information.
   manager_->GetData(ParamsToPerformInstallableCheck(GetIdealIconSizeInDp(),
                                                     GetMinimumIconSizeInDp()),
                     base::Bind(&AppBannerManager::OnDidPerformInstallableCheck,
@@ skipping 36 matching lines @@
       GetCurrentTime());
   rappor::SampleDomainAndRegistryFromGURL(g_browser_process->rappor_service(),
                                           event_name,
                                           contents->GetLastCommittedURL());
 }
 
 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;
   was_canceled_by_page_ = false;
   page_requested_prompt_ = false;
   referrer_.erase();
 }
 
 void AppBannerManager::SendBannerPromptRequest() {
   RecordCouldShowBanner();
@@ skipping 68 matching lines @@
   Stop();
 }
 
 void AppBannerManager::OnEngagementIncreased(content::WebContents* contents,
                                              const GURL& url,
                                              double score) {
   // Only trigger a banner using site engagement if:
   //  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 (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_) {
-      // Wait until the main frame finishes loading before requesting a banner.
-      banner_request_queued_ = true;
+      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);
     }
   }
 }
 
 void AppBannerManager::RecordCouldShowBanner() {
   content::WebContents* contents = web_contents();
   DCHECK(contents);
 
   AppBannerSettingsHelper::RecordBannerCouldShowEvent(
       contents, validated_url_, GetAppIdentifier(),
       GetCurrentTime(), last_transition_type_);
 }
 
 bool AppBannerManager::CheckIfShouldShowBanner() {
   content::WebContents* contents = web_contents();
   DCHECK(contents);
 
+  // ShouldShowBanner will call TrackInstallableErrorCode as appropriate.
   return AppBannerSettingsHelper::ShouldShowBanner(
       contents, validated_url_, GetAppIdentifier(), GetCurrentTime());
 }
 
 bool AppBannerManager::OnMessageReceived(
     const IPC::Message& message,
     content::RenderFrameHost* render_frame_host) {
   bool handled = true;
 
   IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(AppBannerManager, message, render_frame_host)
@@ skipping 22 matching lines @@
   // called if a redisplay isn't asked for.
   //
   // We use the additional page_requested_prompt_ variable because the redisplay
   // request may be received *before* the Cancel prompt reply (e.g. if redisplay
   // is requested in the beforeinstallprompt event handler).
   referrer_ = referrer;
   if (reply == blink::WebAppBannerPromptReply::Cancel &&
       !page_requested_prompt_) {
     TrackBeforeInstallEvent(BEFORE_INSTALL_EVENT_PREVENT_DEFAULT_CALLED);
     was_canceled_by_page_ = true;
-    ReportError(contents, RENDERER_CANCELLED);
     return;
   }
 
   // If we haven't yet returned, but either of |was_canceled_by_page_| or
   // |page_requested_prompt_| is true, the page has requested a delayed showing
   // of the prompt. Otherwise, the prompt was never canceled by the page.
   if (was_canceled_by_page_ || page_requested_prompt_) {
     TrackBeforeInstallEvent(
         BEFORE_INSTALL_EVENT_PROMPT_CALLED_AFTER_PREVENT_DEFAULT);
     was_canceled_by_page_ = false;
   } else {
     TrackBeforeInstallEvent(BEFORE_INSTALL_EVENT_NO_ACTION);
   }
 
   AppBannerSettingsHelper::RecordMinutesFromFirstVisitToShow(
       contents, validated_url_, GetAppIdentifier(), GetCurrentTime());
 
   DCHECK(!manifest_url_.is_empty());
   DCHECK(!manifest_.IsEmpty());
   DCHECK(!icon_url_.is_empty());
   DCHECK(icon_.get());
+
   TrackBeforeInstallEvent(BEFORE_INSTALL_EVENT_COMPLETE);
   ShowBanner();
   is_active_ = false;
 }
 
 void AppBannerManager::OnRequestShowAppBanner(
     content::RenderFrameHost* render_frame_host,
     int request_id) {
   if (was_canceled_by_page_) {
     // Simulate a non-canceled OnBannerPromptReply to show the delayed banner.
     // Don't reset |was_canceled_by_page_| yet for metrics purposes.
     OnBannerPromptReply(render_frame_host, request_id,
                         blink::WebAppBannerPromptReply::None, referrer_);
   } else {
     // Log that the prompt request was made for when we get the prompt reply.
     page_requested_prompt_ = true;
   }
 }
 
 }  // namespace banners