Fix the ability to bypass app banner engagement checks.

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 <algorithm>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 373 matching lines @@
 
 void AppBannerManager::DidFinishLoad(
     content::RenderFrameHost* render_frame_host,
     const GURL& validated_url) {
   // Don't start the banner flow unless the main frame has finished loading.
   if (render_frame_host->GetParent())
     return;
 
   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 */);
   }
 }
 
 void AppBannerManager::MediaStartedPlaying(const MediaPlayerInfo& media_info,
                                            const MediaPlayerId& id) {
   active_media_players_.push_back(id);
 }
@@ skipping 140 matching lines @@
   if (is_pending_event()) {
     // Simulate a non-canceled OnBannerPromptReply to show the delayed banner.
     OnBannerPromptReply(blink::mojom::AppBannerPromptReply::NONE, referrer_);
   } else {
     // Log that the prompt request was made for when we get the prompt reply.
     page_requested_prompt_ = true;
   }
 }
 
 }  // namespace banners