Skip to content

Commit

Permalink
Revert of Revert of [ServivceWorker] Treat rejecting respondWith as a…
Browse files Browse the repository at this point in the history
… Network Error (patchset #1 id:1 of https://codereview.chromium.org/580993002/)

Reason for revert:
Landed the patch to initialize WebServiceWorkerResponse's status.

https://codereview.chromium.org/575153002/

Original issue's description:
> Revert of [ServivceWorker] Treat rejecting respondWith as a Network Error (patchset #4 id:60001 of https://codereview.chromium.org/571843003/)
> 
> Reason for revert:
> WebServiceWorkerResponse's status is not initialised.
> This may cause problems.
> 
> I will re-land this patch after https://codereview.chromium.org/575153002/ is submitted.
> 
> Original issue's description:
> > [ServivceWorker] Treat rejecting respondWith as a Network Error
> > 
> > Chromium side change is this: https://codereview.chromium.org/574643003/
> > 
> > 
> > BUG=411173
> > TEST=layouttest http/tests/serviceworker/
> > 
> > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182147
> 
> [email protected],[email protected],[email protected],[email protected]
> NOTREECHECKS=true
> NOTRY=true
> BUG=411173
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182204

[email protected],[email protected],[email protected],[email protected]
NOTREECHECKS=true
NOTRY=true
BUG=411173

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

git-svn-id: svn://svn.chromium.org/blink/trunk@182290 bbb929c8-8fbe-4397-9dbb-9b2b20218538
  • Loading branch information
horo-t committed Sep 19, 2014
1 parent ada6784 commit e0dc629
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 39 deletions.
4 changes: 2 additions & 2 deletions LayoutTests/http/tests/serviceworker/fetch-event.html
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@
.then(function() { return with_iframe(scope); })
.then(function(frame) {
assert_equals(frame.contentDocument.body.textContent,
'Here\'s a simple html file.\n',
'Response should come from fallback to native fetch');
'',
'Response should be a NetworkError');
unload_iframe(frame);
return service_worker_unregister_and_done(t, scope);
})
Expand Down
33 changes: 17 additions & 16 deletions Source/modules/serviceworkers/RespondWithObserver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "bindings/modules/v8/V8Response.h"
#include "core/dom/ExecutionContext.h"
#include "modules/serviceworkers/ServiceWorkerGlobalScopeClient.h"
#include "public/platform/WebServiceWorkerResponse.h"
#include "wtf/Assertions.h"
#include "wtf/RefPtr.h"
#include <v8.h>
Expand Down Expand Up @@ -74,8 +75,11 @@ void RespondWithObserver::contextDestroyed()

void RespondWithObserver::didDispatchEvent()
{
if (m_state == Initial)
sendResponse(nullptr);
ASSERT(executionContext());
if (m_state != Initial)
return;
ServiceWorkerGlobalScopeClient::from(executionContext())->didHandleFetchEvent(m_eventID);
m_state = Done;
}

void RespondWithObserver::respondWith(ScriptState* scriptState, const ScriptValue& value, ExceptionState& exceptionState)
Expand All @@ -91,30 +95,27 @@ void RespondWithObserver::respondWith(ScriptState* scriptState, const ScriptValu
ThenFunction::createFunction(scriptState, this, ThenFunction::Rejected));
}

void RespondWithObserver::sendResponse(Response* response)
{
if (!executionContext())
return;
ServiceWorkerGlobalScopeClient::from(executionContext())->didHandleFetchEvent(m_eventID, response);
m_state = Done;
}

void RespondWithObserver::responseWasRejected()
{
// FIXME: Throw a NetworkError to service worker's execution context.
sendResponse(nullptr);
ASSERT(executionContext());
// The default value of WebServiceWorkerResponse's status is 0, which maps
// to a network error.
WebServiceWorkerResponse webResponse;
ServiceWorkerGlobalScopeClient::from(executionContext())->didHandleFetchEvent(m_eventID, webResponse);
m_state = Done;
}

void RespondWithObserver::responseWasFulfilled(const ScriptValue& value)
{
if (!executionContext())
return;
ASSERT(executionContext());
if (!V8Response::hasInstance(value.v8Value(), toIsolate(executionContext()))) {
responseWasRejected();
return;
}
v8::Handle<v8::Object> object = v8::Handle<v8::Object>::Cast(value.v8Value());
sendResponse(V8Response::toImpl(object));
WebServiceWorkerResponse webResponse;
V8Response::toImplWithTypeCheck(toIsolate(executionContext()), value.v8Value())->populateWebServiceWorkerResponse(webResponse);
ServiceWorkerGlobalScopeClient::from(executionContext())->didHandleFetchEvent(m_eventID, webResponse);
m_state = Done;
}

RespondWithObserver::RespondWithObserver(ExecutionContext* context, int eventID)
Expand Down
4 changes: 0 additions & 4 deletions Source/modules/serviceworkers/RespondWithObserver.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,6 @@ class RespondWithObserver FINAL : public GarbageCollectedFinalized<RespondWithOb

RespondWithObserver(ExecutionContext*, int eventID);

// Sends a response back to the client. The null response means to fallback
// to native.
void sendResponse(Response*);

int m_eventID;

enum State { Initial, Pending, Done };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@
namespace blink {

class ExecutionContext;
class Response;
class WebServiceWorkerCacheStorage;
class WebServiceWorkerResponse;
class WebURL;
class WorkerClients;

Expand All @@ -59,8 +59,10 @@ class ServiceWorkerGlobalScopeClient : public WillBeHeapSupplement<WorkerClients

virtual void didHandleActivateEvent(int eventID, WebServiceWorkerEventResult) = 0;
virtual void didHandleInstallEvent(int installEventID, WebServiceWorkerEventResult) = 0;
// A null response means no valid response was provided by the service worker, so fallback to native.
virtual void didHandleFetchEvent(int fetchEventID, Response* = 0) = 0;
// Calling didHandleFetchEvent without response means no response was
// provided by the service worker in the fetch events, so fallback to native.
virtual void didHandleFetchEvent(int fetchEventID) = 0;
virtual void didHandleFetchEvent(int fetchEventID, const WebServiceWorkerResponse&) = 0;
virtual void didHandleSyncEvent(int syncEventID) = 0;
virtual void postMessageToClient(int clientID, const WebString& message, PassOwnPtr<WebMessagePortChannelArray>) = 0;

Expand Down
19 changes: 6 additions & 13 deletions Source/web/ServiceWorkerGlobalScopeClientImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,20 +74,13 @@ void ServiceWorkerGlobalScopeClientImpl::didHandleInstallEvent(int installEventI
m_client.didHandleInstallEvent(installEventID, result);
}

void ServiceWorkerGlobalScopeClientImpl::didHandleFetchEvent(int fetchEventID, Response* response)
void ServiceWorkerGlobalScopeClientImpl::didHandleFetchEvent(int fetchEventID)
{
m_client.didHandleFetchEvent(fetchEventID);
}

void ServiceWorkerGlobalScopeClientImpl::didHandleFetchEvent(int fetchEventID, const WebServiceWorkerResponse& webResponse)
{
if (!response) {
m_client.didHandleFetchEvent(fetchEventID);
return;
}

WebServiceWorkerResponse webResponse;
response->populateWebServiceWorkerResponse(webResponse);
if (webResponse.status() == 0) {
// The status code is 0 means a network error.
m_client.didHandleFetchEvent(fetchEventID);
return;
}
m_client.didHandleFetchEvent(fetchEventID, webResponse);
}

Expand Down
4 changes: 3 additions & 1 deletion Source/web/ServiceWorkerGlobalScopeClientImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
namespace blink {

class WebServiceWorkerContextClient;
class WebServiceWorkerResponse;
class WebURL;

class ServiceWorkerGlobalScopeClientImpl FINAL : public NoBaseWillBeGarbageCollectedFinalized<ServiceWorkerGlobalScopeClientImpl>, public ServiceWorkerGlobalScopeClient {
Expand All @@ -52,7 +53,8 @@ class ServiceWorkerGlobalScopeClientImpl FINAL : public NoBaseWillBeGarbageColle

virtual void didHandleActivateEvent(int eventID, WebServiceWorkerEventResult) OVERRIDE;
virtual void didHandleInstallEvent(int installEventID, WebServiceWorkerEventResult) OVERRIDE;
virtual void didHandleFetchEvent(int fetchEventID, Response*) OVERRIDE;
virtual void didHandleFetchEvent(int fetchEventID) OVERRIDE;
virtual void didHandleFetchEvent(int fetchEventID, const WebServiceWorkerResponse&) OVERRIDE;
virtual void didHandleSyncEvent(int syncEventID) OVERRIDE;
virtual void postMessageToClient(int clientID, const WebString& message, PassOwnPtr<WebMessagePortChannelArray>) OVERRIDE;

Expand Down

0 comments on commit e0dc629

Please sign in to comment.