Web App Installability: Check web manifest scope in service worker scope

The web app installability check now verifies that the entire web app
manifest scope is within a service worker's scope, instead of simply
checking the start url.


Bug: 809837
Change-Id: Ia4a957aae3e3d1737401376d2188c6f8dd85837e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1799649Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#696003}

chrome/browser/installable/installable_manager.cc

@@ -564,17 +564,9 @@ void InstallableManager::CheckServiceWorker() {
   DCHECK(!worker_->fetched);
   DCHECK(!manifest().IsEmpty());
 
-  if (!manifest().start_url.is_valid()) {
-    worker_->has_worker = false;
-    worker_->error = NO_URL_FOR_SERVICE_WORKER;
-    worker_->fetched = true;
-    WorkOnTask();
-    return;
-  }
-
-  // Check to see if there is a service worker for the manifest's start url.
+  // Check to see if there is a service worker for the manifest's scope.
   service_worker_context_->CheckHasServiceWorker(
-      manifest().start_url,
+      manifest().scope,
       base::BindOnce(&InstallableManager::OnDidCheckHasServiceWorker,
                      weak_factory_.GetWeakPtr()));
 }
@@ -659,10 +651,8 @@ void InstallableManager::OnIconFetched(const GURL icon_url,
 
 void InstallableManager::OnRegistrationCompleted(const GURL& pattern) {
   // If the scope doesn't match we keep waiting.
-  if (!content::ServiceWorkerContext::ScopeMatches(pattern,
-                                                   manifest().start_url)) {
+  if (!content::ServiceWorkerContext::ScopeMatches(pattern, manifest().scope))
     return;
-  }
 
   bool was_active = task_queue_.HasCurrent();
 

chrome/browser/installable/installable_manager_browsertest.cc

@@ -14,8 +14,10 @@
 #include "chrome/browser/installable/installable_logging.h"
 #include "chrome/browser/installable/installable_manager.h"
 #include "chrome/browser/installable/installable_metrics.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/web_applications/test/service_worker_registration_waiter.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/test/browser_test_utils.h"
@@ -1483,7 +1485,7 @@ IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest,
   EXPECT_EQ(std::vector<InstallableStatusCode>(
                 {START_URL_NOT_VALID, MANIFEST_MISSING_NAME_OR_SHORT_NAME,
                  MANIFEST_DISPLAY_NOT_SUPPORTED, MANIFEST_MISSING_SUITABLE_ICON,
-                 NO_URL_FOR_SERVICE_WORKER, NO_ACCEPTABLE_ICON}),
+                 NO_ACCEPTABLE_ICON}),
             tester->errors());
 }
 
@@ -1507,7 +1509,6 @@ IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest,
                  GetErrorMessage(MANIFEST_MISSING_NAME_OR_SHORT_NAME),
                  GetErrorMessage(MANIFEST_DISPLAY_NOT_SUPPORTED),
                  GetErrorMessage(MANIFEST_MISSING_SUITABLE_ICON),
-                 GetErrorMessage(NO_URL_FOR_SERVICE_WORKER),
                  GetErrorMessage(NO_ACCEPTABLE_ICON)}),
             NavigateAndGetAllErrors(browser(),
                                     GetURLOfPageWithServiceWorkerAndManifest(
@@ -1526,3 +1527,26 @@ IN_PROC_BROWSER_TEST_F(InstallableManagerAllowlistOriginBrowserTest,
   ui_test_utils::NavigateToURL(browser(), GURL(kOtherInsecureOrigin));
   EXPECT_FALSE(InstallableManager::IsContentSecure(contents));
 }
+
+IN_PROC_BROWSER_TEST_F(InstallableManagerBrowserTest, NarrowServiceWorker) {
+  const GURL url =
+      embedded_test_server()->GetURL("/banners/scope_c/scope_c.html");
+  {
+    web_app::ServiceWorkerRegistrationWaiter registration_waiter(
+        browser()->profile(), url);
+    ui_test_utils::NavigateToURL(browser(), url);
+    registration_waiter.AwaitRegistration();
+  }
+  base::RunLoop run_loop;
+  std::unique_ptr<CallbackTester> tester(
+      new CallbackTester(run_loop.QuitClosure()));
+
+  InstallableParams params = GetWebAppParams();
+  params.wait_for_worker = false;
+
+  RunInstallableManager(browser(), tester.get(), params);
+  run_loop.Run();
+
+  EXPECT_EQ(std::vector<InstallableStatusCode>({NO_MATCHING_SERVICE_WORKER}),
+            tester->errors());
+}

chrome/browser/web_applications/BUILD.gn

@@ -79,6 +79,8 @@ source_set("web_applications_test_support") {
   testonly = true
 
   sources = [
+    "test/service_worker_registration_waiter.cc",
+    "test/service_worker_registration_waiter.h",
     "test/test_app_registrar.cc",
     "test/test_app_registrar.h",
     "test/test_data_retriever.cc",

chrome/browser/web_applications/test/service_worker_registration_waiter.cc

+// Copyright 2019 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/web_applications/test/service_worker_registration_waiter.h"
+
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/service_worker_context.h"
+#include "content/public/browser/storage_partition.h"
+
+namespace web_app {
+
+ServiceWorkerRegistrationWaiter::ServiceWorkerRegistrationWaiter(
+    content::BrowserContext* browser_context,
+    const GURL& url)
+    : url_(std::move(url)) {
+  content::StoragePartition* storage_partition =
+      content::BrowserContext::GetStoragePartitionForSite(browser_context,
+                                                          url_);
+  DCHECK(storage_partition);
+
+  service_worker_context_ = storage_partition->GetServiceWorkerContext();
+  service_worker_context_->AddObserver(this);
+}
+
+ServiceWorkerRegistrationWaiter::~ServiceWorkerRegistrationWaiter() {
+  if (service_worker_context_)
+    service_worker_context_->RemoveObserver(this);
+}
+
+void ServiceWorkerRegistrationWaiter::AwaitRegistration() {
+  run_loop_.Run();
+}
+
+void ServiceWorkerRegistrationWaiter::OnRegistrationCompleted(
+    const GURL& pattern) {
+  if (content::ServiceWorkerContext::ScopeMatches(pattern, url_))
+    run_loop_.Quit();
+}
+
+void ServiceWorkerRegistrationWaiter::OnDestruct(
+    content::ServiceWorkerContext* context) {
+  service_worker_context_->RemoveObserver(this);
+  service_worker_context_ = nullptr;
+}
+
+}  // namespace web_app

chrome/browser/web_applications/test/service_worker_registration_waiter.h

+// Copyright 2019 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_WEB_APPLICATIONS_TEST_SERVICE_WORKER_REGISTRATION_WAITER_H_
+#define CHROME_BROWSER_WEB_APPLICATIONS_TEST_SERVICE_WORKER_REGISTRATION_WAITER_H_
+
+#include "base/macros.h"
+#include "base/run_loop.h"
+#include "content/public/browser/service_worker_context_observer.h"
+#include "url/gurl.h"
+
+namespace content {
+class BrowserContext;
+class ServiceWorkerContext;
+}  // namespace content
+
+namespace web_app {
+
+class ServiceWorkerRegistrationWaiter
+    : public content::ServiceWorkerContextObserver {
+ public:
+  ServiceWorkerRegistrationWaiter(content::BrowserContext* browser_context,
+                                  const GURL& url);
+  ~ServiceWorkerRegistrationWaiter() override;
+
+  void AwaitRegistration();
+
+ private:
+  // content::ServiceWorkerContextObserver:
+  void OnRegistrationCompleted(const GURL& pattern) override;
+  void OnDestruct(content::ServiceWorkerContext* context) override;
+
+  content::ServiceWorkerContext* service_worker_context_;
+  const GURL url_;
+  base::RunLoop run_loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(ServiceWorkerRegistrationWaiter);
+};
+
+}  // namespace web_app
+
+#endif  // CHROME_BROWSER_WEB_APPLICATIONS_TEST_SERVICE_WORKER_REGISTRATION_WAITER_H_

chrome/test/data/banners/scope_c/manifest.webmanifest

+{
+  "name": "Service worker scope is narrower than manifest scope",
+  "icons": [
+    {
+      "src": "/banners/image-512px.png",
+      "sizes": "512x512",
+      "type": "image/png"
+    }
+  ],
+  "scope": "/banners/",
+  "start_url": "/banners/scope_c/scope_c.html",
+  "display": "standalone"
+}

chrome/test/data/banners/scope_c/scope_c.html

+<head>
+  <title>Service worker scope is narrower than manifest scope</title>
+  <link rel="manifest" href="manifest.webmanifest">
+  <script>
+  navigator.serviceWorker.register('/banners/service_worker.js',
+                                   {scope: '/banners/scope_c/'});
+  </script>
+</head>