Allow app banners to be triggered by increases in site engagement.

chrome/browser/banners/app_banner_manager.h

 // 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.
 
 #ifndef CHROME_BROWSER_BANNERS_APP_BANNER_MANAGER_H_
 #define CHROME_BROWSER_BANNERS_APP_BANNER_MANAGER_H_
 
 #include <memory>
+#include <vector>
 
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/banners/app_banner_data_fetcher.h"
+#include "chrome/browser/engagement/site_engagement_observer.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "third_party/WebKit/public/platform/modules/app_banner/WebAppBannerPromptReply.h"
 
-namespace content {
-struct FrameNavigateParams;
-struct LoadCommittedDetails;
-}  // namespace content
-
 namespace banners {
 class AppBannerDataFetcher;
 
 /**
  * Creates an app banner.
  *
  * Hooks the wiring together for getting the data for a particular app.
  * Monitors at most one app at a time, tracking the info for the most recently
  * requested app. Any work in progress for other apps is discarded.
  */
 class AppBannerManager : public content::WebContentsObserver,
-                         public AppBannerDataFetcher::Delegate {
+                         public AppBannerDataFetcher::Delegate,
+                         public SiteEngagementObserver {
  public:
   static void DisableSecureSchemeCheckForTesting();
 
   static void SetEngagementWeights(double direct_engagement,
                                    double indirect_engagement);
 
   // Returns whether or not the URLs match for everything except for the ref.
   static bool URLsAreForTheSamePage(const GURL& first, const GURL& second);
 
   // Requests an app banner. Set |is_debug_mode| when it is triggered by the
   // developer's action in DevTools.
-  void RequestAppBanner(content::RenderFrameHost* render_frame_host,
-                        const GURL& validated_url,
-                        bool is_debug_mode);
+  void RequestAppBanner(const GURL& validated_url, bool is_debug_mode);
 
-  AppBannerManager();
   ~AppBannerManager() override;
 
  protected:
   explicit AppBannerManager(content::WebContents* web_contents);
 
   void ReplaceWebContents(content::WebContents* web_contents);
 
   // Creates an AppBannerDataFetcher, which constructs an app banner.
   virtual AppBannerDataFetcher* CreateAppBannerDataFetcher(
       base::WeakPtr<AppBannerDataFetcher::Delegate> weak_delegate,
       bool is_debug_mode) = 0;
 
   // Return whether the AppBannerDataFetcher is active.
   bool IsFetcherActive();
 
   scoped_refptr<AppBannerDataFetcher> data_fetcher() { return data_fetcher_; }
 
  private:
   // WebContentsObserver overrides.
-  void DidNavigateMainFrame(
-      const content::LoadCommittedDetails& details,
-      const content::FrameNavigateParams& params) override;
-
+  void DidStartNavigation(
+      content::NavigationHandle* navigation_handle) override;
+  void DidFinishNavigation(
+      content::NavigationHandle* navigation_handle) override;
   void DidFinishLoad(content::RenderFrameHost* render_frame_host,
                      const GURL& validated_url) override;
+  void MediaStartedPlaying(const MediaPlayerId& id) override;
+  void MediaStoppedPlaying(const MediaPlayerId& id) override;
+  void WasShown() override;
+  void WasHidden() override;
 
   // AppBannerDataFetcher::Delegate overrides.
   bool HandleNonWebApp(const std::string& platform,
                        const GURL& url,
                        const std::string& id,
                        bool is_debug_mode) override;
 
+  // SiteEngagementObserver overrides.
+  void OnEngagementIncreased(content::WebContents* web_contents,
+                             const GURL& url,
+                             double score) override;
+
   // Cancels an active DataFetcher, stopping its banners from appearing.
   void CancelActiveFetcher();
 
   // The type of navigation made to the page
   ui::PageTransition last_transition_type_;
 
   // Fetches the data required to display a banner for the current page.
   scoped_refptr<AppBannerDataFetcher> data_fetcher_;
 
+  // We do not want to trigger a banner when the manager is attached to
+  // a WebContents that is playing video or in the background.
+  std::vector<MediaPlayerId> active_media_players_;
+  bool is_hidden_;

[benwells, 2016/06/28 00:57:50]

I recall we chatted about this, and you were going to test if triggering a
banner while a web contents is hidden has any problems. Did you do the test?

[dominickn, 2016/06/28 03:52:29]

Thanks for reminding me about this. The right thing happens and the banner
triggers when you switch back, so I'm removing this variable.

+
+  // If a banner is requested before the page has finished loading, defer
+  // triggering the pipeline until the load is complete.
+  bool load_finished_;
+  bool banner_requested_;

[benwells, 2016/06/28 00:57:50]

Instead of these two bools, did you consider using an enum to represent the
valid states? E.g. might be 'LOADING, LOAD_FINISHED, ELIGIBLE_FOR_BANNER,
BANNER_REQUESTED'.

[dominickn, 2016/06/28 03:52:29]

We need to know BANNER_REQUESTED independently of LOAD_FINISHED; we'd end up
having to have two enums stored instead of two bools. If you feel that it's more
readable, I don't mind changing it.

[benwells, 2016/06/29 03:38:24]

I was finding the logic hard to follow and thought that might make it easier,
but two enums sounds worse :D

+
   // A weak pointer is used as the lifetime of the ServiceWorkerContext is
   // longer than the lifetime of this banner manager. The banner manager
   // might be gone when calls sent to the ServiceWorkerContext are completed.
   base::WeakPtrFactory<AppBannerManager> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(AppBannerManager);
 };  // class AppBannerManager
 
 }  // namespace banners
 
 #endif  // CHROME_BROWSER_BANNERS_APP_BANNER_MANAGER_H_