[Backport] Fix for CVE-2019-5779

service worker: Make navigate/openWindow go through more security checks.

WindowClient.navigate() and Clients.openWindow() were implemented in
a way that directly navigated to the URL without going through
some checks that the normal navigation path goes through. This CL
attempts to fix that:
- WindowClient.navigate() now goes through Navigator::RequestOpenURL()
  instead of directly through WebContents::OpenURL().
- Clients.openWindow() now calls more ContentBrowserClient functions
  for manipulating the navigation before invoking
  ContentBrowserClient::OpenURL().

Bug: 904219
Change-Id: Ic38978aee98c09834fdbbc240164068faa3fd4f5
Reviewed-on: https://chromium-review.googlesource.com/c/1345686
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610753}
Reviewed-by: Michael Brüning <michael.bruning@qt.io>

10 files changed, 384 insertions, 26 deletions

diff --git a/chromium/content/browser/service_worker/service_worker_client_utils.cc b/chromium/content/browser/service_worker/service_worker_client_utils.cc
index c2252ec96fa..0d3db56fa94 100644
--- a/chromium/content/browser/service_worker/service_worker_client_utils.cc
+++ b/chromium/content/browser/service_worker/service_worker_client_utils.cc
@@ -12,6 +12,7 @@
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "content/browser/frame_host/frame_tree_node.h"
+#include "content/browser/frame_host/navigator.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
@@ -197,20 +198,13 @@ void DidOpenURLOnUI(WindowType type,
 void OpenWindowOnUI(
     const GURL& url,
     const GURL& script_url,
+    int worker_id,
     int worker_process_id,
     const scoped_refptr<ServiceWorkerContextWrapper>& context_wrapper,
     WindowType type,
     OpenURLCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  BrowserContext* browser_context =
-      context_wrapper->storage_partition()
-          ? context_wrapper->storage_partition()->browser_context()
-          : nullptr;
-  // We are shutting down.
-  if (!browser_context)
-    return;
-
   RenderProcessHost* render_process_host =
       RenderProcessHost::FromID(worker_process_id);
   if (render_process_host->IsForGuestsOnly()) {
@@ -221,18 +215,44 @@ void OpenWindowOnUI(
     return;
   }
 
+  SiteInstance* site_instance =
+      context_wrapper->process_manager()->GetSiteInstanceForWorker(worker_id);
+  if (!site_instance) {
+    // Worker isn't running anymore. Fail.
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::BindOnce(std::move(callback), ChildProcessHost::kInvalidUniqueID,
+                       MSG_ROUTING_NONE));
+    return;
+  }
+
+  // The following code is a rough copy of NavigatorImpl::RequestOpenURL. That
+  // function can't be used directly since there is no render frame host yet
+  // that the navigation will occur in.
+
+  GURL dest_url(url);
+  if (!GetContentClient()->browser()->ShouldAllowOpenURL(site_instance, url))
+    dest_url = GURL(url::kAboutBlankURL);
+
   OpenURLParams params(
-      url,
+      dest_url,
       Referrer::SanitizeForRequest(
-          url, Referrer(script_url, blink::kWebReferrerPolicyDefault)),
+          dest_url,
+          Referrer(script_url, blink::kWebReferrerPolicyDefault)),
       type == WindowType::PAYMENT_HANDLER_WINDOW
           ? WindowOpenDisposition::NEW_POPUP
           : WindowOpenDisposition::NEW_FOREGROUND_TAB,
       ui::PAGE_TRANSITION_AUTO_TOPLEVEL, true /* is_renderer_initiated */);
   params.open_app_window_if_possible = type == WindowType::NEW_TAB_WINDOW;
 
+  GetContentClient()->browser()->OverrideNavigationParams(
+      site_instance, &params.transition, &params.is_renderer_initiated,
+      &params.referrer);
+
+  // End of RequestOpenURL copy.
+
   GetContentClient()->browser()->OpenURL(
-      browser_context, params,
+      site_instance, params,
       base::AdaptCallbackForRepeating(
           base::BindOnce(&DidOpenURLOnUI, type, std::move(callback))));
 }
@@ -255,18 +275,18 @@ void NavigateClientOnUI(const GURL& url,
     return;
   }
 
-  ui::PageTransition transition = rfhi->GetParent()
-                                      ? ui::PAGE_TRANSITION_AUTO_SUBFRAME
-                                      : ui::PAGE_TRANSITION_AUTO_TOPLEVEL;
   int frame_tree_node_id = rfhi->frame_tree_node()->frame_tree_node_id();
 
-  OpenURLParams params(
-      url,
+  Navigator* navigator = rfhi->frame_tree_node()->navigator();
+  navigator->RequestOpenURL(
+      rfhi, url, false /* uses_post */, nullptr /* body */,
+      std::string() /* extra_headers */,
       Referrer::SanitizeForRequest(
           url, Referrer(script_url, blink::kWebReferrerPolicyDefault)),
-      frame_tree_node_id, WindowOpenDisposition::CURRENT_TAB, transition,
-      true /* is_renderer_initiated */);
-  web_contents->OpenURL(params);
+      WindowOpenDisposition::CURRENT_TAB,
+      false /* should_replace_current_entry */, false /* user_gesture */,
+      blink::WebTriggeringEventInfo::kUnknown,
+      nullptr /* blob_url_loader_factory */);
   new OpenURLObserver(web_contents, frame_tree_node_id, std::move(callback));
 }
 
@@ -458,6 +478,7 @@ void FocusWindowClient(ServiceWorkerProviderHost* provider_host,
 
 void OpenWindow(const GURL& url,
                 const GURL& script_url,
+                int worker_id,
                 int worker_process_id,
                 const base::WeakPtr<ServiceWorkerContextCore>& context,
                 WindowType type,
@@ -466,7 +487,7 @@ void OpenWindow(const GURL& url,
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::BindOnce(
-          &OpenWindowOnUI, url, script_url, worker_process_id,
+          &OpenWindowOnUI, url, script_url, worker_id, worker_process_id,
           base::WrapRefCounted(context->wrapper()), type,
           base::BindOnce(&DidNavigate, context, script_url.GetOrigin(),
                          std::move(callback))));
diff --git a/chromium/content/browser/service_worker/service_worker_client_utils.h b/chromium/content/browser/service_worker/service_worker_client_utils.h
index e927d202c5d..e4a18044b42 100644
--- a/chromium/content/browser/service_worker/service_worker_client_utils.h
+++ b/chromium/content/browser/service_worker/service_worker_client_utils.h
@@ -52,6 +52,7 @@ void FocusWindowClient(ServiceWorkerProviderHost* provider_host,
 // app, we will open a new foreground tab instead.
 void OpenWindow(const GURL& url,
                 const GURL& script_url,
+                int worker_id,
                 int worker_process_id,
                 const base::WeakPtr<ServiceWorkerContextCore>& context,
                 WindowType type,
diff --git a/chromium/content/browser/service_worker/service_worker_clients_api_browsertest.cc b/chromium/content/browser/service_worker/service_worker_clients_api_browsertest.cc
new file mode 100644
index 00000000000..e4efc7f9aed
--- /dev/null
+++ b/chromium/content/browser/service_worker/service_worker_clients_api_browsertest.cc
@@ -0,0 +1,319 @@
+// Copyright 2018 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 <string>
+
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/callback_forward.h"
+#include "base/logging.h"
+#include "base/memory/scoped_refptr.h"
+#include "base/optional.h"
+#include "base/strings/string16.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/task/post_task.h"
+#include "base/task/task_traits.h"
+#include "base/test/scoped_feature_list.h"
+#include "content/browser/service_worker/service_worker_context_core.h"
+#include "content/browser/service_worker/service_worker_context_wrapper.h"
+#include "content/browser/service_worker/service_worker_version.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/browser_task_traits.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/content_browser_client.h"
+#include "content/public/browser/platform_notification_context.h"
+#include "content/public/browser/service_worker_context_observer.h"
+#include "content/public/browser/storage_partition.h"
+#include "content/public/test/browser_test_utils.h"
+#include "content/public/test/content_browser_test.h"
+#include "content/public/test/content_browser_test_utils.h"
+#include "content/public/test/test_utils.h"
+#include "content/shell/browser/shell.h"
+#include "content/shell/browser/shell_content_browser_client.h"
+#include "content/test/test_content_browser_client.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "third_party/blink/public/mojom/service_worker/service_worker_event_status.mojom.h"
+#include "url/gurl.h"
+
+namespace content {
+
+namespace {
+// Waits for a service worker to be activated.
+class ActivatedServiceWorkerObserver : public ServiceWorkerContextObserver {
+ public:
+  ActivatedServiceWorkerObserver() : ServiceWorkerContextObserver() {}
+  ~ActivatedServiceWorkerObserver() override {}
+
+  void OnVersionActivated(int64_t version_id, const GURL& scope) override {
+    version_id_ = version_id;
+    if (activated_callback_)
+      std::move(activated_callback_).Run();
+  }
+
+  void WaitForActivated() {
+    if (version_id_ != -1)
+      return;
+    base::RunLoop loop;
+    activated_callback_ = loop.QuitClosure();
+    loop.Run();
+  }
+
+  int64_t version_id() const { return version_id_; }
+
+ private:
+  base::OnceClosure activated_callback_;
+  int64_t version_id_ = -1;
+
+  DISALLOW_COPY_AND_ASSIGN(ActivatedServiceWorkerObserver);
+};
+
+// Mainly for testing that ShouldAllowOpenURL() is properly consulted.
+class ServiceWorkerClientsContentBrowserClient
+    : public TestContentBrowserClient {
+ public:
+  ServiceWorkerClientsContentBrowserClient() : TestContentBrowserClient() {}
+  ~ServiceWorkerClientsContentBrowserClient() override {}
+
+  const GURL& opened_url() const { return opened_url_; }
+
+  void SetToAllowOpenURL(bool allow) { allow_open_url_ = allow; }
+
+  void WaitForOpenURL() {
+    if (!opened_url_.is_empty())
+      return;
+    base::RunLoop run_loop;
+    opened_url_callback_ = run_loop.QuitClosure();
+    run_loop.Run();
+  }
+
+  // ContentBrowserClient overrides:
+  bool ShouldAllowOpenURL(SiteInstance* site_instance,
+                          const GURL& url) override {
+    return allow_open_url_;
+  }
+
+  void OpenURL(
+      SiteInstance* site_instance,
+      const OpenURLParams& params,
+      const base::RepeatingCallback<void(WebContents*)>& callback) override {
+    opened_url_ = params.url;
+    callback.Run(nullptr);
+    if (opened_url_callback_)
+      std::move(opened_url_callback_).Run();
+  }
+
+ private:
+  bool allow_open_url_ = true;
+  GURL opened_url_;
+  base::OnceClosure opened_url_callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(ServiceWorkerClientsContentBrowserClient);
+};
+
+}  // namespace
+
+// Tests for the service worker Clients API.
+class ServiceWorkerClientsApiBrowserTest : public ContentBrowserTest {
+ public:
+  ServiceWorkerClientsApiBrowserTest() = default;
+
+  void SetUp() override {
+    ASSERT_TRUE(embedded_test_server()->InitializeAndListen());
+    ContentBrowserTest::SetUp();
+  }
+
+  void SetUpOnMainThread() override {
+    embedded_test_server()->StartAcceptingConnections();
+
+    StoragePartition* partition = BrowserContext::GetDefaultStoragePartition(
+        shell()->web_contents()->GetBrowserContext());
+    wrapper_ = static_cast<ServiceWorkerContextWrapper*>(
+        partition->GetServiceWorkerContext());
+  }
+
+  void DispatchNotificationClickEvent(int64_t version_id) {
+    base::RunLoop run_loop;
+    base::PostTaskWithTraits(
+        FROM_HERE, {BrowserThread::IO},
+        base::BindOnce(&ServiceWorkerClientsApiBrowserTest::
+                           DispatchNotificationClickEventOnIOThread,
+                       base::Unretained(this), version_id,
+                       run_loop.QuitClosure()));
+    run_loop.Run();
+  }
+
+  void DispatchNotificationClickEventOnIOThread(int64_t version_id,
+                                                base::OnceClosure done) {
+    ServiceWorkerVersion* version =
+        wrapper_->context()->GetLiveVersion(version_id);
+    ASSERT_TRUE(version);
+    version->RunAfterStartWorker(
+        ServiceWorkerMetrics::EventType::NOTIFICATION_CLICK,
+        base::BindOnce(&ServiceWorkerClientsApiBrowserTest::
+                           DispatchNotificationClickAfterStartWorker,
+                       base::Unretained(this), base::WrapRefCounted(version),
+                       std::move(done)));
+  }
+
+  void DispatchNotificationClickAfterStartWorker(
+      scoped_refptr<ServiceWorkerVersion> version,
+      base::OnceClosure done,
+      blink::ServiceWorkerStatusCode status) {
+    EXPECT_EQ(blink::ServiceWorkerStatusCode::kOk, status);
+    version->StartRequest(ServiceWorkerMetrics::EventType::NOTIFICATION_CLICK,
+                          base::DoNothing());
+    version->endpoint()->DispatchNotificationClickEvent(
+        "notification_id", {} /* notification_data */, -1 /* action_index */,
+        base::nullopt /* reply */,
+        base::BindOnce(
+            [](base::OnceClosure done,
+               blink::mojom::ServiceWorkerEventStatus event_status) {
+              EXPECT_EQ(blink::mojom::ServiceWorkerEventStatus::COMPLETED,
+                        event_status);
+              std::move(done).Run();
+            },
+            std::move(done)));
+  }
+
+  ServiceWorkerContextWrapper* wrapper() { return wrapper_.get(); }
+
+ private:
+  scoped_refptr<ServiceWorkerContextWrapper> wrapper_;
+};
+
+// Tests a successful WindowClient.navigate() call.
+IN_PROC_BROWSER_TEST_F(ServiceWorkerClientsApiBrowserTest, Navigate) {
+  // Load a page that registers a service worker.
+  EXPECT_TRUE(NavigateToURL(shell(),
+                            embedded_test_server()->GetURL(
+                                "/service_worker/create_service_worker.html")));
+  EXPECT_EQ("DONE", EvalJs(shell(), "register('client_api_worker.js');"));
+
+  // Load the page again so we are controlled.
+  EXPECT_TRUE(NavigateToURL(shell(),
+                            embedded_test_server()->GetURL(
+                                "/service_worker/create_service_worker.html")));
+  EXPECT_EQ(true, EvalJs(shell(), "!!navigator.serviceWorker.controller"));
+
+  // Have the service worker call client.navigate() on the page.
+  const std::string navigate_script = R"(
+    (async () => {
+      const registration = await navigator.serviceWorker.ready;
+      registration.active.postMessage({command: 'navigate', url: 'empty.html'});
+      return true;
+    })();
+  )";
+  EXPECT_EQ(true, EvalJs(shell(), navigate_script));
+
+  // The page should be navigated to empty.html.
+  const base::string16 title =
+      base::ASCIIToUTF16("ServiceWorker test - empty page");
+  TitleWatcher title_watcher(shell()->web_contents(), title);
+  EXPECT_EQ(title, title_watcher.WaitAndGetTitle());
+}
+
+// Tests a WindowClient.navigate() call to a disallowed URL.
+IN_PROC_BROWSER_TEST_F(ServiceWorkerClientsApiBrowserTest,
+                       NavigateDisallowedUrl) {
+  ServiceWorkerClientsContentBrowserClient content_browser_client;
+  content_browser_client.SetToAllowOpenURL(false);
+  auto* old_content_browser_client =
+      SetBrowserClientForTesting(&content_browser_client);
+
+  // Load a page that registers a service worker.
+  EXPECT_TRUE(NavigateToURL(shell(),
+                            embedded_test_server()->GetURL(
+                                "/service_worker/create_service_worker.html")));
+  EXPECT_EQ("DONE", EvalJs(shell(), "register('client_api_worker.js');"));
+
+  // Load the page again so we are controlled.
+  EXPECT_TRUE(NavigateToURL(shell(),
+                            embedded_test_server()->GetURL(
+                                "/service_worker/create_service_worker.html")));
+  EXPECT_EQ(true, EvalJs(shell(), "!!navigator.serviceWorker.controller"));
+
+  // Have the service worker call client.navigate() on the page.
+  const std::string navigate_script = R"(
+    (async () => {
+      const registration = await navigator.serviceWorker.ready;
+      registration.active.postMessage({command: 'navigate', url: 'empty.html'});
+      return true;
+    })();
+  )";
+  EXPECT_EQ(true, EvalJs(shell(), navigate_script));
+
+  // The page should be navigated to about:blank instead of the requested URL.
+  const base::string16 title = base::ASCIIToUTF16("about:blank");
+  TitleWatcher title_watcher(shell()->web_contents(), title);
+  EXPECT_EQ(title, title_watcher.WaitAndGetTitle());
+
+  SetBrowserClientForTesting(old_content_browser_client);
+}
+
+// Tests a successful Clients.openWindow() call.
+IN_PROC_BROWSER_TEST_F(ServiceWorkerClientsApiBrowserTest, OpenWindow) {
+  ServiceWorkerClientsContentBrowserClient content_browser_client;
+  auto* old_content_browser_client =
+      SetBrowserClientForTesting(&content_browser_client);
+
+  ActivatedServiceWorkerObserver observer;
+  wrapper()->AddObserver(&observer);
+
+  // Load a page that registers a service worker.
+  EXPECT_TRUE(NavigateToURL(shell(),
+                            embedded_test_server()->GetURL(
+                                "/service_worker/create_service_worker.html")));
+  EXPECT_EQ("DONE", EvalJs(shell(), "register('client_api_worker.js');"));
+  observer.WaitForActivated();
+
+  // Tell the service worker to call clients.openWindow(). Do it from a
+  // notification click event so it has a user interaction token that allows
+  // popups.
+  int64_t id = observer.version_id();
+  EXPECT_NE(-1, id);
+  DispatchNotificationClickEvent(id);
+
+  // OpenURL() should be called.
+  content_browser_client.WaitForOpenURL();
+  EXPECT_EQ(embedded_test_server()->GetURL("/service_worker/empty.html"),
+            content_browser_client.opened_url());
+
+  wrapper()->RemoveObserver(&observer);
+  SetBrowserClientForTesting(old_content_browser_client);
+}
+
+// Tests a Clients.openWindow() call to a disallowed URL.
+IN_PROC_BROWSER_TEST_F(ServiceWorkerClientsApiBrowserTest,
+                       OpenWindowDisallowedUrl) {
+  ServiceWorkerClientsContentBrowserClient content_browser_client;
+  content_browser_client.SetToAllowOpenURL(false);
+  auto* old_content_browser_client =
+      SetBrowserClientForTesting(&content_browser_client);
+
+  ActivatedServiceWorkerObserver observer;
+  wrapper()->AddObserver(&observer);
+
+  // Load a page that registers a service worker.
+  EXPECT_TRUE(NavigateToURL(shell(),
+                            embedded_test_server()->GetURL(
+                                "/service_worker/create_service_worker.html")));
+  EXPECT_EQ("DONE", EvalJs(shell(), "register('client_api_worker.js');"));
+  observer.WaitForActivated();
+
+  // Tell the service worker to call clients.openWindow(). Do it from a
+  // notification click event so it has a user interaction token that allows
+  // popups.
+  int64_t id = observer.version_id();
+  EXPECT_NE(-1, id);
+  DispatchNotificationClickEvent(id);
+
+  // OpenURL() should be called with about:blank instead of the requested URL.
+  content_browser_client.WaitForOpenURL();
+  EXPECT_EQ(GURL("about:blank"), content_browser_client.opened_url());
+
+  wrapper()->RemoveObserver(&observer);
+  SetBrowserClientForTesting(old_content_browser_client);
+}
+
+}  // namespace content
diff --git a/chromium/content/browser/service_worker/service_worker_context_wrapper.h b/chromium/content/browser/service_worker/service_worker_context_wrapper.h
index f1b9b94c5cc..3d3653d2816 100644
--- a/chromium/content/browser/service_worker/service_worker_context_wrapper.h
+++ b/chromium/content/browser/service_worker/service_worker_context_wrapper.h
@@ -278,6 +278,7 @@ class CONTENT_EXPORT ServiceWorkerContextWrapper
   friend class EmbeddedWorkerTestHelper;
   friend class EmbeddedWorkerBrowserTest;
   friend class FakeServiceWorkerContextWrapper;
+  friend class ServiceWorkerClientsApiBrowserTest;
   friend class ServiceWorkerDispatcherHost;
   friend class ServiceWorkerInternalsUI;
   friend class ServiceWorkerNavigationHandleCore;
diff --git a/chromium/content/browser/service_worker/service_worker_process_manager.cc b/chromium/content/browser/service_worker/service_worker_process_manager.cc
index 403f8b6525c..83459518200 100644
--- a/chromium/content/browser/service_worker/service_worker_process_manager.cc
+++ b/chromium/content/browser/service_worker/service_worker_process_manager.cc
@@ -195,6 +195,15 @@ void ServiceWorkerProcessManager::ReleaseWorkerProcess(int embedded_worker_id) {
   worker_process_map_.erase(it);
 }
 
+SiteInstance* ServiceWorkerProcessManager::GetSiteInstanceForWorker(
+    int embedded_worker_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  auto it = worker_process_map_.find(embedded_worker_id);
+  if (it == worker_process_map_.end())
+    return nullptr;
+  return it->second.get();
+}
+
 }  // namespace content
 
 namespace std {
diff --git a/chromium/content/browser/service_worker/service_worker_process_manager.h b/chromium/content/browser/service_worker/service_worker_process_manager.h
index f768781f2c9..f93d9d1abda 100644
--- a/chromium/content/browser/service_worker/service_worker_process_manager.h
+++ b/chromium/content/browser/service_worker/service_worker_process_manager.h
@@ -105,6 +105,8 @@ class CONTENT_EXPORT ServiceWorkerProcessManager {
     storage_partition_ = storage_partition;
   }
 
+  SiteInstance* GetSiteInstanceForWorker(int embedded_worker_id);
+
  private:
   friend class ServiceWorkerProcessManagerTest;
 
diff --git a/chromium/content/browser/service_worker/service_worker_version.cc b/chromium/content/browser/service_worker/service_worker_version.cc
index 5f63a1b98a0..3d2c9d0bdf4 100644
--- a/chromium/content/browser/service_worker/service_worker_version.cc
+++ b/chromium/content/browser/service_worker/service_worker_version.cc
@@ -1336,7 +1336,8 @@ void ServiceWorkerVersion::OpenWindow(
   }
 
   service_worker_client_utils::OpenWindow(
-      url, script_url_, embedded_worker_->process_id(), context_, type,
+      url, script_url_, embedded_worker_->embedded_worker_id(),
+      embedded_worker_->process_id(), context_, type,
       base::BindOnce(&OnOpenWindowFinished, std::move(callback)));
 }
 
diff --git a/chromium/content/public/browser/content_browser_client.cc b/chromium/content/public/browser/content_browser_client.cc
index 2a896c7072c..4fa8e54bce8 100644
--- a/chromium/content/public/browser/content_browser_client.cc
+++ b/chromium/content/public/browser/content_browser_client.cc
@@ -537,9 +537,10 @@ ContentBrowserClient::GetReceiverPresentationServiceDelegate(
 }
 
 void ContentBrowserClient::OpenURL(
-    content::BrowserContext* browser_context,
+    content::SiteInstance* site_instance,
     const content::OpenURLParams& params,
     const base::Callback<void(content::WebContents*)>& callback) {
+  DCHECK(site_instance);
   callback.Run(nullptr);
 }
 
diff --git a/chromium/content/public/browser/content_browser_client.h b/chromium/content/public/browser/content_browser_client.h
index b7973f853aa..34d4293bf2b 100644
--- a/chromium/content/public/browser/content_browser_client.h
+++ b/chromium/content/public/browser/content_browser_client.h
@@ -333,7 +333,8 @@ class CONTENT_EXPORT ContentBrowserClient {
   // This is called on the UI thread.
   virtual bool CanCommitURL(RenderProcessHost* process_host, const GURL& url);
 
-  // Returns whether a URL should be allowed to open from a specific context.
+  // Returns whether |url| should be allowed to open from the context indicated
+  // by |site_instance|.
   // This also applies in cases where the new URL will open in another process.
   virtual bool ShouldAllowOpenURL(SiteInstance* site_instance, const GURL& url);
 
@@ -969,9 +970,10 @@ class CONTENT_EXPORT ContentBrowserClient {
   GetReceiverPresentationServiceDelegate(WebContents* web_contents);
 
   // Allows programmatic opening of a new tab/window without going through
-  // another WebContents. For example, from a Worker. |callback| will be
+  // another WebContents. For example, from a Worker. |site_instance|
+  // describes the context initiating the navigation. |callback| will be
   // invoked with the appropriate WebContents* when available.
-  virtual void OpenURL(BrowserContext* browser_context,
+  virtual void OpenURL(SiteInstance* site_instance,
                        const OpenURLParams& params,
                        const base::Callback<void(WebContents*)>& callback);
 
diff --git a/chromium/content/test/BUILD.gn b/chromium/content/test/BUILD.gn
index 83788bd01e6..c80d1e80ded 100644
--- a/chromium/content/test/BUILD.gn
+++ b/chromium/content/test/BUILD.gn
@@ -10,8 +10,8 @@ import("//build/config/features.gni")
 import("//build/config/jumbo.gni")
 import("//build/config/ui.gni")
 import("//media/media_options.gni")
-import("//net/features.gni")
 import("//mojo/public/tools/bindings/mojom.gni")
+import("//net/features.gni")
 import("//ppapi/buildflags/buildflags.gni")
 import("//services/catalog/public/tools/catalog.gni")
 import("//services/service_manager/public/service_manifest.gni")
@@ -834,6 +834,7 @@ test("content_browsertests") {
     "../browser/security_exploit_browsertest.cc",
     "../browser/service_manager/service_manager_context_browsertest.cc",
     "../browser/service_worker/service_worker_browsertest.cc",
+    "../browser/service_worker/service_worker_clients_api_browsertest.cc",
     "../browser/session_history_browsertest.cc",
     "../browser/shape_detection/shape_detection_browsertest.cc",
     "../browser/shared_worker/worker_browsertest.cc",