diff --git a/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc b/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc index 78d329947b4c6f..73dc5a2fe8740b 100644 --- a/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc +++ b/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc @@ -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" @@ -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); } diff --git a/chrome/browser/extensions/service_worker_apitest.cc b/chrome/browser/extensions/service_worker_apitest.cc index 227584f99c1a18..9db618f80f6ba1 100644 --- a/chrome/browser/extensions/service_worker_apitest.cc +++ b/chrome/browser/extensions/service_worker_apitest.cc @@ -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" @@ -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 @@ -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 diff --git a/chrome/renderer/chrome_content_renderer_client.cc b/chrome/renderer/chrome_content_renderer_client.cc index 4f0bc44c8f6382..78884c5ef0d74a 100644 --- a/chrome/renderer/chrome_content_renderer_client.cc +++ b/chrome/renderer/chrome_content_renderer_client.cc @@ -1392,23 +1392,23 @@ void ChromeContentRendererClient::RunScriptsAtDocumentEnd( void ChromeContentRendererClient:: DidInitializeServiceWorkerContextOnWorkerThread( v8::Local 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 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 } diff --git a/chrome/renderer/chrome_content_renderer_client.h b/chrome/renderer/chrome_content_renderer_client.h index f3665481e50f26..c32efe22fda3d6 100644 --- a/chrome/renderer/chrome_content_renderer_client.h +++ b/chrome/renderer/chrome_content_renderer_client.h @@ -178,11 +178,11 @@ class ChromeContentRendererClient : public content::ContentRendererClient { void RunScriptsAtDocumentEnd(content::RenderFrame* render_frame) override; void DidInitializeServiceWorkerContextOnWorkerThread( v8::Local context, - int embedded_worker_id, + int64_t service_worker_version_id, const GURL& url) override; void WillDestroyServiceWorkerContextOnWorkerThread( v8::Local context, - int embedded_worker_id, + int64_t service_worker_version_id, const GURL& url) override; bool ShouldEnforceWebRTCRoutingPreferences() override; GURL OverrideFlashEmbedWithHTML(const GURL& url) override; diff --git a/chrome/test/data/extensions/api_test/service_worker/api_worker_ref_count/manifest.json b/chrome/test/data/extensions/api_test/service_worker/api_worker_ref_count/manifest.json new file mode 100644 index 00000000000000..875562c948c8f9 --- /dev/null +++ b/chrome/test/data/extensions/api_test/service_worker/api_worker_ref_count/manifest.json @@ -0,0 +1,5 @@ +{ + "name": "Tests that worker is kept alive during extension API call.", + "version": "1.0", + "manifest_version": 2 +} diff --git a/chrome/test/data/extensions/api_test/service_worker/api_worker_ref_count/page.html b/chrome/test/data/extensions/api_test/service_worker/api_worker_ref_count/page.html new file mode 100644 index 00000000000000..f6e2de85f420ee --- /dev/null +++ b/chrome/test/data/extensions/api_test/service_worker/api_worker_ref_count/page.html @@ -0,0 +1 @@ + diff --git a/chrome/test/data/extensions/api_test/service_worker/api_worker_ref_count/page.js b/chrome/test/data/extensions/api_test/service_worker/api_worker_ref_count/page.js new file mode 100644 index 00000000000000..0f26b96e0e51cd --- /dev/null +++ b/chrome/test/data/extensions/api_test/service_worker/api_worker_ref_count/page.js @@ -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]); +}; diff --git a/chrome/test/data/extensions/api_test/service_worker/api_worker_ref_count/sw.js b/chrome/test/data/extensions/api_test/service_worker/api_worker_ref_count/sw.js new file mode 100644 index 00000000000000..70945d31734b0c --- /dev/null +++ b/chrome/test/data/extensions/api_test/service_worker/api_worker_ref_count/sw.js @@ -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(); + } +}; diff --git a/content/browser/service_worker/service_worker_context_wrapper.cc b/content/browser/service_worker/service_worker_context_wrapper.cc index eeac4d3bfa1a06..686e5ea57d812c 100644 --- a/content/browser/service_worker/service_worker_context_wrapper.cc +++ b/content/browser/service_worker/service_worker_context_wrapper.cc @@ -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" @@ -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 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)) { @@ -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); diff --git a/content/browser/service_worker/service_worker_context_wrapper.h b/content/browser/service_worker/service_worker_context_wrapper.h index 12c8b7220cd833..905e72552eeb86 100644 --- a/content/browser/service_worker/service_worker_context_wrapper.h +++ b/content/browser/service_worker/service_worker_context_wrapper.h @@ -105,6 +105,9 @@ 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( @@ -112,6 +115,10 @@ class CONTENT_EXPORT ServiceWorkerContextWrapper 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); diff --git a/content/browser/service_worker/service_worker_metrics.cc b/content/browser/service_worker/service_worker_metrics.cc index e2459fd76c08e7..310e76e3d8a69c 100644 --- a/content/browser/service_worker/service_worker_metrics.cc +++ b/content/browser/service_worker/service_worker_metrics.cc @@ -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(event_type); } @@ -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; } @@ -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: diff --git a/content/browser/service_worker/service_worker_metrics.h b/content/browser/service_worker/service_worker_metrics.h index c57ae0f33433da..9f94599399eb06 100644 --- a/content/browser/service_worker/service_worker_metrics.h +++ b/content/browser/service_worker/service_worker_metrics.h @@ -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 }; diff --git a/content/browser/service_worker/service_worker_version.cc b/content/browser/service_worker/service_worker_version.cc index ee8408d063172b..5bdeb7b37524cd 100644 --- a/content/browser/service_worker/service_worker_version.cc +++ b/content/browser/service_worker/service_worker_version.cc @@ -566,6 +566,24 @@ int ServiceWorkerVersion::StartRequestWithCustomTimeout( return request_id; } +bool ServiceWorkerVersion::StartExternalRequest( + const std::string& request_uuid) { + // It's possible that the renderer is lying or the version started stopping + // right around the time of the IPC. + if (running_status() != EmbeddedWorkerStatus::RUNNING) + return false; + + if (external_request_uuid_to_request_id_.count(request_uuid) > 0u) + return false; + + int request_id = + StartRequest(ServiceWorkerMetrics::EventType::EXTERNAL_REQUEST, + base::Bind(&ServiceWorkerVersion::CleanUpExternalRequest, + this, request_uuid)); + external_request_uuid_to_request_id_[request_uuid] = request_id; + return true; +} + bool ServiceWorkerVersion::FinishRequest(int request_id, bool was_handled, base::Time dispatch_event_time) { @@ -593,6 +611,27 @@ bool ServiceWorkerVersion::FinishRequest(int request_id, return true; } +bool ServiceWorkerVersion::FinishExternalRequest( + const std::string& request_uuid) { + // It's possible that the renderer is lying or the version started stopping + // right around the time of the IPC. + if (running_status() != EmbeddedWorkerStatus::RUNNING) + return false; + + RequestUUIDToRequestIDMap::iterator iter = + external_request_uuid_to_request_id_.find(request_uuid); + if (iter != external_request_uuid_to_request_id_.end()) { + int request_id = iter->second; + external_request_uuid_to_request_id_.erase(iter); + return FinishRequest(request_id, true, base::Time::Now()); + } + + // It is possible that the request was cancelled or timed out before and we + // won't find it in |external_request_uuid_to_request_id_|. + // Return true so we don't kill the process. + return true; +} + void ServiceWorkerVersion::RunAfterStartWorker( ServiceWorkerMetrics::EventType purpose, const base::Closure& task, @@ -1792,6 +1831,7 @@ void ServiceWorkerVersion::OnStoppedInternal(EmbeddedWorkerStatus old_status) { iter.Advance(); } pending_requests_.Clear(); + external_request_uuid_to_request_id_.clear(); // Close all mojo services. This will also fire and clear all callbacks // for messages that are still outstanding for those services. @@ -1831,4 +1871,12 @@ void ServiceWorkerVersion::FinishStartWorker(ServiceWorkerStatusCode status) { RunCallbacks(this, &start_callbacks_, status); } +void ServiceWorkerVersion::CleanUpExternalRequest( + const std::string& request_uuid, + ServiceWorkerStatusCode status) { + if (status == SERVICE_WORKER_OK) + return; + external_request_uuid_to_request_id_.erase(request_uuid); +} + } // namespace content diff --git a/content/browser/service_worker/service_worker_version.h b/content/browser/service_worker/service_worker_version.h index 6aef95e382098b..d3bd26af7dbb55 100644 --- a/content/browser/service_worker/service_worker_version.h +++ b/content/browser/service_worker/service_worker_version.h @@ -236,6 +236,12 @@ class CONTENT_EXPORT ServiceWorkerVersion const base::TimeDelta& timeout, TimeoutBehavior timeout_behavior); + // Starts a request of type EventType::EXTERNAL_REQUEST. + // Provides a mechanism to external clients to keep the worker running. + // |request_uuid| is a GUID for clients to identify the request. + // Returns true if the request was successfully scheduled to starrt. + bool StartExternalRequest(const std::string& request_uuid); + // Informs ServiceWorkerVersion that an event has finished being dispatched. // Returns false if no pending requests with the provided id exist, for // example if the request has already timed out. @@ -246,6 +252,11 @@ class CONTENT_EXPORT ServiceWorkerVersion bool was_handled, base::Time dispatch_event_time); + // Finishes an external request that was started by StartExternalRequest(). + // Returns false if there was an error finishing the request: e.g. the request + // was not found or the worker already terminated. + bool FinishExternalRequest(const std::string& request_uuid); + // Connects to a specific mojo service exposed by the (running) service // worker. If a connection to a service for the same Interface already exists // this will return that existing connection. The |request_id| must be a value @@ -383,6 +394,11 @@ class CONTENT_EXPORT ServiceWorkerVersion // requests, in-progress streaming URLRequestJobs, or pending start callbacks. bool HasWork() const; + // Returns the number of pending external request count of this worker. + size_t GetExternalRequestCountForTest() const { + return external_request_uuid_to_request_id_.size(); + } + private: friend class base::RefCounted; friend class ServiceWorkerMetrics; @@ -695,6 +711,10 @@ class CONTENT_EXPORT ServiceWorkerVersion // callbacks. void FinishStartWorker(ServiceWorkerStatusCode status); + // Removes any pending external request that has GUID of |request_uuid|. + void CleanUpExternalRequest(const std::string& request_uuid, + ServiceWorkerStatusCode status); + const int64_t version_id_; const int64_t registration_id_; const GURL script_url_; @@ -714,6 +734,11 @@ class CONTENT_EXPORT ServiceWorkerVersion // fetch, sync, etc. events. IDMap pending_requests_; + // Container for pending external requests for this service worker. + // (key, value): (request uuid, request id). + using RequestUUIDToRequestIDMap = std::map; + RequestUUIDToRequestIDMap external_request_uuid_to_request_id_; + // Stores all open connections to mojo services. Maps the service name to // the actual interface pointer. When a connection is closed it is removed // from this map. diff --git a/content/public/browser/service_worker_context.h b/content/public/browser/service_worker_context.h index 47f355c734ae46..c095f9d1314c7f 100644 --- a/content/public/browser/service_worker_context.h +++ b/content/public/browser/service_worker_context.h @@ -23,15 +23,18 @@ class ServiceWorkerContext { public: // https://rawgithub.com/slightlyoff/ServiceWorker/master/spec/service_worker/index.html#url-scope: // roughly, must be of the form "//*". - typedef GURL Scope; + using Scope = GURL; - typedef base::Callback ResultCallback; + using ResultCallback = base::Callback; - typedef base::Callback& - usage_info)> GetUsageInfoCallback; + using GetUsageInfoCallback = base::Callback& usage_info)>; - typedef base::Callback - CheckHasServiceWorkerCallback; + using CheckHasServiceWorkerCallback = + base::Callback; + + using CountExternalRequestsCallback = + base::Callback; // Registers the header name which should not be passed to the ServiceWorker. // Must be called from the IO thread. @@ -60,6 +63,21 @@ class ServiceWorkerContext { const GURL& script_url, const ResultCallback& callback) = 0; + // Mechanism for embedder to increment/decrement ref count of a service + // worker. + // Embedders can call StartingExternalRequest() while it is performing some + // work with the worker. The worker is considered to be working until embedder + // calls FinishedExternalRequest(). This ensures that content/ does not + // shut the worker down while embedder is expecting the worker to be kept + // alive. + // + // Must be called from the IO thread. Returns whether or not changing the ref + // count succeeded. + virtual bool StartingExternalRequest(int64_t service_worker_version_id, + const std::string& request_uuid) = 0; + virtual bool FinishedExternalRequest(int64_t service_worker_version_id, + const std::string& request_uuid) = 0; + // Equivalent to calling navigator.serviceWorker.unregister(pattern) from a // renderer, except that |pattern| is an absolute URL instead of relative to // some current origin. |callback| is passed true when the JS promise is @@ -96,6 +114,12 @@ class ServiceWorkerContext { const GURL& other_url, const CheckHasServiceWorkerCallback& callback) = 0; + // Returns the pending external request count for the worker with the + // specified |origin| via |callback|. + virtual void CountExternalRequestsForTest( + const GURL& origin, + const CountExternalRequestsCallback& callback) = 0; + // Stops all running workers on the given |origin|. // // This function can be called from any thread. diff --git a/content/public/renderer/content_renderer_client.h b/content/public/renderer/content_renderer_client.h index fc9fba976e11aa..32c5092d952de3 100644 --- a/content/public/renderer/content_renderer_client.h +++ b/content/public/renderer/content_renderer_client.h @@ -346,14 +346,14 @@ class CONTENT_EXPORT ContentRendererClient { // is called from the worker thread. virtual void DidInitializeServiceWorkerContextOnWorkerThread( v8::Local context, - int embedded_worker_id, + int64_t service_worker_version_id, const GURL& url) {} // Notifies that a service worker context will be destroyed. This function // is called from the worker thread. virtual void WillDestroyServiceWorkerContextOnWorkerThread( v8::Local context, - int embedded_worker_id, + int64_t service_worker_version_id, const GURL& url) {} // Whether this renderer should enforce preferences related to the WebRTC diff --git a/content/renderer/service_worker/service_worker_context_client.cc b/content/renderer/service_worker/service_worker_context_client.cc index a08b1ded5a3e5d..55c29d35e21f35 100644 --- a/content/renderer/service_worker/service_worker_context_client.cc +++ b/content/renderer/service_worker/service_worker_context_client.cc @@ -459,7 +459,7 @@ void ServiceWorkerContextClient::didInitializeWorkerContext( GetContentClient() ->renderer() ->DidInitializeServiceWorkerContextOnWorkerThread( - context, embedded_worker_id_, script_url_); + context, service_worker_version_id_, script_url_); } void ServiceWorkerContextClient::willDestroyWorkerContext( @@ -492,7 +492,7 @@ void ServiceWorkerContextClient::willDestroyWorkerContext( g_worker_client_tls.Pointer()->Set(NULL); GetContentClient()->renderer()->WillDestroyServiceWorkerContextOnWorkerThread( - context, embedded_worker_id_, script_url_); + context, service_worker_version_id_, script_url_); } void ServiceWorkerContextClient::workerContextDestroyed() { diff --git a/extensions/browser/bad_message.cc b/extensions/browser/bad_message.cc index 520610a68e0f1e..bae713ea3764dd 100644 --- a/extensions/browser/bad_message.cc +++ b/extensions/browser/bad_message.cc @@ -6,20 +6,35 @@ #include "base/logging.h" #include "base/metrics/sparse_histogram.h" +#include "content/public/browser/browser_message_filter.h" #include "content/public/browser/render_process_host.h" namespace extensions { namespace bad_message { -void ReceivedBadMessage(content::RenderProcessHost* host, - BadMessageReason reason) { +namespace { + +void LogBadMessage(BadMessageReason reason) { LOG(ERROR) << "Terminating extension renderer for bad IPC message, reason " << reason; UMA_HISTOGRAM_SPARSE_SLOWLY("Stability.BadMessageTerminated.Extensions", reason); +} + +} // namespace + +void ReceivedBadMessage(content::RenderProcessHost* host, + BadMessageReason reason) { + LogBadMessage(reason); host->ShutdownForBadMessage( content::RenderProcessHost::CrashReportMode::GENERATE_CRASH_DUMP); } +void ReceivedBadMessage(content::BrowserMessageFilter* filter, + BadMessageReason reason) { + LogBadMessage(reason); + filter->ShutdownForBadMessage(); +} + } // namespace bad_message } // namespace extensions diff --git a/extensions/browser/bad_message.h b/extensions/browser/bad_message.h index bef0c712b388f9..2abea64171e62f 100644 --- a/extensions/browser/bad_message.h +++ b/extensions/browser/bad_message.h @@ -6,6 +6,7 @@ #define EXTENSIONS_BROWSER_BAD_MESSAGE_H_ namespace content { +class BrowserMessageFilter; class RenderProcessHost; } @@ -28,6 +29,8 @@ enum BadMessageReason { AVG_BAD_INST_ID = 4, AVG_BAD_EXT_ID = 5, AVG_NULL_AVG = 6, + // Invalid decrement of an Extensions SW ref count. + ESWMF_INVALID_DECREMENT_ACTIVIY = 7, // Please add new elements here. The naming convention is abbreviated class // name (e.g. ExtensionHost becomes EH) plus a unique description of the // reason. After making changes, you MUST update histograms.xml by running: @@ -41,6 +44,12 @@ enum BadMessageReason { void ReceivedBadMessage(content::RenderProcessHost* host, BadMessageReason reason); +// Called when a browser message filter receives a bad IPC message from a +// renderer or other child process. Logs the event, records a histogram metric +// for the |reason|, and terminates the process for |filter|. +void ReceivedBadMessage(content::BrowserMessageFilter* filter, + BadMessageReason reason); + } // namespace bad_message } // namespace extensions diff --git a/extensions/browser/extension_function.cc b/extensions/browser/extension_function.cc index 042169340f22a4..359507c53aeeeb 100644 --- a/extensions/browser/extension_function.cc +++ b/extensions/browser/extension_function.cc @@ -450,11 +450,14 @@ void ExtensionFunction::SendResponseImpl(bool success) { UIThreadExtensionFunction::UIThreadExtensionFunction() : context_(nullptr), render_frame_host_(nullptr), - is_from_service_worker_(false) {} + service_worker_version_id_(extensions::kInvalidServiceWorkerVersionId) {} UIThreadExtensionFunction::~UIThreadExtensionFunction() { - if (dispatcher() && render_frame_host()) - dispatcher()->OnExtensionFunctionCompleted(extension()); + if (dispatcher() && (render_frame_host() || is_from_service_worker())) { + dispatcher()->OnExtensionFunctionCompleted(extension(), + is_from_service_worker()); + } + // The extension function should always respond to avoid leaks in the // renderer, dangling callbacks, etc. The exception is if the system is // shutting down. @@ -503,7 +506,7 @@ void UIThreadExtensionFunction::Destruct() const { void UIThreadExtensionFunction::SetRenderFrameHost( content::RenderFrameHost* render_frame_host) { // An extension function from Service Worker does not have a RenderFrameHost. - if (is_from_service_worker_) { + if (is_from_service_worker()) { DCHECK(!render_frame_host); return; } diff --git a/extensions/browser/extension_function.h b/extensions/browser/extension_function.h index a547b0e101ae34..89ac5a2d24c149 100644 --- a/extensions/browser/extension_function.h +++ b/extensions/browser/extension_function.h @@ -23,6 +23,7 @@ #include "content/public/common/console_message_level.h" #include "extensions/browser/extension_function_histogram_value.h" #include "extensions/browser/info_map.h" +#include "extensions/common/constants.h" #include "extensions/common/extension.h" #include "extensions/common/features/feature.h" #include "ipc/ipc_message.h" @@ -518,8 +519,8 @@ class UIThreadExtensionFunction : public ExtensionFunction { return dispatcher_.get(); } - void set_is_from_service_worker(bool value) { - is_from_service_worker_ = value; + void set_service_worker_version_id(int64_t version_id) { + service_worker_version_id_ = version_id; } // Gets the "current" web contents if any. If there is no associated web @@ -557,15 +558,20 @@ class UIThreadExtensionFunction : public ExtensionFunction { void Destruct() const override; + bool is_from_service_worker() const { + return service_worker_version_id_ != + extensions::kInvalidServiceWorkerVersionId; + } + // The dispatcher that will service this extension function call. base::WeakPtr dispatcher_; // The RenderFrameHost we will send responses to. content::RenderFrameHost* render_frame_host_; - // Whether or not this ExtensionFunction was called by an extension Service - // Worker. - bool is_from_service_worker_; + // If this ExtensionFunction was called by an extension Service Worker, then + // this contains the worker's version id. + int64_t service_worker_version_id_; std::unique_ptr tracker_; diff --git a/extensions/browser/extension_function_dispatcher.cc b/extensions/browser/extension_function_dispatcher.cc index b944303ad7315f..43469fb5a12732 100644 --- a/extensions/browser/extension_function_dispatcher.cc +++ b/extensions/browser/extension_function_dispatcher.cc @@ -24,6 +24,7 @@ #include "content/public/browser/render_process_host.h" #include "content/public/browser/render_process_host_observer.h" #include "content/public/browser/render_view_host.h" +#include "content/public/browser/service_worker_context.h" #include "content/public/browser/user_metrics.h" #include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents_observer.h" @@ -32,11 +33,13 @@ #include "extensions/browser/extension_function_registry.h" #include "extensions/browser/extension_registry.h" #include "extensions/browser/extension_system.h" +#include "extensions/browser/extension_util.h" #include "extensions/browser/extensions_browser_client.h" #include "extensions/browser/io_thread_extension_message_filter.h" #include "extensions/browser/process_manager.h" #include "extensions/browser/process_map.h" #include "extensions/browser/quota_service.h" +#include "extensions/common/constants.h" #include "extensions/common/extension_api.h" #include "extensions/common/extension_messages.h" #include "extensions/common/extension_set.h" @@ -60,6 +63,12 @@ void NotifyApiFunctionCalled(const std::string& extension_id, args); } +bool IsRequestFromServiceWorker( + const ExtensionHostMsg_Request_Params& request_params) { + return request_params.service_worker_version_id != + extensions::kInvalidServiceWorkerVersionId; +} + // Separate copy of ExtensionAPI used for IO thread extension functions. We need // this because ExtensionAPI has mutable data. It should be possible to remove // this once all the extension APIs are updated to the feature system. @@ -277,17 +286,18 @@ class ExtensionFunctionDispatcher::UIThreadWorkerResponseCallbackWrapper }; struct ExtensionFunctionDispatcher::WorkerResponseCallbackMapKey { - WorkerResponseCallbackMapKey(int render_process_id, int embedded_worker_id) + WorkerResponseCallbackMapKey(int render_process_id, + int64_t service_worker_version_id) : render_process_id(render_process_id), - embedded_worker_id(embedded_worker_id) {} + service_worker_version_id(service_worker_version_id) {} bool operator<(const WorkerResponseCallbackMapKey& other) const { - return std::tie(render_process_id, embedded_worker_id) < - std::tie(other.render_process_id, other.embedded_worker_id); + return std::tie(render_process_id, service_worker_version_id) < + std::tie(other.render_process_id, other.service_worker_version_id); } int render_process_id; - int embedded_worker_id; + int64_t service_worker_version_id; }; WindowController* @@ -418,9 +428,9 @@ void ExtensionFunctionDispatcher::Dispatch( callback_wrapper->CreateCallback(params.request_id)); } else { // Extension API from Service Worker. - DCHECK_GE(params.embedded_worker_id, 0); + DCHECK_NE(kInvalidServiceWorkerVersionId, params.service_worker_version_id); WorkerResponseCallbackMapKey key(render_process_id, - params.embedded_worker_id); + params.service_worker_version_id); UIThreadWorkerResponseCallbackWrapperMap::const_iterator iter = ui_thread_response_callback_wrappers_for_worker_.find(key); UIThreadWorkerResponseCallbackWrapper* callback_wrapper = nullptr; @@ -472,8 +482,9 @@ void ExtensionFunctionDispatcher::DispatchWithCallbackInternal( NOTREACHED(); return; } - if (params.embedded_worker_id != -1) { - function_ui->set_is_from_service_worker(true); + if (IsRequestFromServiceWorker(params)) { + function_ui->set_service_worker_version_id( + params.service_worker_version_id); } else { function_ui->SetRenderFrameHost(render_frame_host); } @@ -536,13 +547,16 @@ void ExtensionFunctionDispatcher::DispatchWithCallbackInternal( if (!registry->enabled_extensions().GetByID(params.extension_id)) return; - // We only adjust the keepalive count for UIThreadExtensionFunction for - // now, largely for simplicity's sake. This is OK because currently, only - // the webRequest API uses IOThreadExtensionFunction, and that API is not - // compatible with lazy background pages. - // TODO(lazyboy): API functions from extension Service Worker will incorrectly - // change keepalive count below. - process_manager->IncrementLazyKeepaliveCount(extension); + if (!IsRequestFromServiceWorker(params)) { + // Increment ref count for non-service worker extension API. Ref count for + // service worker extension API is handled separately on IO thread via IPC. + + // We only adjust the keepalive count for UIThreadExtensionFunction for + // now, largely for simplicity's sake. This is OK because currently, only + // the webRequest API uses IOThreadExtensionFunction, and that API is not + // compatible with lazy background pages. + process_manager->IncrementLazyKeepaliveCount(function->extension()); + } } void ExtensionFunctionDispatcher::RemoveWorkerCallbacksForProcess( @@ -560,10 +574,12 @@ void ExtensionFunctionDispatcher::RemoveWorkerCallbacksForProcess( } void ExtensionFunctionDispatcher::OnExtensionFunctionCompleted( - const Extension* extension) { - // TODO(lazyboy): API functions from extension Service Worker will incorrectly - // change keepalive count below. - if (extension) { + const Extension* extension, + bool is_from_service_worker) { + if (extension && !is_from_service_worker) { + // Decrement ref count for non-service worker extension API. Service + // worker extension API ref counts are handled separately on IO thread + // directly via IPC. ProcessManager::Get(browser_context_) ->DecrementLazyKeepaliveCount(extension); } diff --git a/extensions/browser/extension_function_dispatcher.h b/extensions/browser/extension_function_dispatcher.h index 074c2411204ada..9aa833f45edee0 100644 --- a/extensions/browser/extension_function_dispatcher.h +++ b/extensions/browser/extension_function_dispatcher.h @@ -101,7 +101,8 @@ class ExtensionFunctionDispatcher // Called when an ExtensionFunction is done executing, after it has sent // a response (if any) to the extension. - void OnExtensionFunctionCompleted(const Extension* extension); + void OnExtensionFunctionCompleted(const Extension* extension, + bool is_from_service_worker); // See the Delegate class for documentation on these methods. // TODO(devlin): None of these belong here. We should kill diff --git a/extensions/browser/extension_service_worker_message_filter.cc b/extensions/browser/extension_service_worker_message_filter.cc index fa3357e7903bf0..98195f401313b3 100644 --- a/extensions/browser/extension_service_worker_message_filter.cc +++ b/extensions/browser/extension_service_worker_message_filter.cc @@ -4,6 +4,8 @@ #include "extensions/browser/extension_service_worker_message_filter.h" +#include "content/public/browser/service_worker_context.h" +#include "extensions/browser/bad_message.h" #include "extensions/browser/extension_function_dispatcher.h" #include "extensions/common/extension_messages.h" @@ -11,9 +13,11 @@ namespace extensions { ExtensionServiceWorkerMessageFilter::ExtensionServiceWorkerMessageFilter( int render_process_id, - content::BrowserContext* context) + content::BrowserContext* context, + content::ServiceWorkerContext* service_worker_context) : content::BrowserMessageFilter(ExtensionWorkerMsgStart), render_process_id_(render_process_id), + service_worker_context_(service_worker_context), dispatcher_(new ExtensionFunctionDispatcher(context)) {} ExtensionServiceWorkerMessageFilter::~ExtensionServiceWorkerMessageFilter() { @@ -33,6 +37,10 @@ bool ExtensionServiceWorkerMessageFilter::OnMessageReceived( bool handled = true; IPC_BEGIN_MESSAGE_MAP(ExtensionServiceWorkerMessageFilter, message) IPC_MESSAGE_HANDLER(ExtensionHostMsg_RequestWorker, OnRequestWorker) + IPC_MESSAGE_HANDLER(ExtensionHostMsg_IncrementServiceWorkerActivity, + OnIncrementServiceWorkerActivity) + IPC_MESSAGE_HANDLER(ExtensionHostMsg_DecrementServiceWorkerActivity, + OnDecrementServiceWorkerActivity) IPC_MESSAGE_UNHANDLED(handled = false) IPC_END_MESSAGE_MAP() return handled; @@ -44,4 +52,27 @@ void ExtensionServiceWorkerMessageFilter::OnRequestWorker( dispatcher_->Dispatch(params, nullptr, render_process_id_); } +void ExtensionServiceWorkerMessageFilter::OnIncrementServiceWorkerActivity( + int64_t service_worker_version_id, + const std::string& request_uuid) { + DCHECK_CURRENTLY_ON(content::BrowserThread::IO); + // The worker might have already stopped before we got here, so the increment + // below might fail legitimately. Therefore, we do not send bad_message to the + // worker even if it fails. + service_worker_context_->StartingExternalRequest(service_worker_version_id, + request_uuid); +} + +void ExtensionServiceWorkerMessageFilter::OnDecrementServiceWorkerActivity( + int64_t service_worker_version_id, + const std::string& request_uuid) { + DCHECK_CURRENTLY_ON(content::BrowserThread::IO); + bool status = service_worker_context_->FinishedExternalRequest( + service_worker_version_id, request_uuid); + if (!status) { + bad_message::ReceivedBadMessage( + this, bad_message::ESWMF_INVALID_DECREMENT_ACTIVIY); + } +} + } // namespace extensions diff --git a/extensions/browser/extension_service_worker_message_filter.h b/extensions/browser/extension_service_worker_message_filter.h index 44f4f60a9203c3..f76bf9b43d214d 100644 --- a/extensions/browser/extension_service_worker_message_filter.h +++ b/extensions/browser/extension_service_worker_message_filter.h @@ -12,6 +12,7 @@ struct ExtensionHostMsg_Request_Params; namespace content { class BrowserContext; +class ServiceWorkerContext; } namespace extensions { @@ -22,8 +23,10 @@ class ExtensionFunctionDispatcher; class ExtensionServiceWorkerMessageFilter : public content::BrowserMessageFilter { public: - ExtensionServiceWorkerMessageFilter(int render_process_id, - content::BrowserContext* context); + ExtensionServiceWorkerMessageFilter( + int render_process_id, + content::BrowserContext* context, + content::ServiceWorkerContext* service_worker_context); // content::BrowserMessageFilter: bool OnMessageReceived(const IPC::Message& message) override; @@ -33,10 +36,18 @@ class ExtensionServiceWorkerMessageFilter private: ~ExtensionServiceWorkerMessageFilter() override; + // Message handlers. void OnRequestWorker(const ExtensionHostMsg_Request_Params& params); + void OnIncrementServiceWorkerActivity(int64_t service_worker_version_id, + const std::string& request_uuid); + void OnDecrementServiceWorkerActivity(int64_t service_worker_version_id, + const std::string& request_uuid); const int render_process_id_; + // Owned by the StoragePartition of our profile. + content::ServiceWorkerContext* service_worker_context_; + std::unique_ptr dispatcher_; diff --git a/extensions/browser/extension_util.cc b/extensions/browser/extension_util.cc index 4981790a9ed3b6..142e1e482dd1e1 100644 --- a/extensions/browser/extension_util.cc +++ b/extensions/browser/extension_util.cc @@ -4,6 +4,8 @@ #include "extensions/browser/extension_util.h" +#include "content/public/browser/browser_context.h" +#include "content/public/browser/site_instance.h" #include "extensions/browser/extension_prefs.h" #include "extensions/browser/extension_registry.h" #include "extensions/common/manifest_handlers/app_isolation_info.h" @@ -43,5 +45,16 @@ bool CanBeIncognitoEnabled(const Extension* extension) { extension->location() == Manifest::COMPONENT); } +content::StoragePartition* GetStoragePartitionForExtensionId( + const std::string& extension_id, + content::BrowserContext* browser_context) { + GURL site_url = content::SiteInstance::GetSiteForURL( + browser_context, Extension::GetBaseURLFromExtensionId(extension_id)); + content::StoragePartition* storage_partition = + content::BrowserContext::GetStoragePartitionForSite(browser_context, + site_url); + return storage_partition; +} + } // namespace util } // namespace extensions diff --git a/extensions/browser/extension_util.h b/extensions/browser/extension_util.h index cdd4218ad14be2..b3e823350c4a74 100644 --- a/extensions/browser/extension_util.h +++ b/extensions/browser/extension_util.h @@ -11,6 +11,7 @@ class GURL; namespace content { class BrowserContext; +class StoragePartition; } namespace extensions { @@ -34,6 +35,10 @@ bool SiteHasIsolatedStorage(const GURL& extension_site_url, // Returns true if the extension can be enabled in incognito mode. bool CanBeIncognitoEnabled(const Extension* extension); +content::StoragePartition* GetStoragePartitionForExtensionId( + const std::string& extension_id, + content::BrowserContext* browser_context); + } // namespace util } // namespace extensions diff --git a/extensions/common/api/_api_features.json b/extensions/common/api/_api_features.json index 10e15425c6db46..5380a15a5e340f 100644 --- a/extensions/common/api/_api_features.json +++ b/extensions/common/api/_api_features.json @@ -446,6 +446,7 @@ "blessed_extension", "blessed_web_page", "content_script", + "extension_service_worker", "unblessed_extension" ] }, { diff --git a/extensions/common/constants.cc b/extensions/common/constants.cc index 94684c560b011f..ac685dd57e94a7 100644 --- a/extensions/common/constants.cc +++ b/extensions/common/constants.cc @@ -87,6 +87,8 @@ const int kWebstoreSignaturesPublicKeySize = const char kMimeTypeJpeg[] = "image/jpeg"; const char kMimeTypePng[] = "image/png"; +const int64_t kInvalidServiceWorkerVersionId = -1; + } // namespace extensions namespace extension_misc { diff --git a/extensions/common/constants.h b/extensions/common/constants.h index d3f0b5320983e6..4e1ef6a974a873 100644 --- a/extensions/common/constants.h +++ b/extensions/common/constants.h @@ -104,6 +104,11 @@ extern const char kAuthUserQueryKey[]; extern const char kMimeTypeJpeg[]; extern const char kMimeTypePng[]; +// TODO(lazyboy): This is a hack and it is copied from service_worker_types.cc, +// which is not available to extensions/ code. Move the constant to +// content/public/common. +extern const int64_t kInvalidServiceWorkerVersionId; + // The extension id of the Web Store component application. extern const char kWebStoreAppId[]; diff --git a/extensions/common/extension_messages.h b/extensions/common/extension_messages.h index f9e9bcb31674e6..55f72aa1e88a96 100644 --- a/extensions/common/extension_messages.h +++ b/extensions/common/extension_messages.h @@ -116,9 +116,9 @@ IPC_STRUCT_BEGIN(ExtensionHostMsg_Request_Params) // id. Otherwise, this is -1. IPC_STRUCT_MEMBER(int, worker_thread_id) - // If this API call is for a service worker, then this is the embedded - // worker id. Otherwise, this is -1. - IPC_STRUCT_MEMBER(int, embedded_worker_id) + // If this API call is for a service worker, then this is the service + // worker version id. Otherwise, this is -1. + IPC_STRUCT_MEMBER(int64_t, service_worker_version_id) IPC_STRUCT_END() // Allows an extension to execute code in a tab. @@ -887,3 +887,20 @@ IPC_MESSAGE_CONTROL5(ExtensionMsg_ResponseWorker, bool /* success */, base::ListValue /* response wrapper (see comment above) */, std::string /* error */) + +// Asks the browser to increment the pending activity count for +// the worker with version id |service_worker_version_id|. +// Each request to increment must use unique |request_uuid|. If a request with +// |request_uuid| is already in progress (due to race condition or renderer +// compromise), browser process ignores the IPC. +IPC_MESSAGE_CONTROL2(ExtensionHostMsg_IncrementServiceWorkerActivity, + int64_t /* service_worker_version_id */, + std::string /* request_uuid */) + +// Asks the browser to decrement the pending activity count for +// the worker with version id |service_worker_version_id|. +// |request_uuid| must match the GUID of a previous request, otherwise the +// browser process ignores the IPC. +IPC_MESSAGE_CONTROL2(ExtensionHostMsg_DecrementServiceWorkerActivity, + int64_t /* service_worker_version_id */, + std::string /* request_uuid */) diff --git a/extensions/renderer/dispatcher.cc b/extensions/renderer/dispatcher.cc index 053d688a6d4f8c..4b7525bbb73254 100644 --- a/extensions/renderer/dispatcher.cc +++ b/extensions/renderer/dispatcher.cc @@ -87,6 +87,7 @@ #include "extensions/renderer/script_injection.h" #include "extensions/renderer/script_injection_manager.h" #include "extensions/renderer/send_request_natives.h" +#include "extensions/renderer/service_worker_request_sender.h" #include "extensions/renderer/set_icon_natives.h" #include "extensions/renderer/static_v8_external_one_byte_string_resource.h" #include "extensions/renderer/test_features_native_handler.h" @@ -397,7 +398,7 @@ void Dispatcher::DidCreateScriptContext( void Dispatcher::DidInitializeServiceWorkerContextOnWorkerThread( v8::Local v8_context, - int embedded_worker_id, + int64_t service_worker_version_id, const GURL& url) { const base::TimeTicks start_time = base::TimeTicks::Now(); @@ -443,7 +444,7 @@ void Dispatcher::DidInitializeServiceWorkerContextOnWorkerThread( context->set_url(url); if (ExtensionsClient::Get()->ExtensionAPIEnabledInExtensionServiceWorkers()) { - WorkerThreadDispatcher::Get()->AddWorkerData(embedded_worker_id); + WorkerThreadDispatcher::Get()->AddWorkerData(service_worker_version_id); { // TODO(lazyboy): Make sure accessing |source_map_| in worker thread is // safe. @@ -537,7 +538,7 @@ void Dispatcher::WillReleaseScriptContext( // static void Dispatcher::WillDestroyServiceWorkerContextOnWorkerThread( v8::Local v8_context, - int embedded_worker_id, + int64_t service_worker_version_id, const GURL& url) { if (url.SchemeIs(kExtensionScheme) || url.SchemeIs(kExtensionResourceScheme)) { @@ -545,7 +546,7 @@ void Dispatcher::WillDestroyServiceWorkerContextOnWorkerThread( g_worker_script_context_set.Get().Remove(v8_context, url); } if (ExtensionsClient::Get()->ExtensionAPIEnabledInExtensionServiceWorkers()) - WorkerThreadDispatcher::Get()->RemoveWorkerData(embedded_worker_id); + WorkerThreadDispatcher::Get()->RemoveWorkerData(service_worker_version_id); } void Dispatcher::DidCreateDocumentElement(blink::WebLocalFrame* frame) { diff --git a/extensions/renderer/dispatcher.h b/extensions/renderer/dispatcher.h index b592b075de6bc2..b960bef335a44d 100644 --- a/extensions/renderer/dispatcher.h +++ b/extensions/renderer/dispatcher.h @@ -102,7 +102,7 @@ class Dispatcher : public content::RenderThreadObserver, // variables. void DidInitializeServiceWorkerContextOnWorkerThread( v8::Local v8_context, - int embedded_worker_id, + int64_t service_worker_version_id, const GURL& url); void WillReleaseScriptContext(blink::WebLocalFrame* frame, @@ -112,7 +112,7 @@ class Dispatcher : public content::RenderThreadObserver, // Runs on a different thread and should not use any member variables. static void WillDestroyServiceWorkerContextOnWorkerThread( v8::Local v8_context, - int embedded_worker_id, + int64_t service_worker_version_id, const GURL& url); // This method is not allowed to run JavaScript code in the frame. diff --git a/extensions/renderer/request_sender.cc b/extensions/renderer/request_sender.cc index 99373894d9d9fc..b3fc126a71ab56 100644 --- a/extensions/renderer/request_sender.cc +++ b/extensions/renderer/request_sender.cc @@ -8,6 +8,7 @@ #include "base/timer/elapsed_timer.h" #include "base/values.h" #include "content/public/renderer/render_frame.h" +#include "extensions/common/constants.h" #include "extensions/common/extension_messages.h" #include "extensions/renderer/script_context.h" #include "third_party/WebKit/public/web/WebDocument.h" @@ -118,7 +119,7 @@ bool RequestSender::StartRequest(Source* source, // Set Service Worker specific params to default values. params.worker_thread_id = -1; - params.embedded_worker_id = -1; + params.service_worker_version_id = kInvalidServiceWorkerVersionId; SendRequest(render_frame, for_io_thread, params); return true; diff --git a/extensions/renderer/service_worker_data.cc b/extensions/renderer/service_worker_data.cc index 197bff952dea7c..b10b015b637c03 100644 --- a/extensions/renderer/service_worker_data.cc +++ b/extensions/renderer/service_worker_data.cc @@ -10,11 +10,12 @@ namespace extensions { ServiceWorkerData::ServiceWorkerData(WorkerThreadDispatcher* dispatcher, - int embedded_worker_id) - : embedded_worker_id_(embedded_worker_id), + int64_t service_worker_version_id) + : service_worker_version_id_(service_worker_version_id), v8_schema_registry_(new V8SchemaRegistry), request_sender_( - new ServiceWorkerRequestSender(dispatcher, embedded_worker_id)) {} + new ServiceWorkerRequestSender(dispatcher, + service_worker_version_id)) {} ServiceWorkerData::~ServiceWorkerData() {} diff --git a/extensions/renderer/service_worker_data.h b/extensions/renderer/service_worker_data.h index b273960e0c15a2..701a76133190b8 100644 --- a/extensions/renderer/service_worker_data.h +++ b/extensions/renderer/service_worker_data.h @@ -19,15 +19,18 @@ class WorkerThreadDispatcher; // TODO(lazyboy): Also put worker ScriptContexts in this. class ServiceWorkerData { public: - ServiceWorkerData(WorkerThreadDispatcher* dispatcher, int embedded_worker_id); + ServiceWorkerData(WorkerThreadDispatcher* dispatcher, + int64_t service_worker_version_id); ~ServiceWorkerData(); V8SchemaRegistry* v8_schema_registry() { return v8_schema_registry_.get(); } - RequestSender* request_sender() { return request_sender_.get(); } - int embedded_worker_id() const { return embedded_worker_id_; } + ServiceWorkerRequestSender* request_sender() { return request_sender_.get(); } + int64_t service_worker_version_id() const { + return service_worker_version_id_; + } private: - const int embedded_worker_id_; + const int64_t service_worker_version_id_; std::unique_ptr v8_schema_registry_; std::unique_ptr request_sender_; diff --git a/extensions/renderer/service_worker_request_sender.cc b/extensions/renderer/service_worker_request_sender.cc index a4d089ae7f039e..6830ed3213b025 100644 --- a/extensions/renderer/service_worker_request_sender.cc +++ b/extensions/renderer/service_worker_request_sender.cc @@ -4,6 +4,7 @@ #include "extensions/renderer/service_worker_request_sender.h" +#include "base/guid.h" #include "content/public/child/worker_thread.h" #include "extensions/common/extension_messages.h" #include "extensions/renderer/worker_thread_dispatcher.h" @@ -12,8 +13,9 @@ namespace extensions { ServiceWorkerRequestSender::ServiceWorkerRequestSender( WorkerThreadDispatcher* dispatcher, - int embedded_worker_id) - : dispatcher_(dispatcher), embedded_worker_id_(embedded_worker_id) {} + int64_t service_worker_version_id) + : dispatcher_(dispatcher), + service_worker_version_id_(service_worker_version_id) {} ServiceWorkerRequestSender::~ServiceWorkerRequestSender() {} @@ -25,9 +27,32 @@ void ServiceWorkerRequestSender::SendRequest( int worker_thread_id = content::WorkerThread::GetCurrentId(); DCHECK_GT(worker_thread_id, 0); params.worker_thread_id = worker_thread_id; - params.embedded_worker_id = embedded_worker_id_; + params.service_worker_version_id = service_worker_version_id_; + std::string guid = base::GenerateGUID(); + request_id_to_guid_[params.request_id] = guid; + + // Keeps the worker alive during extension function call. Balanced in + // HandleWorkerResponse(). + dispatcher_->Send(new ExtensionHostMsg_IncrementServiceWorkerActivity( + service_worker_version_id_, guid)); dispatcher_->Send(new ExtensionHostMsg_RequestWorker(params)); } +void ServiceWorkerRequestSender::HandleWorkerResponse( + int request_id, + int64_t service_worker_version_id, + bool success, + const base::ListValue& response, + const std::string& error) { + RequestSender::HandleResponse(request_id, success, response, error); + + std::map::iterator iter = + request_id_to_guid_.find(request_id); + DCHECK(iter != request_id_to_guid_.end()); + dispatcher_->Send(new ExtensionHostMsg_DecrementServiceWorkerActivity( + service_worker_version_id_, iter->second)); + request_id_to_guid_.erase(iter); +} + } // namespace extensions diff --git a/extensions/renderer/service_worker_request_sender.h b/extensions/renderer/service_worker_request_sender.h index ad22f47c5bf389..0493f617a5b253 100644 --- a/extensions/renderer/service_worker_request_sender.h +++ b/extensions/renderer/service_worker_request_sender.h @@ -14,16 +14,24 @@ class WorkerThreadDispatcher; class ServiceWorkerRequestSender : public RequestSender { public: ServiceWorkerRequestSender(WorkerThreadDispatcher* dispatcher, - int embedded_worker_id); + int64_t service_worker_version_id); ~ServiceWorkerRequestSender() override; void SendRequest(content::RenderFrame* render_frame, bool for_io_thread, ExtensionHostMsg_Request_Params& params) override; + void HandleWorkerResponse(int request_id, + int64_t service_worker_version_id, + bool success, + const base::ListValue& response, + const std::string& error); private: WorkerThreadDispatcher* const dispatcher_; - const int embedded_worker_id_; + const int64_t service_worker_version_id_; + + // request id -> GUID map for each outstanding requests. + std::map request_id_to_guid_; DISALLOW_COPY_AND_ASSIGN(ServiceWorkerRequestSender); }; diff --git a/extensions/renderer/worker_thread_dispatcher.cc b/extensions/renderer/worker_thread_dispatcher.cc index cc9fdc88def46f..0d189d44fbde27 100644 --- a/extensions/renderer/worker_thread_dispatcher.cc +++ b/extensions/renderer/worker_thread_dispatcher.cc @@ -24,8 +24,10 @@ void OnResponseOnWorkerThread(int request_id, bool succeeded, const std::unique_ptr& response, const std::string& error) { - WorkerThreadDispatcher::GetRequestSender()->HandleResponse( - request_id, succeeded, *response, error); + ServiceWorkerData* data = g_data_tls.Pointer()->Get(); + WorkerThreadDispatcher::GetRequestSender()->HandleWorkerResponse( + request_id, data->service_worker_version_id(), succeeded, *response, + error); } } // namespace @@ -52,7 +54,7 @@ V8SchemaRegistry* WorkerThreadDispatcher::GetV8SchemaRegistry() { } // static -RequestSender* WorkerThreadDispatcher::GetRequestSender() { +ServiceWorkerRequestSender* WorkerThreadDispatcher::GetRequestSender() { ServiceWorkerData* data = g_data_tls.Pointer()->Get(); DCHECK(data); return data->request_sender(); @@ -84,19 +86,20 @@ void WorkerThreadDispatcher::OnResponseWorker(int worker_thread_id, base::Passed(response.CreateDeepCopy()), error)); } -void WorkerThreadDispatcher::AddWorkerData(int embedded_worker_id) { +void WorkerThreadDispatcher::AddWorkerData(int64_t service_worker_version_id) { ServiceWorkerData* data = g_data_tls.Pointer()->Get(); if (!data) { ServiceWorkerData* new_data = - new ServiceWorkerData(this, embedded_worker_id); + new ServiceWorkerData(this, service_worker_version_id); g_data_tls.Pointer()->Set(new_data); } } -void WorkerThreadDispatcher::RemoveWorkerData(int embedded_worker_id) { +void WorkerThreadDispatcher::RemoveWorkerData( + int64_t service_worker_version_id) { ServiceWorkerData* data = g_data_tls.Pointer()->Get(); if (data) { - DCHECK_EQ(embedded_worker_id, data->embedded_worker_id()); + DCHECK_EQ(service_worker_version_id, data->service_worker_version_id()); delete data; g_data_tls.Pointer()->Set(nullptr); } diff --git a/extensions/renderer/worker_thread_dispatcher.h b/extensions/renderer/worker_thread_dispatcher.h index 4a93011ddd3f95..da8946a586af5a 100644 --- a/extensions/renderer/worker_thread_dispatcher.h +++ b/extensions/renderer/worker_thread_dispatcher.h @@ -19,6 +19,7 @@ class RenderThread; namespace extensions { class RequestSender; +class ServiceWorkerRequestSender; class V8SchemaRegistry; // Sends and receives IPC in an extension Service Worker. @@ -34,12 +35,12 @@ class WorkerThreadDispatcher : public content::RenderThreadObserver { // Thread safe. static WorkerThreadDispatcher* Get(); - static RequestSender* GetRequestSender(); + static ServiceWorkerRequestSender* GetRequestSender(); void Init(content::RenderThread* render_thread); bool Send(IPC::Message* message); - void AddWorkerData(int embedded_worker_id); - void RemoveWorkerData(int embedded_worker_id); + void AddWorkerData(int64_t service_worker_version_id); + void RemoveWorkerData(int64_t service_worker_version_id); V8SchemaRegistry* GetV8SchemaRegistry(); private: diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 50858884c61959..4e5b7cfb81118f 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -55936,6 +55936,15 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. + + lazyboy@chromium.org + + The duration an external request spent to keep a service worker alive. + Currently, extension service workers use external requests to keep the + worker alive during the time the worker requests an extension API. + + + jeremyarcher@chromium.org