Allow banners to trigger on sites which don't register a service worker onload.

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 <map>
 #include <memory>
 #include <tuple>
 #include <utility>
 #include <vector>
 
 #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_metrics.h"
 #include "content/public/browser/service_worker_context.h"
+#include "content/public/browser/service_worker_context_observer.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.
 // Data is cached and fetched in the order specified in this struct. A web app
 // manifest will always be fetched first.
 struct InstallableParams {
@@ skipping 11 matching lines @@
 
   // The minimum badge icon size to fetch. Used only if
   // |fetch_valid_badge_icon| is true.
   int minimum_badge_icon_size_in_px = -1;
 
   // Check whether the site is installable. That is, it has a manifest valid for
   // a web app and a service worker controlling the manifest start URL and the
   // current URL.
   bool check_installable = false;
 
+  // Whether or not to wait indefinitely for a service worker.
+  bool wait_for_worker = true;

[pkotwicz, 2017/06/14 15:34:36]

Drive by: This should be false for add_to_homescreen_data_fetcher.cc ?

[dominickn, 2017/06/15 04:16:07]

Nope, it should not be false for add_to_homescreen_data_fetcher.

1. Setting this to false means that https://crbug.com/721881 won't be fixed,
since we need to wait for the service worker to fix it.

2. add_to_homescreen_data_fetcher.cc already has a 4 second wait timeout so it
handles the case where this waits indefinitely.

[pkotwicz, 2017/06/15 15:10:04]

2. Doesn't your change cause the Web Manifest to be ignored if the page does not
have a service worker? e.g.
https://googlechrome.github.io/samples/web-application-manifest/index.html

1. I don't fully understand https://crbug.com/721881 It seems that
InstallableManager::OnRegistrationStored() should update the cached value
regardless of InstallableManager::worker_::is_waiting

[dominickn, 2017/06/15 23:45:38]

The fix for this is to make add to homescreen do what app banners do - get the
manifest first, pull necessary things out of it, and then do the rest of the
check. That's a follow up.
If the check has already finished, we've already dispatched the callback and
then there's no way for the InstallableManager to tell the add to homescreen
fetcher that something has changed. You end up with this:

1. User presses add to homescreen. No service worker registered yet
2. We do the check, there's no service worker. Run the callback
3. Fetcher determines it's not WebAPK compatible and shows the usual A2HS dialog
4. Service worker is registered. But the fetcher has already finished so there's
no point doing anything now.

The simple thing to do is as above: ask for the manifest first, and then do the
rest of the check.

[pkotwicz, 2017/06/16 23:07:43]

We will need to fetch the icons in the first call which gets the manifest.
Otherwise, we would ignore icons in the Web Manifest if the web page does not
have a service worker

We will need to add a timeout to webapk_update_data_fetcher.cc We want to update
WebAPKs for critical updates even if they no longer have a service worker

[dominickn, 2017/06/17 00:00:26]

No, we'll ignore icons if the web page has a manifest *that's WebAPKable*,
because that is checked before a service worker.

We can easily fetch manifest + icon, then check installability with
InstallableManager.
Off topic: I feel like what should happen is if a WebAPK loses its service
worker is that it should just start opening in a tab in Chrome. It's a pretty
bad experience for users if their WebAPK suddenly can't work offline any more.

[pkotwicz, 2017/06/19 18:46:50]

We need to decide whether to launch a page in full screen mode prior to loading
the page. Thus, we cannot check whether the page has a service worker registered
prior to deciding whether to launch the page in full screen mode.

[dominickn, 2017/06/20 02:26:24]

As discussed offline, this shouldn't be an issue because updating happens after
launch, and affects the next launch, not the current one.

+
   // Check whether there is a fetchable, non-empty icon in the manifest
   // conforming to the primary icon size parameters.
   bool fetch_valid_primary_icon = false;
 
   // Check whether there is a fetchable, non-empty icon in the manifest
   // conforming to the badge icon size parameters.
   bool fetch_valid_badge_icon = false;
 };
 
 // This struct is passed to an InstallableCallback when the InstallableManager
@@ skipping 35 matching lines @@
   // app manifest. If check_installable was true and the site isn't installable,
   // the reason will be in error_code.
   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.
 class InstallableManager
-    : public content::WebContentsObserver,
+    : public content::ServiceWorkerContextObserver,
+      public content::WebContentsObserver,
       public content::WebContentsUserData<InstallableManager> {
  public:
   explicit InstallableManager(content::WebContents* web_contents);
   ~InstallableManager() override;
 
   // Returns true if the overall security state of |web_contents| is sufficient
   // to be considered installable.
   static bool IsContentSecure(content::WebContents* web_contents);
 
   // Returns the minimum icon size in pixels for a site to be installable.
   // TODO(dominickn): consolidate this concept with minimum_icon_size_in_px
   // across all platforms.
   static int GetMinimumIconSizeInPx();
 
   // Get the installable data, fetching the resources specified in |params|.
   // |callback| is invoked synchronously (i.e. no via PostTask on the UI thread
   // when the data is ready; the synchronous execution ensures that the
   // references |callback| receives in its InstallableData argument are valid.
   //
+  // Clients must be prepared for |callback| to not ever be invoked. For
+  // instance, if installability checking is requested, this method will wait
+  // until the site registers a service worker (and hence not invoke |callback|
+  // at all if a service worker is never registered).
+  //
   // Calls requesting data that is already fetched will return the cached data.
-  // This method is marked virtual so clients may mock this object in tests.
-  virtual void GetData(const InstallableParams& params,
-                       const InstallableCallback& callback);
+  void GetData(const InstallableParams& params,
+               const InstallableCallback& callback);
 
   // Called via AppBannerManagerAndroid to record metrics on how often the
   // installable check is completed when the menu or add to homescreen menu item
   // is opened on Android.
   void RecordMenuOpenHistogram();
   void RecordMenuItemAddToHomescreenHistogram();
   void RecordQueuedMetricsOnTaskCompletion(const InstallableParams& params,
                                            bool check_passed);
 
  private:
   friend class InstallableManagerBrowserTest;
   friend class InstallableManagerUnitTest;
   FRIEND_TEST_ALL_PREFIXES(InstallableManagerBrowserTest,
                            ManagerBeginsInEmptyState);
   FRIEND_TEST_ALL_PREFIXES(InstallableManagerBrowserTest, CheckWebapp);
+  FRIEND_TEST_ALL_PREFIXES(InstallableManagerBrowserTest,
+                           CheckLazyServiceWorker);
 
   using Task = std::pair<InstallableParams, InstallableCallback>;
   using IconParams = std::tuple<int, int, content::Manifest::Icon::IconPurpose>;
 
   struct ManifestProperty;
-  struct InstallableProperty;
+  struct ValidManifestProperty;
+  struct ServiceWorkerProperty;
   struct IconProperty;
 
   // Returns an IconParams object that queries for a primary icon conforming to
   // the primary icon size parameters in |params|.
   IconParams ParamsForPrimaryIcon(const InstallableParams& params) const;
   // Returns an IconParams object that queries for a badge icon conforming to
   // the badge icon size parameters in |params|.
   IconParams ParamsForBadgeIcon(const InstallableParams& params) const;
 
   // Returns true if |params| matches any fetched icon, or false if no icon has
   // been requested yet or there is no match.
   bool IsIconFetched(const IconParams& params) const;
 
   // Sets the icon matching |params| as fetched.
   void SetIconFetched(const IconParams& params);
 
   // Returns the error code associated with the resources requested in |params|,
   // or NO_ERROR_DETECTED if there is no error.
   InstallableStatusCode GetErrorCode(const InstallableParams& params);
 
   // Gets/sets parts of particular properties. Exposed for testing.
   InstallableStatusCode manifest_error() const;
-  InstallableStatusCode installable_error() const;
-  void set_installable_error(InstallableStatusCode error_code);
+  InstallableStatusCode valid_manifest_error() const;
+  void set_valid_manifest_error(InstallableStatusCode error_code);
+  InstallableStatusCode worker_error() const;
   InstallableStatusCode icon_error(const IconParams& icon_params);
   GURL& icon_url(const IconParams& icon_params);
   const SkBitmap* icon(const IconParams& icon);
 
   // 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;
@@ skipping 16 matching lines @@
   void FetchManifest();
   void OnDidGetManifest(const GURL& manifest_url,
                         const content::Manifest& manifest);
 
   void CheckInstallable();
   bool IsManifestValidForWebApp(const content::Manifest& manifest);
   void CheckServiceWorker();
   void OnDidCheckHasServiceWorker(content::ServiceWorkerCapability capability);
 
   void CheckAndFetchBestIcon(const IconParams& params);
-  void OnIconFetched(
-      const GURL icon_url, const IconParams& params, const SkBitmap& bitmap);
+  void OnIconFetched(const GURL icon_url,
+                     const IconParams& params,
+                     const SkBitmap& bitmap);
+
+  // content::ServiceWorkerContextObserver overrides
+  void OnRegistrationStored(const GURL& pattern) override;
 
   // 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.
   std::vector<Task> tasks_;
 
   // Installable properties cached on this object.
   std::unique_ptr<ManifestProperty> manifest_;
-  std::unique_ptr<InstallableProperty> installable_;
+  std::unique_ptr<ValidManifestProperty> valid_manifest_;
+  std::unique_ptr<ServiceWorkerProperty> worker_;
   std::map<IconParams, IconProperty> icons_;
 
+  // Owned by the storage partition attached to the content::WebContents which
+  // this object is scoped to.
+  content::ServiceWorkerContext* service_worker_context_;
+
   // Whether or not the current page is a PWA. This is reset per navigation and
   // is independent of the caching mechanism, i.e. if a PWA check is run
   // multiple times for one page, this will be set on the first check.
   InstallabilityCheckStatus page_status_;
 
   // Counts for the number of queued requests of the menu and add to homescreen
   // menu item there have been whilst the installable check is awaiting
   // completion. Used for metrics recording.
   int menu_open_count_;
   int menu_item_add_to_homescreen_count_;
 
   bool is_active_;
   bool is_pwa_check_complete_;
 
   base::WeakPtrFactory<InstallableManager> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(InstallableManager);
 };
 
 #endif  // CHROME_BROWSER_INSTALLABLE_INSTALLABLE_MANAGER_H_