ServiceWorker: Implement ServiceWorkerRegistration [1/3]

ServiceWorker: Implement ServiceWorkerRegistration [1/3]

This implements a part of ServiceWorkerRegistration interface as per the latest
spec, and makes navigator.serviceWorker.register() resolve a promise with a
ServiceWorkerRegistration object instead of a ServiceWorker object.

Spec: https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#service-worker-registration-obj

[1] Blink: This patch.
[2] Chromium: https://codereview.chromium.org/415963002/
[3] Blink: Remove the macro and fix a bunch of layout tests.


BUG=396400
TEST=compile (tests will be added by subsequent patches)

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=178919

[nhiroki, 2014-07-24 07:19:15]

Hi, can you review this? Thanks!

[falken, 2014-07-25 06:10:24]

lgtm

https://codereview.chromium.org/413123002/diff/100001/Source/modules/servicew...
File Source/modules/serviceworkers/ServiceWorkerRegistration.cpp (right):

https://codereview.chromium.org/413123002/diff/100001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorkerRegistration.cpp:54: return
serviceWorkerRegistration;
can this just be return ServiceWorkerRegistration::from(...)

[nhiroki, 2014-07-25 06:55:14]

https://codereview.chromium.org/413123002/diff/100001/Source/modules/servicew...
File Source/modules/serviceworkers/ServiceWorkerRegistration.cpp (right):

https://codereview.chromium.org/413123002/diff/100001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorkerRegistration.cpp:54: return
serviceWorkerRegistration;
On 2014/07/25 06:10:23, falken wrote:
> can this just be return ServiceWorkerRegistration::from(...)

Done.

[nhiroki, 2014-07-25 07:02:43]

tkent@: could you review this as an OWNER of following files?

- Source/modules/EventTargetModulesFactory.in
- Source/modules/serviceworkers/ServiceWorkerRegistration.idl
- public/platform/*

Thanks.

[tkent, 2014-07-25 07:50:25]

https://codereview.chromium.org/413123002/diff/120001/Source/modules/servicew...
File Source/modules/serviceworkers/ServiceWorkerRegistration.cpp (right):

https://codereview.chromium.org/413123002/diff/120001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorkerRegistration.cpp:44:
PassRefPtrWillBeRawPtr<ServiceWorkerRegistration>
ServiceWorkerRegistration::from(ExecutionContext* executionContext, WebType*
registration)
We don't name 'from' for such function.  'from' is used for function to get
something from other object.
This should be creteIfFoo

https://codereview.chromium.org/413123002/diff/120001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorkerRegistration.cpp:51:
PassRefPtrWillBeRawPtr<ServiceWorkerRegistration>
ServiceWorkerRegistration::from(ScriptPromiseResolver* resolver, WebType*
registration)
Ditto.

https://codereview.chromium.org/413123002/diff/120001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorkerRegistration.cpp:83:
PassRefPtrWillBeRawPtr<ServiceWorkerRegistration>
ServiceWorkerRegistration::create(ExecutionContext* executionContext,
PassOwnPtr<blink::WebServiceWorkerRegistration> outerRegistration)
Remove blink::

https://codereview.chromium.org/413123002/diff/120001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorkerRegistration.cpp:90:
ServiceWorkerRegistration::ServiceWorkerRegistration(ExecutionContext*
executionContext, PassOwnPtr<blink::WebServiceWorkerRegistration>
outerRegistration)
Remove blink::

https://codereview.chromium.org/413123002/diff/120001/Source/modules/servicew...
File Source/modules/serviceworkers/ServiceWorkerRegistration.h (right):

https://codereview.chromium.org/413123002/diff/120001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorkerRegistration.h:37: typedef
blink::WebServiceWorkerRegistration WebType;
Remove blink::

https://codereview.chromium.org/413123002/diff/120001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorkerRegistration.h:48:
ServiceWorkerRegistration(ExecutionContext*,
PassOwnPtr<blink::WebServiceWorkerRegistration>);
Remove blink::

https://codereview.chromium.org/413123002/diff/120001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorkerRegistration.h:50:
OwnPtr<blink::WebServiceWorkerRegistration> m_outerRegistration;
Remove blink::

https://codereview.chromium.org/413123002/diff/120001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorkerRegistration.h:51:
blink::WebServiceWorkerProvider* m_provider;
Remove blink::

https://codereview.chromium.org/413123002/diff/120001/public/platform/WebServ...
File public/platform/WebServiceWorkerProvider.h (right):

https://codereview.chromium.org/413123002/diff/120001/public/platform/WebServ...
public/platform/WebServiceWorkerProvider.h:53: // FIXME: Remove this macro after
two-side patches are landed (http://crbug.com/396400).
nit: I recommend to wrap code comments in 80 columns.

https://codereview.chromium.org/413123002/diff/120001/public/platform/WebServ...
public/platform/WebServiceWorkerProvider.h:60: // The
WebServiceWorkerRegistration and WebServiceWorkerError ownership are passed to
the WebServiceWorkerRegistrationCallbacks implementation.
Ditto.

https://codereview.chromium.org/413123002/diff/120001/public/platform/WebServ...
public/platform/WebServiceWorkerProvider.h:68: // FIXME: "unregister" does not
provide a WebServiceWorkerRegistration, revisit this
Ditto.

https://codereview.chromium.org/413123002/diff/120001/public/platform/WebServ...
File public/platform/WebServiceWorkerRegistration.h (right):

https://codereview.chromium.org/413123002/diff/120001/public/platform/WebServ...
public/platform/WebServiceWorkerRegistration.h:8: #include "WebURL.h"
WebURL.h -> public/platform/WebURL.h

https://codereview.chromium.org/413123002/diff/120001/public/platform/WebServ...
public/platform/WebServiceWorkerRegistration.h:21: #endif //
Webserviceworkerregistration_h
Webserviceworkerregistration_h -> WebServiceWorkerRegistration_h

[nhiroki, 2014-07-25 08:28:57]

Thank you for reviewing. Updated.

https://codereview.chromium.org/413123002/diff/120001/Source/modules/servicew...
File Source/modules/serviceworkers/ServiceWorkerRegistration.cpp (right):

https://codereview.chromium.org/413123002/diff/120001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorkerRegistration.cpp:44:
PassRefPtrWillBeRawPtr<ServiceWorkerRegistration>
ServiceWorkerRegistration::from(ExecutionContext* executionContext, WebType*
registration)
On 2014/07/25 07:50:24, tkent wrote:
> We don't name 'from' for such function.  'from' is used for function to get
> something from other object.
> This should be creteIfFoo

Removed (this was not necessary).

https://codereview.chromium.org/413123002/diff/120001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorkerRegistration.cpp:51:
PassRefPtrWillBeRawPtr<ServiceWorkerRegistration>
ServiceWorkerRegistration::from(ScriptPromiseResolver* resolver, WebType*
registration)
On 2014/07/25 07:50:24, tkent wrote:
> Ditto.

This function name is defined by CallbackPromiseAdapter, so let me leave this as
is for now.

(I'm thinking about renaming this after http://crrev.com/396963002 is landed).

https://codereview.chromium.org/413123002/diff/120001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorkerRegistration.cpp:83:
PassRefPtrWillBeRawPtr<ServiceWorkerRegistration>
ServiceWorkerRegistration::create(ExecutionContext* executionContext,
PassOwnPtr<blink::WebServiceWorkerRegistration> outerRegistration)
On 2014/07/25 07:50:24, tkent wrote:
> Remove blink::

Done.

https://codereview.chromium.org/413123002/diff/120001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorkerRegistration.cpp:90:
ServiceWorkerRegistration::ServiceWorkerRegistration(ExecutionContext*
executionContext, PassOwnPtr<blink::WebServiceWorkerRegistration>
outerRegistration)
On 2014/07/25 07:50:24, tkent wrote:
> Remove blink::

Done.

https://codereview.chromium.org/413123002/diff/120001/Source/modules/servicew...
File Source/modules/serviceworkers/ServiceWorkerRegistration.h (right):

https://codereview.chromium.org/413123002/diff/120001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorkerRegistration.h:37: typedef
blink::WebServiceWorkerRegistration WebType;
On 2014/07/25 07:50:24, tkent wrote:
> Remove blink::

Done.

https://codereview.chromium.org/413123002/diff/120001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorkerRegistration.h:48:
ServiceWorkerRegistration(ExecutionContext*,
PassOwnPtr<blink::WebServiceWorkerRegistration>);
On 2014/07/25 07:50:24, tkent wrote:
> Remove blink::

Done.

https://codereview.chromium.org/413123002/diff/120001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorkerRegistration.h:50:
OwnPtr<blink::WebServiceWorkerRegistration> m_outerRegistration;
On 2014/07/25 07:50:24, tkent wrote:
> Remove blink::

Done.

https://codereview.chromium.org/413123002/diff/120001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorkerRegistration.h:51:
blink::WebServiceWorkerProvider* m_provider;
On 2014/07/25 07:50:24, tkent wrote:
> Remove blink::

Done.

https://codereview.chromium.org/413123002/diff/120001/public/platform/WebServ...
File public/platform/WebServiceWorkerProvider.h (right):

https://codereview.chromium.org/413123002/diff/120001/public/platform/WebServ...
public/platform/WebServiceWorkerProvider.h:53: // FIXME: Remove this macro after
two-side patches are landed (http://crbug.com/396400).
On 2014/07/25 07:50:24, tkent wrote:
> nit: I recommend to wrap code comments in 80 columns.

Done.

https://codereview.chromium.org/413123002/diff/120001/public/platform/WebServ...
public/platform/WebServiceWorkerProvider.h:60: // The
WebServiceWorkerRegistration and WebServiceWorkerError ownership are passed to
the WebServiceWorkerRegistrationCallbacks implementation.
On 2014/07/25 07:50:24, tkent wrote:
> Ditto.

Done.

https://codereview.chromium.org/413123002/diff/120001/public/platform/WebServ...
public/platform/WebServiceWorkerProvider.h:68: // FIXME: "unregister" does not
provide a WebServiceWorkerRegistration, revisit this
On 2014/07/25 07:50:24, tkent wrote:
> Ditto.

Done.

https://codereview.chromium.org/413123002/diff/120001/public/platform/WebServ...
File public/platform/WebServiceWorkerRegistration.h (right):

https://codereview.chromium.org/413123002/diff/120001/public/platform/WebServ...
public/platform/WebServiceWorkerRegistration.h:8: #include "WebURL.h"
On 2014/07/25 07:50:25, tkent wrote:
> WebURL.h -> public/platform/WebURL.h

Done.

https://codereview.chromium.org/413123002/diff/120001/public/platform/WebServ...
public/platform/WebServiceWorkerRegistration.h:21: #endif //
Webserviceworkerregistration_h
On 2014/07/25 07:50:25, tkent wrote:
> Webserviceworkerregistration_h -> WebServiceWorkerRegistration_h

Done.

[tkent, 2014-07-25 08:32:54]

lgtm

[nhiroki, 2014-07-25 08:42:59]

The CQ bit was checked by nhiroki@chromium.org

[commit-bot: I haz the power, 2014-07-25 08:43:15]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/nhiroki@chromium.org/413123002/160001

[commit-bot: I haz the power, 2014-07-25 09:37:48]

Change committed as 178919