Skip to content

Commit

Permalink
Prevent WindowClient.navigate() from cancelling a browser-initiated n…
Browse files Browse the repository at this point in the history
…avigation.

Otherwise, a service worker can prevent you from navigating where you
want to go via the omnibox.

Note: this is similar to WebContentsImpl::OnGoToEntryAtOffset() for
renderer-initiated history navigations.

Bug: 930154
Change-Id: I3a687ccc8ba4420d2369adb24f63c2702bdeeff1
Reviewed-on: https://chromium-review.googlesource.com/c/1477454
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Auto-Submit: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#633231}
  • Loading branch information
mfalken authored and Commit Bot committed Feb 19, 2019
1 parent 81845c8 commit e8bf23b
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 3 deletions.
15 changes: 14 additions & 1 deletion content/browser/service_worker/service_worker_client_utils.cc
Expand Up @@ -285,8 +285,21 @@ void NavigateClientOnUI(const GURL& url,
return;
}

int frame_tree_node_id = rfhi->frame_tree_node()->frame_tree_node_id();
// Reject the navigate() call if there is an ongoing browser-initiated
// navigation. Not rejecting it would allow websites to prevent the user from
// navigating away. See https://crbug.com/930154.
NavigationRequest* ongoing_navigation_request =
rfhi->frame_tree_node()->frame_tree()->root()->navigation_request();
if (ongoing_navigation_request &&
ongoing_navigation_request->browser_initiated()) {
base::PostTaskWithTraits(
FROM_HERE, {BrowserThread::IO},
base::BindOnce(std::move(callback), ChildProcessHost::kInvalidUniqueID,
MSG_ROUTING_NONE));
return;
}

int frame_tree_node_id = rfhi->frame_tree_node()->frame_tree_node_id();
Navigator* navigator = rfhi->frame_tree_node()->navigator();
navigator->RequestOpenURL(
rfhi, url, url::Origin::Create(script_url), false /* uses_post */,
Expand Down
Expand Up @@ -258,6 +258,36 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerClientsApiBrowserTest,
EXPECT_EQ(title, title_watcher.WaitAndGetTitle());
}

// Tests a WindowClient.navigate() call during a browser-initiated navigation.
// Regression test for https://crbug.com/930154.
IN_PROC_BROWSER_TEST_F(ServiceWorkerClientsApiBrowserTest,
NavigateDuringBrowserNavigation) {
// 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 test page.
EXPECT_TRUE(NavigateToURL(
shell(),
embedded_test_server()->GetURL("/service_worker/request_navigate.html")));

// Start a browser-initiated navigation.
GURL url(embedded_test_server()->GetURL("/title1.html"));
TestNavigationManager navigation(shell()->web_contents(), url);
shell()->LoadURL(url);
EXPECT_TRUE(navigation.WaitForRequestStart());

// Have the service worker call client.navigate() to try to go to another
// URL. It should fail.
EXPECT_EQ("navigate failed", EvalJs(shell(), "requestToNavigate();"));

// The browser-initiated navigation should finish.
navigation.WaitForNavigationFinished(); // Resume navigation.
EXPECT_TRUE(navigation.was_successful());
}

// Tests a successful Clients.openWindow() call.
IN_PROC_BROWSER_TEST_F(ServiceWorkerClientsApiBrowserTest, OpenWindow) {
ActivatedServiceWorkerObserver observer;
Expand Down
8 changes: 6 additions & 2 deletions content/test/data/service_worker/client_api_worker.js
Expand Up @@ -2,10 +2,14 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

self.addEventListener('message', event => {
self.addEventListener('message', async (event) => {
if (event.data.command == 'navigate') {
const url = event.data.url;
event.source.navigate(url);
try {
await event.source.navigate(url);
} catch (err) {
event.source.postMessage('navigate failed');
}
}
});

Expand Down
17 changes: 17 additions & 0 deletions content/test/data/service_worker/request_navigate.html
@@ -0,0 +1,17 @@
<!DOCTYPE html>
<meta charset="utf-8">
<title>Request navigate</title>
<script>
// Asks the service worker to call WindowClient.navigate() on us. It will
// message back the result if it failed, or else we navigate away.
async function requestToNavigate() {
const sawMessage = new Promise((resolve) => {
navigator.serviceWorker.onmessage = (event) => {
resolve(event.data);
};
});
const registration = await navigator.serviceWorker.ready;
registration.active.postMessage({command: 'navigate', url: 'empty.html'});
return await sawMessage;
}
</script>

0 comments on commit e8bf23b

Please sign in to comment.