Skip to content

Commit

Permalink
Add a status code to install event handled message from Service Worker
Browse files Browse the repository at this point in the history
The Service Worker can reject an install event by passing a promise to
waitUntil that is rejected. This patch allows handling for this situation.

This patch depends on the Blink change:
https://codereview.chromium.org/210833004/

BUG=285976
R=jam@chromium.org, kinuko@chromium.org, tkent@chromium.org, tsepez@chromium.org

Review URL: https://codereview.chromium.org/205563006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@260356 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
falken@chromium.org committed Mar 29, 2014
1 parent 6771991 commit 90a9878
Show file tree
Hide file tree
Showing 11 changed files with 120 additions and 57 deletions.
15 changes: 7 additions & 8 deletions content/browser/service_worker/embedded_worker_test_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,14 @@ bool EmbeddedWorkerTestHelper::OnSendMessageToWorker(
return handled;
}

void EmbeddedWorkerTestHelper::OnInstallEvent(
int embedded_worker_id,
int request_id,
int active_version_embedded_worker_id) {
void EmbeddedWorkerTestHelper::OnInstallEvent(int embedded_worker_id,
int request_id,
int active_version_id) {
SimulateSendMessageToBrowser(
embedded_worker_id,
request_id,
ServiceWorkerHostMsg_InstallEventFinished());
ServiceWorkerHostMsg_InstallEventFinished(
blink::WebServiceWorkerEventResultCompleted));
}

void EmbeddedWorkerTestHelper::OnFetchEvent(
Expand Down Expand Up @@ -179,15 +179,14 @@ void EmbeddedWorkerTestHelper::OnSendMessageToWorkerStub(
message));
}

void EmbeddedWorkerTestHelper::OnInstallEventStub(
int active_version_embedded_worker_id) {
void EmbeddedWorkerTestHelper::OnInstallEventStub(int active_version_id) {
base::MessageLoopProxy::current()->PostTask(
FROM_HERE,
base::Bind(&EmbeddedWorkerTestHelper::OnInstallEvent,
weak_factory_.GetWeakPtr(),
current_embedded_worker_id_,
current_request_id_,
active_version_embedded_worker_id));
active_version_id));
}

void EmbeddedWorkerTestHelper::OnFetchEventStub(
Expand Down
4 changes: 2 additions & 2 deletions content/browser/service_worker/embedded_worker_test_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class EmbeddedWorkerTestHelper : public IPC::Sender,
// SimulateSendMessageToBrowser.
virtual void OnInstallEvent(int embedded_worker_id,
int request_id,
int active_version_embedded_worker_id);
int active_version_id);
virtual void OnFetchEvent(int embedded_worker_id,
int request_id,
const ServiceWorkerFetchRequest& request);
Expand All @@ -107,7 +107,7 @@ class EmbeddedWorkerTestHelper : public IPC::Sender,
int embedded_worker_id,
int request_id,
const IPC::Message& message);
void OnInstallEventStub(int active_version_embedded_worker_id);
void OnInstallEventStub(int active_version_id);
void OnFetchEventStub(const ServiceWorkerFetchRequest& request);

EmbeddedWorkerRegistry* registry();
Expand Down
15 changes: 8 additions & 7 deletions content/browser/service_worker/service_worker_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,8 @@ class ServiceWorkerVersionBrowserTest : public ServiceWorkerBrowserTest {
}
}

void InstallTestHelper(const std::string& worker_url) {
void InstallTestHelper(const std::string& worker_url,
ServiceWorkerStatusCode expected_status) {
RunOnIOThread(base::Bind(&self::SetUpRegistrationOnIOThread, this,
worker_url));

Expand All @@ -245,7 +246,7 @@ class ServiceWorkerVersionBrowserTest : public ServiceWorkerBrowserTest {
install_run_loop.QuitClosure(),
&status));
install_run_loop.Run();
ASSERT_EQ(SERVICE_WORKER_OK, status);
ASSERT_EQ(expected_status, status);

// Stop the worker.
status = SERVICE_WORKER_ERROR_FAILED;
Expand Down Expand Up @@ -389,19 +390,19 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest, StartNotFound) {
}

IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest, Install) {
InstallTestHelper("/service_worker/worker.js");
InstallTestHelper("/service_worker/worker.js", SERVICE_WORKER_OK);
}

IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,
InstallWithWaitUntil_Fulfilled) {
InstallTestHelper("/service_worker/worker_install_fulfilled.js");
InstallTestHelper("/service_worker/worker_install_fulfilled.js",
SERVICE_WORKER_OK);
}

IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,
InstallWithWaitUntil_Rejected) {
// TODO(kinuko): This should also report back an error, but we
// don't have plumbing for it yet.
InstallTestHelper("/service_worker/worker_install_rejected.js");
InstallTestHelper("/service_worker/worker_install_rejected.js",
SERVICE_WORKER_ERROR_INSTALL_WORKER_FAILED);
}

IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest, FetchEvent_Response) {
Expand Down
98 changes: 75 additions & 23 deletions content/browser/service_worker/service_worker_version.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,39 @@ typedef ServiceWorkerVersion::MessageCallback MessageCallback;

namespace {

typedef base::Callback<bool(const IPC::Message* message,
Tuple1<blink::WebServiceWorkerEventResult>* result)>
InstallPhaseEventFinishedMessageReader;

// Parameters for the HandleInstallPhaseEventFinished function, which cannot
// accept them directly without exceeding the max arity supported by Bind().
struct HandleInstallPhaseEventFinishedParameters {
HandleInstallPhaseEventFinishedParameters(
base::WeakPtr<ServiceWorkerVersion> version,
uint32 expected_message_type,
const InstallPhaseEventFinishedMessageReader& message_reader,
const StatusCallback& callback,
ServiceWorkerVersion::Status next_status_on_success,
ServiceWorkerVersion::Status next_status_on_error,
ServiceWorkerStatusCode next_status_code_on_event_rejection)
: version(version),
expected_message_type(expected_message_type),
message_reader(message_reader),
callback(callback),
next_status_on_success(next_status_on_success),
next_status_on_error(next_status_on_error),
next_status_code_on_event_rejection(
next_status_code_on_event_rejection) {}

base::WeakPtr<ServiceWorkerVersion> version;
uint32 expected_message_type;
InstallPhaseEventFinishedMessageReader message_reader;
StatusCallback callback;
ServiceWorkerVersion::Status next_status_on_success;
ServiceWorkerVersion::Status next_status_on_error;
ServiceWorkerStatusCode next_status_code_on_event_rejection;
};

void RunSoon(const base::Closure& callback) {
if (!callback.is_null())
base::MessageLoop::current()->PostTask(FROM_HERE, callback);
Expand Down Expand Up @@ -60,29 +93,34 @@ void RunEmptyMessageCallback(const MessageCallback& callback,
callback.Run(status, IPC::Message());
}

void HandleEventFinished(base::WeakPtr<ServiceWorkerVersion> version,
uint32 expected_message_type,
const StatusCallback& callback,
ServiceWorkerVersion::Status next_status_on_success,
ServiceWorkerVersion::Status next_status_on_error,
ServiceWorkerStatusCode status,
const IPC::Message& message) {
if (!version)
void HandleInstallPhaseEventFinished(
const HandleInstallPhaseEventFinishedParameters& params,
ServiceWorkerStatusCode status,
const IPC::Message& message) {
if (!params.version)
return;
if (status != SERVICE_WORKER_OK) {
version->SetStatus(next_status_on_error);
callback.Run(status);
params.version->SetStatus(params.next_status_on_error);
params.callback.Run(status);
return;
}
if (message.type() != expected_message_type) {
if (message.type() != params.expected_message_type) {
NOTREACHED() << "Got unexpected response: " << message.type()
<< " expected:" << expected_message_type;
version->SetStatus(next_status_on_error);
callback.Run(SERVICE_WORKER_ERROR_FAILED);
<< " expected:" << params.expected_message_type;
params.version->SetStatus(params.next_status_on_error);
params.callback.Run(SERVICE_WORKER_ERROR_FAILED);
return;
}
version->SetStatus(next_status_on_success);
callback.Run(SERVICE_WORKER_OK);
params.version->SetStatus(params.next_status_on_success);

Tuple1<blink::WebServiceWorkerEventResult> result(
blink::WebServiceWorkerEventResultCompleted);
if (!params.message_reader.is_null()) {
params.message_reader.Run(&message, &result);
if (result.a == blink::WebServiceWorkerEventResultRejected)
status = params.next_status_code_on_event_rejection;
}
params.callback.Run(status);
}

void HandleFetchResponse(const ServiceWorkerVersion::FetchCallback& callback,
Expand Down Expand Up @@ -247,15 +285,21 @@ void ServiceWorkerVersion::SendMessageAndRegisterCallback(
}

void ServiceWorkerVersion::DispatchInstallEvent(
int active_version_embedded_worker_id,
int active_version_id,
const StatusCallback& callback) {
DCHECK_EQ(NEW, status()) << status();
SetStatus(INSTALLING);
HandleInstallPhaseEventFinishedParameters params(
weak_factory_.GetWeakPtr(),
ServiceWorkerHostMsg_InstallEventFinished::ID,
base::Bind(&ServiceWorkerHostMsg_InstallEventFinished::Read),
callback,
INSTALLED,
NEW,
SERVICE_WORKER_ERROR_INSTALL_WORKER_FAILED);
SendMessageAndRegisterCallback(
ServiceWorkerMsg_InstallEvent(active_version_embedded_worker_id),
base::Bind(&HandleEventFinished, weak_factory_.GetWeakPtr(),
ServiceWorkerHostMsg_InstallEventFinished::ID,
callback, INSTALLED, NEW));
ServiceWorkerMsg_InstallEvent(active_version_id),
base::Bind(&HandleInstallPhaseEventFinished, params));
}

void ServiceWorkerVersion::DispatchActivateEvent(
Expand All @@ -264,8 +308,16 @@ void ServiceWorkerVersion::DispatchActivateEvent(
SetStatus(ACTIVATING);
// TODO(kinuko): Implement.
NOTIMPLEMENTED();
RunSoon(base::Bind(&HandleEventFinished, weak_factory_.GetWeakPtr(),
-1 /* dummy message_id */, callback, ACTIVE, INSTALLED,
HandleInstallPhaseEventFinishedParameters params(
weak_factory_.GetWeakPtr(),
-1 /* dummy message_id */,
InstallPhaseEventFinishedMessageReader(),
callback,
ACTIVE,
INSTALLED,
SERVICE_WORKER_ERROR_ACTIVATE_WORKER_FAILED);
RunSoon(base::Bind(&HandleInstallPhaseEventFinished,
params,
SERVICE_WORKER_OK,
IPC::Message(-1, -1, IPC::Message::PRIORITY_NORMAL)));
}
Expand Down
4 changes: 2 additions & 2 deletions content/browser/service_worker/service_worker_version.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,14 @@ class CONTENT_EXPORT ServiceWorkerVersion
// Sends install event to the associated embedded worker and asynchronously
// calls |callback| when it errors out or it gets response from the worker
// to notify install completion.
// |active_version_embedded_worker_id| must be a valid positive ID
// |active_version_id| must be a valid positive ID
// if there's an active (previous) version running.
//
// This must be called when the status() is NEW. Calling this changes
// the version's status to INSTALLING.
// Upon completion, the version's status will be changed to INSTALLED
// on success, or back to NEW on failure.
void DispatchInstallEvent(int active_version_embedded_worker_id,
void DispatchInstallEvent(int active_version_id,
const StatusCallback& callback);

// Sends activate event to the associated embedded worker and asynchronously
Expand Down
1 change: 1 addition & 0 deletions content/common/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ include_rules = [
"+third_party/WebKit/public/platform/WebScreenOrientation.h",
"+third_party/WebKit/public/platform/WebScreenInfo.h",
"+third_party/WebKit/public/platform/WebServiceWorkerError.h",
"+third_party/WebKit/public/platform/WebServiceWorkerEventResult.h",
"+third_party/WebKit/public/platform/WebStorageArea.h",
"+third_party/WebKit/public/platform/WebString.h",
"+third_party/WebKit/public/platform/linux/WebFontFamily.h",
Expand Down
15 changes: 9 additions & 6 deletions content/common/service_worker/service_worker_messages.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "ipc/ipc_message_macros.h"
#include "ipc/ipc_param_traits.h"
#include "third_party/WebKit/public/platform/WebServiceWorkerError.h"
#include "third_party/WebKit/public/platform/WebServiceWorkerEventResult.h"
#include "url/gurl.h"

#undef IPC_MESSAGE_EXPORT
Expand All @@ -20,6 +21,9 @@
IPC_ENUM_TRAITS_MAX_VALUE(blink::WebServiceWorkerError::ErrorType,
blink::WebServiceWorkerError::ErrorTypeLast)

IPC_ENUM_TRAITS_MAX_VALUE(blink::WebServiceWorkerEventResult,
blink::WebServiceWorkerEventResultLast)

IPC_STRUCT_TRAITS_BEGIN(content::ServiceWorkerFetchRequest)
IPC_STRUCT_TRAITS_MEMBER(url)
IPC_STRUCT_TRAITS_MEMBER(method)
Expand Down Expand Up @@ -77,8 +81,7 @@ IPC_MESSAGE_CONTROL4(ServiceWorkerMsg_ServiceWorkerRegistrationError,
base::string16 /* message */)

// Sent via EmbeddedWorker to dispatch install event.
IPC_MESSAGE_CONTROL1(ServiceWorkerMsg_InstallEvent,
int /* active_version_embedded_worker_id */)
IPC_MESSAGE_CONTROL1(ServiceWorkerMsg_InstallEvent, int /* active_version_id */)

// Sent via EmbeddedWorker to dispatch fetch event.
IPC_MESSAGE_CONTROL1(ServiceWorkerMsg_FetchEvent,
Expand Down Expand Up @@ -110,12 +113,12 @@ IPC_MESSAGE_CONTROL2(ServiceWorkerHostMsg_RemoveScriptClient,
int /* provider_id */)

// Informs the browser that install event handling has finished.
// Sent via EmbeddedWorker. If there was an exception during the
// event handling it'll be reported back separately (to be propagated
// to the documents).
IPC_MESSAGE_CONTROL0(ServiceWorkerHostMsg_InstallEventFinished)
// Sent via EmbeddedWorker.
IPC_MESSAGE_CONTROL1(ServiceWorkerHostMsg_InstallEventFinished,
blink::WebServiceWorkerEventResult)

// Informs the browser that fetch event handling has finished.
// Sent via EmbeddedWorker.
IPC_MESSAGE_CONTROL2(ServiceWorkerHostMsg_FetchEventFinished,
content::ServiceWorkerFetchEventResult,
content::ServiceWorkerResponse)
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,11 @@ void EmbeddedWorkerContextClient::workerContextDestroyed() {
embedded_worker_id_));
}

void EmbeddedWorkerContextClient::didHandleInstallEvent(int request_id) {
void EmbeddedWorkerContextClient::didHandleInstallEvent(
int request_id,
blink::WebServiceWorkerEventResult result) {
DCHECK(script_context_);
script_context_->DidHandleInstallEvent(request_id);
script_context_->DidHandleInstallEvent(request_id, result);
}

void EmbeddedWorkerContextClient::didHandleFetchEvent(int request_id) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "base/strings/string16.h"
#include "content/common/service_worker/service_worker_types.h"
#include "ipc/ipc_listener.h"
#include "third_party/WebKit/public/platform/WebServiceWorkerEventResult.h"
#include "third_party/WebKit/public/web/WebServiceWorkerContextClient.h"
#include "url/gurl.h"

Expand Down Expand Up @@ -55,7 +56,8 @@ class EmbeddedWorkerContextClient
virtual void workerContextFailedToStart();
virtual void workerContextStarted(blink::WebServiceWorkerContextProxy* proxy);
virtual void workerContextDestroyed();
virtual void didHandleInstallEvent(int request_id);
virtual void didHandleInstallEvent(int request_id,
blink::WebServiceWorkerEventResult result);
virtual void didHandleFetchEvent(int request_id);
virtual void didHandleFetchEvent(
int request_id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,10 @@ void ServiceWorkerScriptContext::OnMessageReceived(
current_request_id_ = kInvalidServiceWorkerRequestId;
}

void ServiceWorkerScriptContext::DidHandleInstallEvent(int request_id) {
Send(request_id, ServiceWorkerHostMsg_InstallEventFinished());
void ServiceWorkerScriptContext::DidHandleInstallEvent(
int request_id,
blink::WebServiceWorkerEventResult result) {
Send(request_id, ServiceWorkerHostMsg_InstallEventFinished(result));
}

void ServiceWorkerScriptContext::DidHandleFetchEvent(
Expand All @@ -55,8 +57,7 @@ void ServiceWorkerScriptContext::Send(int request_id,
embedded_context_->SendMessageToBrowser(request_id, message);
}

void ServiceWorkerScriptContext::OnInstallEvent(
int active_version_embedded_worker_id) {
void ServiceWorkerScriptContext::OnInstallEvent(int active_version_id) {
proxy_->dispatchInstallEvent(current_request_id_);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/strings/string16.h"

#include "content/common/service_worker/service_worker_types.h"
#include "third_party/WebKit/public/platform/WebServiceWorkerEventResult.h"

namespace blink {
class WebServiceWorkerContextProxy;
Expand All @@ -37,7 +38,8 @@ class ServiceWorkerScriptContext {

void OnMessageReceived(int request_id, const IPC::Message& message);

void DidHandleInstallEvent(int request_id);
void DidHandleInstallEvent(int request_id,
blink::WebServiceWorkerEventResult result);
void DidHandleFetchEvent(int request_id,
ServiceWorkerFetchEventResult result,
const ServiceWorkerResponse& response);
Expand All @@ -46,7 +48,7 @@ class ServiceWorkerScriptContext {
// Send message back to the browser.
void Send(int request_id, const IPC::Message& message);

void OnInstallEvent(int active_version_embedded_worker_id);
void OnInstallEvent(int active_version_id);
void OnFetchEvent(const ServiceWorkerFetchRequest& request);
void OnPostMessage(const base::string16& message,
const std::vector<int>& sent_message_port_ids,
Expand Down

0 comments on commit 90a9878

Please sign in to comment.