ServiceWorker: Add 'updatefound' and version attributes on SWRegistration (Blink)

ServiceWorker: Add 'updatefound' and version attributes on SWRegistration (Blink)

This change...
- adds 'onupdatefound' event and version attribtues (eg. '.installing') into
ServiceWorkerRegistration interface.
- adds WebServiceWorkerRegistrationProxy interface to talk to the registration
object from embedder.

[NOTE] ServiceWorkerRegistration is still disabled by the macro since Chromium
side is still actively being implemented:
https://codereview.chromium.org/427523002/

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


BUG=396400
TEST=compile (layout tests will be added after enabling the registration interface)

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

[nhiroki, 2014-08-08 08:09:24]

Hi falken, can you review this?

Chromium-side patches are still actively under development, but Blink-side, that
is, this patch probably won't be affected from the changes I think.

[falken, 2014-08-08 10:09:03]

looks good

https://codereview.chromium.org/424693004/diff/200001/Source/modules/servicew...
File Source/modules/serviceworkers/ServiceWorker.cpp (right):

https://codereview.chromium.org/424693004/diff/200001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorker.cpp:145: // should be changed to the
ready state immediately.
This comment kind of repeats the code and doesn't help me understand what's
happening. Personally I'd just delete it.

https://codereview.chromium.org/424693004/diff/200001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorker.cpp:175: ASSERT(state != Initial);
Better to be explicit here and whitelist the allowed states. The enum values may
change someday.

https://codereview.chromium.org/424693004/diff/200001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorker.cpp:227:
PassRefPtrWillBeRawPtr<ServiceWorker>
ServiceWorker::createIfNeeded(ExecutionContext* executionContext, WebType*
outerWorker)
I'm not a fan of this name, you might expect it to return null if not created.
How about getOrCreate?

https://codereview.chromium.org/424693004/diff/200001/public/platform/WebServ...
File public/platform/WebServiceWorkerRegistrationProxy.h (right):

https://codereview.chromium.org/424693004/diff/200001/public/platform/WebServ...
public/platform/WebServiceWorkerRegistrationProxy.h:20: // Notifies that the
registration enters the installation process.
will enter? or entered?

[nhiroki, 2014-08-08 10:36:24]

Thanks! Updated.

https://codereview.chromium.org/424693004/diff/200001/Source/modules/servicew...
File Source/modules/serviceworkers/ServiceWorker.cpp (right):

https://codereview.chromium.org/424693004/diff/200001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorker.cpp:145: // should be changed to the
ready state immediately.
On 2014/08/08 10:09:02, falken wrote:
> This comment kind of repeats the code and doesn't help me understand what's
> happening. Personally I'd just delete it.

Done.

https://codereview.chromium.org/424693004/diff/200001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorker.cpp:175: ASSERT(state != Initial);
On 2014/08/08 10:09:02, falken wrote:
> Better to be explicit here and whitelist the allowed states. The enum values
may
> change someday.

Done.

https://codereview.chromium.org/424693004/diff/200001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorker.cpp:227:
PassRefPtrWillBeRawPtr<ServiceWorker>
ServiceWorker::createIfNeeded(ExecutionContext* executionContext, WebType*
outerWorker)
On 2014/08/08 10:09:02, falken wrote:
> I'm not a fan of this name, you might expect it to return null if not created.
> How about getOrCreate?

SGTM. Renamed it to "getOrCreate".

https://codereview.chromium.org/424693004/diff/200001/public/platform/WebServ...
File public/platform/WebServiceWorkerRegistrationProxy.h (right):

https://codereview.chromium.org/424693004/diff/200001/public/platform/WebServ...
public/platform/WebServiceWorkerRegistrationProxy.h:20: // Notifies that the
registration enters the installation process.
On 2014/08/08 10:09:02, falken wrote:
> will enter? or entered?

Done. This event is fired after the worker is marked with 'installing', so I
think 'entered' would be correct here.

[nhiroki, 2014-08-08 10:40:43]

+tkent@

Hi kent-san, can you review Source/core/* and public/platform/*? Thanks!

[falken, 2014-08-08 12:12:12]

lgtm

[tkent, 2014-08-11 01:31:32]

lgtm

https://codereview.chromium.org/424693004/diff/220001/Source/modules/servicew...
File Source/modules/serviceworkers/ServiceWorker.cpp (right):

https://codereview.chromium.org/424693004/diff/220001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorker.cpp:142: RefPtr<ServiceWorker>
serviceWorker = getOrCreate(executionContext, worker);
RefPtr -> RefPtrWillBeRawPtr

https://codereview.chromium.org/424693004/diff/220001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorker.cpp:229:
blink::WebServiceWorkerProxy* proxy = outerWorker->proxy();
Remove blink::

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

https://codereview.chromium.org/424693004/diff/220001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorkerRegistration.cpp:44: static void
deleteIfNoExistingOwner(blink::WebServiceWorker* serviceWorker)
Remove blink::

[nhiroki, 2014-08-11 02:26:06]

Thank you for reviewing.

https://codereview.chromium.org/424693004/diff/220001/Source/modules/servicew...
File Source/modules/serviceworkers/ServiceWorker.cpp (right):

https://codereview.chromium.org/424693004/diff/220001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorker.cpp:142: RefPtr<ServiceWorker>
serviceWorker = getOrCreate(executionContext, worker);
On 2014/08/11 01:31:31, tkent wrote:
> RefPtr -> RefPtrWillBeRawPtr

Done.

https://codereview.chromium.org/424693004/diff/220001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorker.cpp:229:
blink::WebServiceWorkerProxy* proxy = outerWorker->proxy();
On 2014/08/11 01:31:31, tkent wrote:
> Remove blink::

Done.

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

https://codereview.chromium.org/424693004/diff/220001/Source/modules/servicew...
Source/modules/serviceworkers/ServiceWorkerRegistration.cpp:44: static void
deleteIfNoExistingOwner(blink::WebServiceWorker* serviceWorker)
On 2014/08/11 01:31:32, tkent wrote:
> Remove blink::

Done.

[nhiroki, 2014-08-11 02:26:10]

The CQ bit was checked by nhiroki@chromium.org

[commit-bot: I haz the power, 2014-08-11 02:26:36]

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

[commit-bot: I haz the power, 2014-08-11 03:25:49]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/19195)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/21946)

[commit-bot: I haz the power, 2014-08-11 04:05:35]

Change committed as 179903