Extract AppBannerDataFetcher into an InstallableManager.

chrome/browser/installable/installable_manager.h

+// 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.
+
+#ifndef CHROME_BROWSER_INSTALLABLE_INSTALLABLE_MANAGER_H_
+#define CHROME_BROWSER_INSTALLABLE_INSTALLABLE_MANAGER_H_
+
+#include <utility>
+
+#include "base/callback_forward.h"
+#include "base/gtest_prod_util.h"
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "chrome/browser/installable/installable_logging.h"
+#include "chrome/browser/installable/installable_property.h"
+#include "content/public/browser/web_contents_observer.h"
+#include "content/public/browser/web_contents_user_data.h"
+#include "content/public/common/manifest.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+#include "url/gurl.h"
+
+// This struct specifies the work to be done by the InstallableManager.
+// Results are cached and fetched in the order specified in this struct. A web
+// app manifest will always be fetched first.
+struct InstallableParams {
+  // The ideal icon size to fetch. Used only if |fetch_valid_icon| is true.
+  int ideal_icon_size_in_dp = -1;
+
+  // The minimum icon size to fetch. Used only if |fetch_valid_icon| is true.
+  int minimum_icon_size_in_dp = -1;
+
+  // Check whether the site is installable. That is, it's manifest is valid for
+  // a web app, and it has a service worker controlling the manifest start URL
+  // and the current URL.
+  bool check_installable = false;
+
+  // Check whether there is an icon in the manifest conforming to the icon size
+  // parameters, and that the icon can be fetched and isn't an empty bitmap.
+  bool fetch_valid_icon = false;
+};
+
+// This struct is passed to an InstallableCallback when the InstallableManager
+// has finished with a task. Each reference is owned by InstallableManager, and
+// callers should copy any objects which they wish to use later.
+// Non-requested fields will be set to null, empty, or false.
+struct InstallableData {
+  // NO_ERROR_DETECTED if there were no issues.
+  const InstallableErrorCode error_code;
+
+  // Empty if the site has no <link rel="manifest"> tag.
+  const GURL& manifest_url;
+
+  // Empty if the site has an unparseable manifest.
+  const content::Manifest& manifest;
+
+  // Empty if no icon was requested.
+  const GURL& icon_url;
+
+  // nullptr if the most appropriate icon couldn't be determined or downloaded.

[benwells, 2016/07/29 04:36:54]

Nit: add comment about ownership. Also mention under what cases the reason would
be in error_code. Something like, if fetch_valid_icon is true and there was a
problem with the icon, problem will be in the error_code.

[dominickn, 2016/07/31 23:32:04]

Done.

+  const SkBitmap* icon;
+
+  // true if the site has a service worker and a viable web app manifest.

[benwells, 2016/07/29 04:36:54]

Nit: mention that if it is not installable the reason will be in error_code.

[dominickn, 2016/07/31 23:32:04]

Done.

+  const bool is_installable;
+};
+
+using InstallableCallback = base::Callback<void(const InstallableData&)>;
+
+// This class is responsible for fetching the resources required to check and
+// install a site. A call to GetData() will always fetch a site's web app
+// manifest if it exists; optionally the full installability criteria can be

[benwells, 2016/07/29 04:36:54]

Nit: you can probably remove the second sentence of this comment.

[dominickn, 2016/07/31 23:32:04]

Done.

+// checked.
+class InstallableManager
+    : public content::WebContentsObserver,
+      public content::WebContentsUserData<InstallableManager> {
+ public:
+  explicit InstallableManager(content::WebContents* web_contents);
+  ~InstallableManager() override;
+
+  // Returns true if |manifest| meets the requirements for being installable.
+  bool IsManifestValidForWebApp(const content::Manifest& manifest);
+
+  // Returns the minimum icon size in pixels for a site to be installable.
+  // TODO(dominickn): consolidate this concept with minimum_icon_size_in_dp
+  // across all platforms.
+  static int GetMinimumIconSizeInPx();
+
+  // Run the installable check, fetching the resources specified in |params|.
+  // |callback| is invoked on the UI thread when the checks are complete,
+  // passing an InstallableData struct containing the requested resources.

[benwells, 2016/07/29 04:36:54]

s/Run the installable check/Get the installable data/
s/when the checks are complete/when the data is ready/

Or something like that.

[dominickn, 2016/07/31 23:32:04]

Done.

+  //
+  // Separate calls to GetData are processed serially; each call is guaranteed
+  // to invoke |callback| before the next one is processed. Resources are
+  // cached; a previous call for the same page and with the same icon sizes
+  // will not perform duplicate work and will run the callback immediately with

[benwells, 2016/07/29 04:36:54]

Will the callback ever be run synchronously / before GetData exits? If so that
is worth noting.

I think earlier versions of this code did not make the guarantee about
|callback| being invoked before the next one is processed. Is it worth making
that guarantee? It might make it harder to change the internals later.

[dominickn, 2016/07/31 23:32:04]

Done.

[benwells, 2016/08/03 01:17:28]

Nit: I don't think it's clear that the callback can run synchronously (which I
thikn it can). this is important to note as it is a bit unusual, and also can
cause reentrancy problems if not dealt with properly.

[dominickn, 2016/08/03 06:38:04]

Done.

+  // the cached resources.
+  // This method is marked virtual so clients may mock this object in tests.
+  virtual void GetData(const InstallableParams& params,
+                       const InstallableCallback& callback);
+
+ private:
+  friend class InstallableManagerUnitTest;
+  FRIEND_TEST_ALL_PREFIXES(InstallableManagerBrowserTest,
+                           ManagerBeginsInEmptyState);
+  FRIEND_TEST_ALL_PREFIXES(InstallableManagerBrowserTest, CheckWebapp);
+
+  using Task = std::pair<InstallableParams, InstallableCallback>;
+  using Tasks = std::vector<Task>;
+  using IconParams = std::pair<int, int>;
+  using IconMap = std::map<IconParams, IconProperty>;

[benwells, 2016/07/29 04:36:54]

Nit: consider not having Tasks / IconMap but just having std::vector / std::map
in the field declaration. These types are only used there so the extra
indirection doesn't buy much.

[dominickn, 2016/07/31 23:32:04]

Done.

+
+  IconProperty& GetIcon(const InstallableParams& params);
+
+  // Returns true if the icon size in |params| matches the icon size which has
+  // most recently been requested. false if no icon has been requested yet or

[benwells, 2016/07/29 04:36:54]

s/most recently/already/

I thikn that is more correct now you have the map.

[dominickn, 2016/07/31 23:32:05]

Done.

+  // they don't match.
+  bool IsIconFetched(const InstallableParams& params) const;
+
+  // Sets the icon parameters in |params| as being fetched.
+  void SetIconFetched(const InstallableParams& params);
+
+  // Returns the error code associated with the resources requested in |params|,
+  // or NO_ERROR_DETECTED if there is no error.
+  InstallableErrorCode GetErrorCode(const InstallableParams& params);
+
+  // Returns the WebContents to which this object is attached, or nullptr if the
+  // WebContents doesn't exist or is currently being destroyed.
+  content::WebContents* GetWebContents();
+
+  // Returns true if |params| requires no more work to be done.
+  bool IsComplete(const InstallableParams& params) const;
+
+  // Returns true if |web_contents| is non-null and this manager remains active.
+  bool IsContentsValid(content::WebContents* web_contents) const;
+
+  // Resets members to empty and removes all queued tasks.
+  // Called when navigating to a new page or if the WebContents has unexpectedly
+  // died whilst waiting for a callback.
+  void Reset();
+
+  // Sets manifest validity, service worker, and icon fetch tasks to complete.
+  // Called if no manifest (or an empty manifest) was fetched from the site.
+  void SetManifestDependentTasksComplete();
+
+  void StartNextTask();
+  void RunCallback(const Task& task, InstallableErrorCode error);
+
+  // Core work loop which coordinates and dispatches the work required for the
+  // current task.
+  void WorkOnTask();
+
+  // Data retrieval methods.
+  void FetchManifest();
+  void OnDidGetManifest(const GURL& manifest_url,
+                        const content::Manifest& manifest);
+
+  void CheckInstallable();
+  void CheckServiceWorker();
+  void OnDidCheckHasServiceWorker(bool has_service_worker);
+
+  void CheckAndFetchBestIcon();
+  void OnAppIconFetched(const GURL icon_url, const SkBitmap& bitmap);
+
+  // content::WebContentsObserver overrides
+  void DidFinishNavigation(content::NavigationHandle* handle) override;
+  void WebContentsDestroyed() override;
+
+  const GURL& manifest_url() const;
+  const content::Manifest& manifest() const;
+  bool is_installable() const;
+
+  // The list of <params, callback> pairs that have come from a call to GetData.
+  Tasks tasks_;
+
+  // Installable properties cached on this object.
+  ManifestProperty manifest_prop_;
+  BooleanProperty installable_prop_;
+  IconMap icons_;
+
+  bool is_active_;
+
+  base::WeakPtrFactory<InstallableManager> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(InstallableManager);
+};
+
+#endif  // CHROME_BROWSER_INSTALLABLE_INSTALLABLE_MANAGER_H_