Allow the app banner installability check to run on page load.

chrome/browser/installable/installable_manager.cc

 // Copyright 2016 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/installable/installable_manager.h"
 
 #include "base/bind.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "chrome/browser/manifest/manifest_icon_downloader.h"
@@ skipping 43 matching lines @@
       if (size.width() >= kIconMinimumSizeInPx &&
           size.height() >= kIconMinimumSizeInPx) {
         return true;
       }
     }
   }
 
   return false;
 }
 
+// Returns true if |params| specifies a full PWA check.
+bool IsParamsForPwaCheck(const InstallableParams& params) {
+  return params.check_installable && params.fetch_valid_primary_icon;
+}
+
 }  // namespace
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(InstallableManager);
 
 struct InstallableManager::ManifestProperty {
   InstallableStatusCode error = NO_ERROR_DETECTED;
   GURL url;
   content::Manifest manifest;
   bool fetched = false;
 };
@@ skipping 13 matching lines @@
   InstallableStatusCode error = NO_ERROR_DETECTED;
   GURL url;
   std::unique_ptr<SkBitmap> icon;
   bool fetched;
 
  private:
   // This class contains a std::unique_ptr and therefore must be move-only.
   DISALLOW_COPY_AND_ASSIGN(IconProperty);
 };
 
-
 InstallableManager::InstallableManager(content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
       manifest_(new ManifestProperty()),
       installable_(new InstallableProperty()),
+      page_status_(InstallableMetrics::NOT_STARTED),
+      menu_open_count_(0),
+      menu_item_count_(0),
       is_active_(false),
-      weak_factory_(this) { }
+      is_pwa_check_complete_(false),
+      weak_factory_(this) {}
 
 InstallableManager::~InstallableManager() = default;
 
 // static
 bool InstallableManager::IsContentSecure(content::WebContents* web_contents) {
   if (!web_contents)
     return false;
 
   // Whitelist localhost. Check the VisibleURL to match what the
   // SecurityStateTabHelper looks at.
@@ skipping 19 matching lines @@
   // Return immediately if we're already working on a task. The new task will be
   // looked at once the current task is finished.
   tasks_.push_back({params, callback});
   if (is_active_)
     return;
 
   is_active_ = true;
   StartNextTask();
 }
 
+void InstallableManager::RecordMenuOpenHistogram() {
+  if (is_pwa_check_complete_)
+    InstallableMetrics::RecordMenuOpenHistogram(page_status_);
+  else
+    ++menu_open_count_;
+}
+
+void InstallableManager::RecordMenuItemAddToHomescreenHistogram() {
+  if (is_pwa_check_complete_)
+    InstallableMetrics::RecordMenuItemAddToHomescreenHistogram(page_status_);
+  else
+    ++menu_item_count_;
+}
+
+void InstallableManager::RecordQueuedMetricsOnTaskCompletion(
+    const InstallableParams& params,
+    bool check_passed) {
+  // Don't do anything if we've already finished the PWA check or if this check
+  // was not for the full PWA params. Once a full check is completed, metrics
+  // will be directly recorded in Record*Histogram since
+  // |is_pwa_check_complete_| will be true.
+  if (is_pwa_check_complete_ || !IsParamsForPwaCheck(params))
+    return;
+
+  is_pwa_check_complete_ = true;
+  page_status_ = check_passed ? InstallableMetrics::COMPLETE_PWA
+                              : InstallableMetrics::COMPLETE_NON_PWA;
+
+  // Compute what the status would have been for any queued calls to
+  // Record*Histogram, and record appropriately.
+  InstallableMetrics::InstallabilityCheckStatus prev_status =
+      check_passed ? InstallableMetrics::IN_PROGRESS_PWA
+                   : InstallableMetrics::IN_PROGRESS_NON_PWA;
+  for (; menu_open_count_ > 0; --menu_open_count_)
+    InstallableMetrics::RecordMenuOpenHistogram(prev_status);
+
+  for (; menu_item_count_ > 0; --menu_item_count_)
+    InstallableMetrics::RecordMenuItemAddToHomescreenHistogram(prev_status);
+
+  DCHECK_EQ(0, menu_open_count_);
+  DCHECK_EQ(0, menu_item_count_);

[pkotwicz, 2017/03/31 05:07:44]

Personally, I would remove the DCHECKs. In my experience, DCHECKS are only
useful in catching bugs if they make tests on the trybots fail

[benwells, 2017/03/31 05:35:28]

+1, these DCHECKS don't add much value.

[dominickn, 2017/04/03 00:06:46]

Done.

+}
+
 InstallableManager::IconParams InstallableManager::ParamsForPrimaryIcon(
     const InstallableParams& params) const {
   return std::make_tuple(params.ideal_primary_icon_size_in_px,
                          params.minimum_primary_icon_size_in_px,
                          IconPurpose::ANY);
 }
 
 InstallableManager::IconParams InstallableManager::ParamsForBadgeIcon(
     const InstallableParams& params) const {
   return std::make_tuple(params.ideal_badge_icon_size_in_px,
@@ skipping 72 matching lines @@
          (!params.fetch_valid_badge_icon ||
              IsIconFetched(ParamsForBadgeIcon(params)));
 }
 
 void InstallableManager::Reset() {
   // Prevent any outstanding callbacks to or from this object from being called.
   weak_factory_.InvalidateWeakPtrs();
   tasks_.clear();
   icons_.clear();
 
+  // We may have reset prior to completion, in which case menu_open_count_ or

[pkotwicz, 2017/03/31 05:07:44]

Nit: |menu_open_count_| and |menu_item_count_| with '|'

[dominickn, 2017/04/03 00:06:46]

Done.

+  // menu_item_count_ will be nonzero. If we completed, then
+  // these values cannot be anything except 0.
+  page_status_ = InstallableMetrics::NOT_STARTED;
+  is_pwa_check_complete_ = false;
+
+  for (; menu_open_count_ > 0; --menu_open_count_) {
+    InstallableMetrics::RecordMenuOpenHistogram(
+        InstallableMetrics::STOPPED_BEFORE_COMPLETION);
+  }
+
+  for (; menu_item_count_ > 0; --menu_item_count_) {
+    InstallableMetrics::RecordMenuItemAddToHomescreenHistogram(
+        InstallableMetrics::STOPPED_BEFORE_COMPLETION);
+  }
+
+  DCHECK_EQ(0, menu_open_count_);
+  DCHECK_EQ(0, menu_item_count_);
+
   manifest_.reset(new ManifestProperty());
   installable_.reset(new InstallableProperty());
 
   is_active_ = false;
 }
 
 void InstallableManager::SetManifestDependentTasksComplete() {
   DCHECK(!tasks_.empty());
   const InstallableParams& params = tasks_[0].first;
 
@@ skipping 40 matching lines @@
   DCHECK(is_active_);
   WorkOnTask();
 }
 
 void InstallableManager::WorkOnTask() {
   DCHECK(!tasks_.empty());
   const Task& task = tasks_[0];
   const InstallableParams& params = task.first;
 
   InstallableStatusCode code = GetErrorCode(params);
-  if (code != NO_ERROR_DETECTED || IsComplete(params)) {
+  bool check_passed = (code == NO_ERROR_DETECTED);
+  if (!check_passed || IsComplete(params)) {
+    RecordQueuedMetricsOnTaskCompletion(params, check_passed);
     RunCallback(task, code);
     tasks_.erase(tasks_.begin());
     StartNextTask();
     return;
   }
 
   if (!manifest_->fetched) {
     FetchManifest();
   } else if (params.check_installable && !installable_->fetched) {
     CheckInstallable();
@@ skipping 193 matching lines @@
   return manifest_->url;
 }
 
 const content::Manifest& InstallableManager::manifest() const {
   return manifest_->manifest;
 }
 
 bool InstallableManager::is_installable() const {
   return installable_->installable;
 }