Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1801)

Unified Diff: content/browser/service_worker/service_worker_version.cc

Issue 1171173002: [Background Sync] Use Mojo IPC to fire background sync events (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Retire old IPC; fix memory leak in ServiceWorkerRegistraion test Created 5 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/service_worker/service_worker_version.cc
diff --git a/content/browser/service_worker/service_worker_version.cc b/content/browser/service_worker/service_worker_version.cc
index db9350a62ffd903432d75d9f46c334d3cfd6749f..46880833ae681465175c600444ba90275591d5e9 100644
--- a/content/browser/service_worker/service_worker_version.cc
+++ b/content/browser/service_worker/service_worker_version.cc
@@ -22,6 +22,7 @@
#include "content/browser/service_worker/embedded_worker_registry.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_event_dispatcher.h"
#include "content/browser/service_worker/service_worker_metrics.h"
#include "content/browser/service_worker/service_worker_registration.h"
#include "content/browser/service_worker/service_worker_utils.h"
@@ -39,6 +40,7 @@
#include "content/public/common/content_client.h"
#include "content/public/common/content_switches.h"
#include "content/public/common/result_codes.h"
+#include "content/public/common/service_registry.h"
#include "net/http/http_response_headers.h"
#include "net/http/http_response_info.h"
@@ -481,6 +483,7 @@ ServiceWorkerVersion::ServiceWorkerVersion(
script_cache_map_(this, context),
ping_controller_(new PingController(this)),
metrics_(new Metrics),
+ event_dispatcher_(new ServiceWorkerEventDispatcher()),
weak_factory_(this) {
DCHECK(context_);
DCHECK(registration);
@@ -742,12 +745,32 @@ void ServiceWorkerVersion::DispatchSyncEvent(const StatusCallback& callback) {
return;
}
- int request_id = AddRequest(callback, &sync_callbacks_, REQUEST_SYNC);
- ServiceWorkerStatusCode status = embedded_worker_->SendMessage(
- ServiceWorkerMsg_SyncEvent(request_id));
- if (status != SERVICE_WORKER_OK) {
- sync_callbacks_.Remove(request_id);
- RunSoon(base::Bind(callback, status));
+ // We can only get the embedded worker's process and thread ids on the IO
+ // thread, but we need to be on the UI thread to get the the mojo service
+ // registry from the RenderProcessHost.
+ BrowserThread::PostTask(
+ BrowserThread::UI, FROM_HERE,
+ base::Bind(
+ &ServiceWorkerEventDispatcher::DispatchSyncEvent, event_dispatcher_,
+ embedded_worker_->process_id(), embedded_worker_->thread_id(),
+ base::Bind(&ServiceWorkerVersion::OnSyncEventFinished,
+ base::Unretained(this), callback),
+ BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO)));
+}
+
+void ServiceWorkerVersion::OnSyncEventFinished(const StatusCallback& callback,
jkarlin 2015/06/12 21:08:14 What happens if the ServiceWorker dies mid sync? D
+ ServiceWorkerStatusCode status) {
+ DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
jkarlin 2015/06/12 21:08:14 You're missing the TRACE_EVENT macro that the orig
iclelland 2015/06/16 15:59:16 You're right -- that got lost in a reorg. Fixed, t
+ scoped_refptr<ServiceWorkerVersion> protect(this);
+ callback.Run(status);
+
+ RestartTick(&idle_time_);
+ // TODO(iclelland) factor this out of RemoveCallbackAndStopIfRedundant
jkarlin 2015/06/12 21:08:14 You might as well make a StopIfRedundant function
iclelland 2015/06/16 15:59:16 Yep; that was the intended refactor. Done.
+ if (is_redundant()) {
+ // The stop should be already scheduled, but try to stop immediately, in
+ // order to release worker resources soon.
+ StopWorkerIfIdle();
}
}
@@ -1101,8 +1124,6 @@ void ServiceWorkerVersion::OnStopped(
SERVICE_WORKER_ERROR_FAILED,
SERVICE_WORKER_FETCH_EVENT_RESULT_FALLBACK,
ServiceWorkerResponse());
- RunIDMapCallbacks(&sync_callbacks_,
- SERVICE_WORKER_ERROR_FAILED);
RunIDMapCallbacks(&notification_click_callbacks_,
SERVICE_WORKER_ERROR_FAILED);
RunIDMapCallbacks(&push_callbacks_,
@@ -1159,8 +1180,6 @@ bool ServiceWorkerVersion::OnMessageReceived(const IPC::Message& message) {
OnInstallEventFinished)
IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_FetchEventFinished,
OnFetchEventFinished)
- IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_SyncEventFinished,
- OnSyncEventFinished)
IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_NotificationClickEventFinished,
OnNotificationClickEventFinished)
IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_PushEventFinished,
@@ -1332,33 +1351,6 @@ void ServiceWorkerVersion::OnFetchEventFinished(
RemoveCallbackAndStopIfRedundant(&fetch_callbacks_, request_id);
}
-void ServiceWorkerVersion::OnSyncEventFinished(
- int request_id,
- blink::WebServiceWorkerEventResult result) {
- TRACE_EVENT1("ServiceWorker",
- "ServiceWorkerVersion::OnSyncEventFinished",
- "Request id", request_id);
- StatusCallback* callback = sync_callbacks_.Lookup(request_id);
- if (!callback) {
- NOTREACHED() << "Got unexpected message: " << request_id;
- return;
- }
-
- ServiceWorkerStatusCode status = SERVICE_WORKER_OK;
- if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kEnableServiceWorkerSync)) {
- // Avoid potential race condition where flag is disabled after a sync event
- // was dispatched
- status = SERVICE_WORKER_ERROR_ABORT;
- } else if (result == blink::WebServiceWorkerEventResultRejected) {
- status = SERVICE_WORKER_ERROR_EVENT_WAITUNTIL_REJECTED;
- }
-
- scoped_refptr<ServiceWorkerVersion> protect(this);
- callback->Run(status);
- RemoveCallbackAndStopIfRedundant(&sync_callbacks_, request_id);
-}
-
void ServiceWorkerVersion::OnNotificationClickEventFinished(
int request_id) {
TRACE_EVENT1("ServiceWorker",
@@ -1903,7 +1895,6 @@ bool ServiceWorkerVersion::HasInflightRequests() const {
!activate_callbacks_.IsEmpty() ||
!install_callbacks_.IsEmpty() ||
!fetch_callbacks_.IsEmpty() ||
- !sync_callbacks_.IsEmpty() ||
!notification_click_callbacks_.IsEmpty() ||
!push_callbacks_.IsEmpty() ||
!geofencing_callbacks_.IsEmpty() ||
@@ -1984,9 +1975,6 @@ bool ServiceWorkerVersion::OnRequestTimeout(const RequestInfo& info) {
&fetch_callbacks_, info.id, SERVICE_WORKER_ERROR_TIMEOUT,
/* The other args are ignored for non-OK status. */
SERVICE_WORKER_FETCH_EVENT_RESULT_FALLBACK, ServiceWorkerResponse());
- case REQUEST_SYNC:
- return RunIDMapCallback(&sync_callbacks_, info.id,
- SERVICE_WORKER_ERROR_TIMEOUT);
case REQUEST_NOTIFICATION_CLICK:
return RunIDMapCallback(&notification_click_callbacks_, info.id,
SERVICE_WORKER_ERROR_TIMEOUT);

Powered by Google App Engine
This is Rietveld 408576698