Skip to content

Commit

Permalink
Add ref count to service workers for extension API.
Browse files Browse the repository at this point in the history
We need a way to keep a service worker alive
1) during extension function's request->response roundtrip completes.
2) when an event is about to be dispatched to a (stopped) service worker.

This CL shows a way to do chromium#1. chromium#2 can follow later.

The CL adds plumbing to expose functions to increment/decrement ref
counting to an ServiceWorkerVersion. This is done by adding a way to
add "external requests" to a ServiceWorkerVersion: when a worker has
external requests pending, it will be considered to be in working state,
i.e ServiceWorkerVersion::HasWork() will return true.
The public interface is exposed through ServiceWorkerContext. And the
interface methods expect a GUID/nonce for each such requests from service
worker renderer:
ServiceWorkerContext::StartingExternalRequest() and
ServiceWorkerContext::FinishedExternalRequest()

Extension APIs that are expected to be long running aren't handled in
this CL. For example: an extension API showing a dialog to user that
waits for user action.

BUG=602442
Test=There's no easy way to test it without tweaking the code, I've
used the following steps to make sure that we keep SW alive when an extension
API is in-flight:
Change the stop worker idle timeout and worker timeout to sth small, e.g. 3s.
Call an extension function that runs for 7s (> 3s + 3s). Without the CL, the
extension function's callback won't be called because the worker would shut
down after 6s.
The added test ServiceWorkerTest.WorkerRefCount tests this at a bit lower level:
by checking ref count (= count of external requests for a ServiceWorkerVersion).

Review-Url: https://codereview.chromium.org/2166523003
Cr-Commit-Position: refs/heads/master@{#425824}
  • Loading branch information
lazyboy authored and Commit bot committed Oct 18, 2016
1 parent 44a5f92 commit 4c82177
Show file tree
Hide file tree
Showing 41 changed files with 572 additions and 84 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "content/public/browser/render_view_host.h"
#include "content/public/browser/resource_dispatcher_host.h"
#include "content/public/browser/site_instance.h"
#include "content/public/browser/storage_partition.h"
#include "content/public/browser/vpn_service_proxy.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/content_switches.h"
Expand Down Expand Up @@ -560,7 +561,8 @@ void ChromeContentBrowserClientExtensionsPart::RenderProcessWillLaunch(
host->AddFilter(new ExtensionsGuestViewMessageFilter(id, profile));
if (extensions::ExtensionsClient::Get()
->ExtensionAPIEnabledInExtensionServiceWorkers()) {
host->AddFilter(new ExtensionServiceWorkerMessageFilter(id, profile));
host->AddFilter(new ExtensionServiceWorkerMessageFilter(
id, profile, host->GetStoragePartition()->GetServiceWorkerContext()));
}
extension_web_request_api_helpers::SendExtensionWebRequestStatusToHost(host);
}
Expand Down
77 changes: 77 additions & 0 deletions chrome/browser/extensions/service_worker_apitest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
#include "content/public/browser/navigation_controller.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/permission_type.h"
#include "content/public/browser/service_worker_context.h"
#include "content/public/browser/storage_partition.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/content_switches.h"
#include "content/public/common/origin_util.h"
Expand Down Expand Up @@ -162,6 +164,24 @@ class ServiceWorkerTest : public ExtensionApiTest {
return ExtractInnerText(Navigate(url));
}

size_t GetWorkerRefCount(const GURL& origin) {
content::ServiceWorkerContext* sw_context =
content::BrowserContext::GetDefaultStoragePartition(
browser()->profile())
->GetServiceWorkerContext();
base::RunLoop run_loop;
size_t ref_count = 0;
auto set_ref_count = [](size_t* ref_count, base::RunLoop* run_loop,
size_t external_request_count) {
*ref_count = external_request_count;
run_loop->Quit();
};
sw_context->CountExternalRequestsForTest(
origin, base::Bind(set_ref_count, &ref_count, &run_loop));
run_loop.Run();
return ref_count;
}

private:
// Sets the channel to "stable".
// Not useful after we've opened extension Service Workers to stable
Expand Down Expand Up @@ -630,6 +650,63 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerTest, TabsCreate) {
EXPECT_EQ(starting_tab_count, browser()->tab_strip_model()->count());
}

// Tests that worker ref count increments while extension API function is
// active.
IN_PROC_BROWSER_TEST_F(ServiceWorkerTest, WorkerRefCount) {
// Extensions APIs from SW are only enabled on trunk.
ScopedCurrentChannel current_channel_override(version_info::Channel::UNKNOWN);
const Extension* extension = LoadExtensionWithFlags(
test_data_dir_.AppendASCII("service_worker/api_worker_ref_count"),
kFlagNone);
ASSERT_TRUE(extension);
ui_test_utils::NavigateToURL(browser(),
extension->GetResourceURL("page.html"));
content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();

ExtensionTestMessageListener worker_start_listener("WORKER STARTED", false);
worker_start_listener.set_failure_message("FAILURE");
ASSERT_TRUE(
content::ExecuteScript(web_contents, "window.runServiceWorker()"));
ASSERT_TRUE(worker_start_listener.WaitUntilSatisfied());

// Service worker should have no pending requests because it hasn't peformed
// any extension API request yet.
EXPECT_EQ(0u, GetWorkerRefCount(extension->url()));

ExtensionTestMessageListener worker_listener("CHECK_REF_COUNT", true);
worker_listener.set_failure_message("FAILURE");
ASSERT_TRUE(content::ExecuteScript(web_contents, "window.testSendMessage()"));
ASSERT_TRUE(worker_listener.WaitUntilSatisfied());

// Service worker should have exactly one pending request because
// chrome.test.sendMessage() API call is in-flight.
EXPECT_EQ(1u, GetWorkerRefCount(extension->url()));

// Peform another extension API request while one is ongoing.
{
ExtensionTestMessageListener listener("CHECK_REF_COUNT", true);
listener.set_failure_message("FAILURE");
ASSERT_TRUE(
content::ExecuteScript(web_contents, "window.testSendMessage()"));
ASSERT_TRUE(listener.WaitUntilSatisfied());

// Service worker currently has two extension API requests in-flight.
EXPECT_EQ(2u, GetWorkerRefCount(extension->url()));
// Finish executing the nested chrome.test.sendMessage() first.
listener.Reply("Hello world");
}

ExtensionTestMessageListener extension_listener("SUCCESS", false);
extension_listener.set_failure_message("FAILURE");
// Finish executing chrome.test.sendMessage().
worker_listener.Reply("Hello world");
ASSERT_TRUE(extension_listener.WaitUntilSatisfied());

// The ref count should drop to 0.
EXPECT_EQ(0u, GetWorkerRefCount(extension->url()));
}

// This test loads a web page that has an iframe pointing to a
// chrome-extension:// URL. The URL is listed in the extension's
// web_accessible_resources. Initially the iframe is served from the extension's
Expand Down
8 changes: 4 additions & 4 deletions chrome/renderer/chrome_content_renderer_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1392,23 +1392,23 @@ void ChromeContentRendererClient::RunScriptsAtDocumentEnd(
void ChromeContentRendererClient::
DidInitializeServiceWorkerContextOnWorkerThread(
v8::Local<v8::Context> context,
int embedded_worker_id,
int64_t service_worker_version_id,
const GURL& url) {
#if defined(ENABLE_EXTENSIONS)
ChromeExtensionsRendererClient::GetInstance()
->extension_dispatcher()
->DidInitializeServiceWorkerContextOnWorkerThread(
context, embedded_worker_id, url);
context, service_worker_version_id, url);
#endif
}

void ChromeContentRendererClient::WillDestroyServiceWorkerContextOnWorkerThread(
v8::Local<v8::Context> context,
int embedded_worker_id,
int64_t service_worker_version_id,
const GURL& url) {
#if defined(ENABLE_EXTENSIONS)
extensions::Dispatcher::WillDestroyServiceWorkerContextOnWorkerThread(
context, embedded_worker_id, url);
context, service_worker_version_id, url);
#endif
}

Expand Down
4 changes: 2 additions & 2 deletions chrome/renderer/chrome_content_renderer_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,11 @@ class ChromeContentRendererClient : public content::ContentRendererClient {
void RunScriptsAtDocumentEnd(content::RenderFrame* render_frame) override;
void DidInitializeServiceWorkerContextOnWorkerThread(
v8::Local<v8::Context> context,
int embedded_worker_id,
int64_t service_worker_version_id,
const GURL& url) override;
void WillDestroyServiceWorkerContextOnWorkerThread(
v8::Local<v8::Context> context,
int embedded_worker_id,
int64_t service_worker_version_id,
const GURL& url) override;
bool ShouldEnforceWebRTCRoutingPreferences() override;
GURL OverrideFlashEmbedWithHTML(const GURL& url) override;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "Tests that worker is kept alive during extension API call.",
"version": "1.0",
"manifest_version": 2
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<script src="page.js"></script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// 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.

var worker = null;
var FAILURE_MESSAGE = 'FAILURE';

window.runServiceWorker = function() {
navigator.serviceWorker.register('sw.js').then(function() {
return navigator.serviceWorker.ready;
}).then(function(registration) {
worker = registration.active;
chrome.test.sendMessage('WORKER STARTED');
}).catch(function(err) {
chrome.test.sendMessage(FAILURE_MESSAGE);
});
};

window.testSendMessage = function() {
if (worker == null) {
chrome.test.sendMessage(FAILURE_MESSAGE);
return;
}
var channel = new MessageChannel();
channel.port1.onmessage = function(e) {
if (e.data == 'Worker reply: Hello world') {
chrome.test.sendMessage('SUCCESS');
} else {
chrome.test.sendMessage(FAILURE_MESSAGE);
}
};
worker.postMessage('sendMessageTest', [channel.port2]);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// 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.

self.onmessage = function(e) {
var fail = function() {
e.ports[0].postMessage('FAILURE');
};
if (e.data == 'sendMessageTest') {
try {
chrome.test.sendMessage('CHECK_REF_COUNT', function(reply) {
e.ports[0].postMessage('Worker reply: ' + reply);
});
} catch (e) {
fail();
}
} else {
fail();
}
};
49 changes: 49 additions & 0 deletions content/browser/service_worker/service_worker_context_wrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "base/threading/sequenced_worker_pool.h"
#include "base/threading/thread_task_runner_handle.h"
#include "content/browser/renderer_host/render_process_host_impl.h"
#include "content/browser/service_worker/embedded_worker_status.h"
#include "content/browser/service_worker/service_worker_context_core.h"
#include "content/browser/service_worker/service_worker_context_observer.h"
#include "content/browser/service_worker/service_worker_process_manager.h"
Expand Down Expand Up @@ -579,6 +580,32 @@ void ServiceWorkerContextWrapper::CheckHasServiceWorker(
callback));
}

void ServiceWorkerContextWrapper::CountExternalRequestsForTest(
const GURL& origin,
const CountExternalRequestsCallback& callback) {
if (!BrowserThread::CurrentlyOn(BrowserThread::IO)) {
BrowserThread::PostTask(
BrowserThread::IO, FROM_HERE,
base::Bind(&ServiceWorkerContextWrapper::CountExternalRequestsForTest,
this, origin, callback));
return;
}

std::vector<ServiceWorkerVersionInfo> live_version_info =
GetAllLiveVersionInfo();
size_t pending_external_request_count = 0;
for (const ServiceWorkerVersionInfo& info : live_version_info) {
ServiceWorkerVersion* version = GetLiveVersion(info.version_id);
if (version && version->scope().GetOrigin() == origin) {
pending_external_request_count =
version->GetExternalRequestCountForTest();
break;
}
}
BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
base::Bind(callback, pending_external_request_count));
}

void ServiceWorkerContextWrapper::ClearAllServiceWorkersForTest(
const base::Closure& callback) {
if (!BrowserThread::CurrentlyOn(BrowserThread::IO)) {
Expand Down Expand Up @@ -832,6 +859,28 @@ void ServiceWorkerContextWrapper::ShutdownOnIO() {
context_core_.reset();
}

bool ServiceWorkerContextWrapper::StartingExternalRequest(
int64_t service_worker_version_id,
const std::string& request_uuid) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
ServiceWorkerVersion* version =
context()->GetLiveVersion(service_worker_version_id);
if (!version)
return false;
return version->StartExternalRequest(request_uuid);
}

bool ServiceWorkerContextWrapper::FinishedExternalRequest(
int64_t service_worker_version_id,
const std::string& request_uuid) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
ServiceWorkerVersion* version =
context()->GetLiveVersion(service_worker_version_id);
if (!version)
return false;
return version->FinishExternalRequest(request_uuid);
}

void ServiceWorkerContextWrapper::DidDeleteAndStartOver(
ServiceWorkerStatusCode status) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,20 @@ class CONTENT_EXPORT ServiceWorkerContextWrapper
const GURL& url,
const GURL& other_url,
const CheckHasServiceWorkerCallback& callback) override;
void CountExternalRequestsForTest(
const GURL& url,
const CountExternalRequestsCallback& callback) override;
void StopAllServiceWorkersForOrigin(const GURL& origin) override;
void ClearAllServiceWorkersForTest(const base::Closure& callback) override;
void StartServiceWorkerForNavigationHint(
const GURL& document_url,
blink::WebNavigationHintType type,
int render_process_id,
const ResultCallback& callback) override;
bool StartingExternalRequest(int64_t service_worker_version_id,
const std::string& request_uuid) override;
bool FinishedExternalRequest(int64_t service_worker_version_id,
const std::string& request_uuid) override;

// These methods must only be called from the IO thread.
ServiceWorkerRegistration* GetLiveRegistration(int64_t registration_id);
Expand Down
7 changes: 7 additions & 0 deletions content/browser/service_worker/service_worker_metrics.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ std::string EventTypeToSuffix(ServiceWorkerMetrics::EventType event_type) {
return "_NAVIGATION_HINT_LINK_TAP_UNCONFIRMED";
case ServiceWorkerMetrics::EventType::NAVIGATION_HINT_LINK_TAP_DOWN:
return "_NAVIGATION_HINT_LINK_TAP_DOWN";
case ServiceWorkerMetrics::EventType::EXTERNAL_REQUEST:
return "_EXTERNAL_REQUEST";
case ServiceWorkerMetrics::EventType::NUM_TYPES:
NOTREACHED() << static_cast<int>(event_type);
}
Expand Down Expand Up @@ -227,6 +229,8 @@ const char* ServiceWorkerMetrics::EventTypeToString(EventType event_type) {
return "Navigation Hint Link Tap Unconfirmed";
case EventType::NAVIGATION_HINT_LINK_TAP_DOWN:
return "Navigation Hint Link Tap Down";
case EventType::EXTERNAL_REQUEST:
return "External Request";
case EventType::NUM_TYPES:
break;
}
Expand Down Expand Up @@ -565,6 +569,9 @@ void ServiceWorkerMetrics::RecordEventDuration(EventType event,
UMA_HISTOGRAM_MEDIUM_TIMES("ServiceWorker.ExtendableMessageEvent.Time",
time);
break;
case EventType::EXTERNAL_REQUEST:
UMA_HISTOGRAM_MEDIUM_TIMES("ServiceWorker.ExternalRequest.Time", time);
break;
// Those navigation hints should not be sent as request events.
case EventType::NAVIGATION_HINT_LINK_MOUSE_DOWN:
case EventType::NAVIGATION_HINT_LINK_TAP_UNCONFIRMED:
Expand Down
3 changes: 3 additions & 0 deletions content/browser/service_worker/service_worker_metrics.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,9 @@ class ServiceWorkerMetrics {
NAVIGATION_HINT_LINK_MOUSE_DOWN = 18,
NAVIGATION_HINT_LINK_TAP_UNCONFIRMED = 19,
NAVIGATION_HINT_LINK_TAP_DOWN = 20,
// Used when external consumers want to add a request to
// ServiceWorkerVersion to keep it alive.
EXTERNAL_REQUEST = 21,
// Add new events to record here.
NUM_TYPES
};
Expand Down
Loading

0 comments on commit 4c82177

Please sign in to comment.