Fix the ability to bypass app banner engagement checks.

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 af60409cd692e10b1b0c721d1e6a42ce6f4d73e4..d6661fde68436a0b0f19e767a82701e07f9126aa 100644
--- a/chrome/browser/banners/app_banner_manager.cc
+++ b/chrome/browser/banners/app_banner_manager.cc
@@ -391,12 +391,17 @@ void AppBannerManager::DidFinishLoad(
 
   load_finished_ = true;
   validated_url_ = validated_url;
+
+  // If the bypass flag is on, or if we require no engagement to trigger the
+  // banner, we can manually set |has_sufficient_engagement_| to true.

[Matt Giuca, 2017/04/24 03:34:30]

Drop "we can".

Also "set |has_sufficient_engagement_| to true" is just describing the code.
Rewrite this for what it actually means to set this to true and why we do this
(e.g., "the rest of the install flow should operate as if the engagement
threshold has been met").

[dominickn, 2017/04/24 03:42:34]

Done.

+  if (AppBannerSettingsHelper::HasSufficientEngagement(0))
+    has_sufficient_engagement_ = true;
+
   // Start the pipeline immediately if:
   //  1. we have sufficient engagement, or

[Matt Giuca, 2017/04/24 03:34:30]

Change the 1., 2., 3. since it no longer corresponds to the if statement:

1. we have sufficient engagement (or 0 engagement is required; see the above
check), or
2. the feature ... is enabled.

(Perhaps the numerals can be dropped also.)

[dominickn, 2017/04/24 03:42:34]

Done.

   //  2. 0 engagement is required, or
-  //  3. the feature to start the installability check in load is enabled
+  //  3. the feature to start the installability check on page load is enabled.
   if (has_sufficient_engagement_ ||
-      AppBannerSettingsHelper::HasSufficientEngagement(0) ||
       base::FeatureList::IsEnabled(
           features::kCheckInstallabilityForBannerOnLoad)) {
     RequestAppBanner(validated_url, false /* is_debug_mode */);