Replace AppBannerDataFetcher with InstallableManager.

chrome/browser/banners/app_banner_manager.h

Index: chrome/browser/banners/app_banner_manager.h
diff --git a/chrome/browser/banners/app_banner_manager.h b/chrome/browser/banners/app_banner_manager.h
index 20969abb1a74ed8bbec2558d8442e49f45c71a7d..8f09b697806251d256a9681599f20093c1d063d5 100644
--- a/chrome/browser/banners/app_banner_manager.h
+++ b/chrome/browser/banners/app_banner_manager.h
@@ -8,105 +8,272 @@
 #include <memory>
 #include <vector>
 
+#include "base/callback_forward.h"
 #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 "chrome/browser/installable/installable_checker.h"
+#include "chrome/browser/installable/installable_logging.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "third_party/WebKit/public/platform/modules/app_banner/WebAppBannerPromptReply.h"
 
+class SkBitmap;
+struct WebApplicationInfo;
+
+namespace content {
+class RenderFrameHost;
+class WebContents;
+}
+
+// This forward declaration exists solely for the DidFinishCreatingBookmarkApp
+// callback, implemented and called on desktop platforms only.
+namespace extensions {
+class Extension;
+}
+
 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.
- */
+
+// Creates an app banner.

[gone, 2016/07/23 23:39:23]

Should probably add // at the beginning of the blank lines to show that the
comments go together.

[dominickn, 2016/07/25 00:23:51]

Done.

+
+// 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.
+
+// This class contains the generic functionality shared between all platforms,
+// as well as callbacks that the platform-specific implementations pass to
+// base::Bind. Subclasses can override the callbacks with custom functionality,
+// allowing a WeakPtrFactory to be housed in this class, rather than being
+// required in all subclasses.
 class AppBannerManager : public content::WebContentsObserver,
-                         public AppBannerDataFetcher::Delegate,
                          public SiteEngagementObserver {
  public:
   static void DisableSecureSchemeCheckForTesting();
 
+  // Returns the current time.
+  static base::Time GetCurrentTime();
+
+  // Fast-forwards the current time for testing.
+  static void SetTimeDeltaForTesting(int days);
+
+  // Sets the weights applied to direct and indirect navigations for triggering
+  // the banner. Deprecated and will be removed when app banners fully migrates
+  // to using site engagement as a trigger.
   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.
+  // Requests an app banner. If |is_debug_mode| is true, any failure in the
+  // pipeline will be reported to the devtools console.
   virtual void RequestAppBanner(const GURL& validated_url, bool is_debug_mode);
 
-  ~AppBannerManager() override;
+  // Overridden and passed through base::Bind on desktop platforms. Called when
+  // the bookmark app install initiated by a banner has completed. Not used on
+  // Android.
+  virtual void DidFinishCreatingBookmarkApp(
+      const extensions::Extension* extension,
+      const WebApplicationInfo& web_app_info) { }
+
+  // Overridden and passed through base::Bind on Android. Called when the
+  // download of a native app's icon is complete, as native banners use an icon
+  // provided from the Play Store rather than the web manifest. Not used on
+  // desktop platforms.
+  virtual void OnAppIconFetched(const SkBitmap& bitmap) { }
+
+  // Overridden and passed through base::Bind on Android. Called after a web app
+  // banner was successfully used to add a web app to homescreen to kick off an
+  // asynchronous fetch of a splash screen icon. Not used on desktop platforms.
+  virtual base::Closure FetchWebappSplashScreenImageCallback(
+      const std::string& webapp_id);
 
  protected:
   explicit AppBannerManager(content::WebContents* web_contents);
+  ~AppBannerManager() override;
 
-  void ReplaceWebContents(content::WebContents* web_contents);
+  // Return a string identifying this app for metrics.
+  virtual std::string GetAppIdentifier();
 
-  // Creates an AppBannerDataFetcher, which constructs an app banner.
-  virtual AppBannerDataFetcher* CreateAppBannerDataFetcher(
-      base::WeakPtr<AppBannerDataFetcher::Delegate> weak_delegate,
-      bool is_debug_mode) = 0;
+  // Return a string describing what type of banner is being created. Used when
+  // alerting websites that a banner is about to be created.
+  virtual std::string GetBannerType();
 
-  // Return whether the AppBannerDataFetcher is active.
-  bool IsFetcherActive();
+  // Returns a string parameter for a devtools console message corresponding to
+  // |code|. Returns the empty string if |code| requires no parameter.
+  std::string GetErrorParam(installable::ErrorCode code);
 
-  scoped_refptr<AppBannerDataFetcher> data_fetcher() { return data_fetcher_; }
+  // Returns the ideal and minimum icon sizes required for being installable.
+  virtual int GetIdealIconSizeInDp();
+  virtual int GetMinimumIconSizeInDp();
 
- private:
-  // WebContentsObserver overrides.
-  void DidStartNavigation(
-      content::NavigationHandle* navigation_handle) override;
-  void DidFinishNavigation(
-      content::NavigationHandle* navigation_handle) override;
+  // Returns a WeakPtr to this object. Exposed so subclasses/infobars may
+  // may bind callbacks without needing their own WeakPtrFactory.
+  base::WeakPtr<AppBannerManager> GetWeakPtr();
+
+  // Returns true if |is_debug_mode_| is true or the
+  // kBypassAppBannerEngagementChecks flag is set.
+  bool IsDebugMode() const;
+
+  // Returns true if the webapp at |start_url| has already been installed.
+  virtual bool IsWebAppInstalled(content::BrowserContext* browser_context,
+                                 const GURL& start_url);
+
+  // This class uses installable::InstallableChecker to help fetch and validate
+  // a site's PWA-ness (and hence its eligibility for banners). The checker is
+  // called twice. First, the manifest is requested, so we can verify whether
+  // the site is already installed (and on Android, divert the flow to a native
+  // app banner if requested). The second call verifies the manifest as valid
+  // for a web app, checks the service worker, and fetches an icon.
+  // InstallableChecker acts as a WebContents-scoped cache for this information,
+  // so anything in the browser process which needs this data can access it

[gone, 2016/07/23 23:39:23]

This entire thing reads like it should be in the class comment to serve as
documentation.  Does it make sense to move it upward?

[dominickn, 2016/07/25 00:23:51]

Done.

+  // without additional IPC overhead or redundant fetches.
+
+  // Callback invoked by the InstallableChecker once it has fetched the page's
+  // manifest.
+  void OnDidGetManifest(const installable::InstallableResult& result);
+
+  // Run at the conclusion of OnDidGetManifest. For web app banners, this calls
+  // back to the InstallableChecker to continue checking criteria. For native
+  // app banners, this checks whether native apps are preferred in the manifest,
+  // and calls to Java to verify native app details. If a native banner isn't or
+  // can't be requested, it continues with the web app banner checks.
+  virtual void ContinueInstallableCheck();
+
+  // Callback invoked by the InstallableChecker once it has finished checking
+  // all other installable properties.
+  void OnDidFinishInstallableCheck(
+      const installable::InstallableResult& result);
+
+  // Records that a banner was shown. The |event_name| corresponds to the RAPPOR
+  // metric being recorded.
+  void RecordDidShowBanner(const std::string& event_name);
+
+  // Logs an error message corresponding to |code| to the devtools console
+  // attached to |web_contents|. Does nothing if IsDebugMode() returns false.
+  void ReportError(content::WebContents* web_contents,
+                   installable::ErrorCode code);
+
+  // Stops the banner pipeline. Any callback currently running will terminate
+  // the next time they check |is_active_|.
+  virtual void Stop();
+
+  // Sends a message to the renderer that the page has met the requirements to
+  // show a banner. The page can respond to cancel the banner (and possibly
+  // display it later), or otherwise allow it to be shown. This is virtual to
+  // allow tests to mock out the renderer IPC.
+  virtual void SendBannerPromptRequest();
+
+  // content::WebContentsObserver overrides.
+  void DidStartNavigation(content::NavigationHandle* handle) override;
+  void DidFinishNavigation(content::NavigationHandle* 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;
-
-  // AppBannerDataFetcher::Delegate overrides.
-  bool HandleNonWebApp(const std::string& platform,
-                       const GURL& url,
-                       const std::string& id,
-                       bool is_debug_mode) override;
+  void WebContentsDestroyed() 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();
+  // Subclass accessors for private fields which should not be changed outside
+  // this class.
+  installable::InstallableChecker* checker() const { return checker_; }
+  int event_request_id() const { return event_request_id_; }
+  bool is_active() const { return is_active_; }
+
+  // The title to display in the banner.
+  base::string16 app_title_;
+
+  // The URL for which the banner check is being conducted.
+  GURL validated_url_;
+
+  // The URL of the manifest.
+  GURL manifest_url_;
+
+  // The manifest object.
+  content::Manifest manifest_;
+
+  // The URL of the icon.
+  GURL icon_url_;
+
+  // The icon object.
+  std::unique_ptr<SkBitmap> icon_;
+
+  // The referrer string (if any) specified in the app URL. Used only for native
+  // app banners.
+  std::string referrer_;
+
+ private:
+  friend class AppBannerManagerTest;
+
+  // Record that the banner could be shown at this point, if the triggering
+  // heuristic allowed.
+  void RecordCouldShowBanner();
+
+  // Creates a banner for the app. Overridden by subclasses as the infobar is
+  // platform-specific.
+  virtual void ShowBanner() = 0;
+
+  // Returns true if the banner should be shown.
+  bool CheckIfShouldShowBanner();
+
+  bool OnMessageReceived(const IPC::Message& message,
+                         content::RenderFrameHost* render_frame_host) override;
+
+  // Called after the manager sends a message to the renderer regarding its
+  // intention to show a prompt. The renderer will send a message back with the
+  // opportunity to cancel.
+  void OnBannerPromptReply(content::RenderFrameHost* render_frame_host,
+                           int request_id,
+                           blink::WebAppBannerPromptReply reply,
+                           std::string referrer);
+
+  // Called when the client has prevented a banner from being shown, and is
+  // now requesting that it be shown later.
+  void OnRequestShowAppBanner(content::RenderFrameHost* render_frame_host,
+                              int request_id);
 
   // 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_;
+  // Fetches and checks the data required to display a banner for the current
+  // page.
+  installable::InstallableChecker* checker_;
+
+  // A monotonically increasing id to verify the response to the
+  // beforeinstallprompt event from the renderer.
+  int event_request_id_;
 
   // We do not want to trigger a banner when the manager is attached to
   // a WebContents that is playing video. Banners triggering on a site in the
   // background will appear when the tab is reactivated.
   std::vector<MediaPlayerId> active_media_players_;
 
+  // Whether we are currently working on whether to show a banner.
+  bool is_active_;
+
   // If a banner is requested before the page has finished loading, defer
   // triggering the pipeline until the load is complete.
   bool banner_request_queued_;
   bool load_finished_;
 
-  // 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.
+  // Record whether the page decides to defer showing the banner, and if it
+  // requests for it to be shown later on.
+  bool was_canceled_by_page_;
+  bool page_requested_prompt_;
+
+  // Whether we should be logging errors to the console for this request.
+  bool is_debug_mode_;
+
+  // The concrete subclasses of this class are expected to have their lifetimes
+  // scoped to the WebContents which they are observing. This allows us to use
+  // weak pointers for callbacks.
   base::WeakPtrFactory<AppBannerManager> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(AppBannerManager);
-};  // class AppBannerManager
+};
 
 }  // namespace banners