ServiceWorker: Make a way to retrieve a service worker version id (Chromium)

ServiceWorker: Make a way to retrieve a service worker version id (Chromium)

This is used to make a key of MemoryCache:
https://codereview.chromium.org/640463003/

Blink-side: https://codereview.chromium.org/650033007/

BUG=423219
TEST=n/a

Committed: https://crrev.com/febadeedcd8bf2339a2718b4fbeaaab35cf1fce9
Cr-Commit-Position: refs/heads/master@{#301782}

[nhiroki, 2014-10-24 01:58:55]

Patchset #1 (id:1) has been deleted

[nhiroki, 2014-10-28 00:59:18]

Patchset #1 (id:20001) has been deleted

[nhiroki, 2014-10-28 00:59:26]

Patchset #1 (id:40001) has been deleted

[nhiroki, 2014-10-28 01:00:49]

nhiroki@chromium.org changed reviewers:
+ horo@chromium.org, jamesr@chromium.org

[nhiroki, 2014-10-28 01:01:29]

nhiroki@chromium.org changed reviewers:
+ nasko@chromium.org

[nhiroki, 2014-10-28 01:06:48]

Hi, could you review this?

horo@ for whole changes.
jamesr@ for content/renderer/*
nasko@ for service_worker_messages.h

Thanks.

[horo, 2014-10-28 01:21:27]

lgtm

[Michael van Ouwerkerk, 2014-10-28 11:15:59]

mvanouwerkerk@chromium.org changed reviewers:
+ mvanouwerkerk@chromium.org

[Michael van Ouwerkerk, 2014-10-28 11:16:00]

https://codereview.chromium.org/669303003/diff/80001/content/renderer/render_...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/669303003/diff/80001/content/renderer/render_...
content/renderer/render_frame_impl.cc:3272: int64_t
RenderFrameImpl::serviceWorkerID(WebDataSource& data_source) {
Drive-by nit: why not just int64 like in the rest of this file?

[nhiroki, 2014-10-28 12:08:55]

https://codereview.chromium.org/669303003/diff/80001/content/renderer/render_...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/669303003/diff/80001/content/renderer/render_...
content/renderer/render_frame_impl.cc:3272: int64_t
RenderFrameImpl::serviceWorkerID(WebDataSource& data_source) {
On 2014/10/28 11:16:00, Michael van Ouwerkerk wrote:
> Drive-by nit: why not just int64 like in the rest of this file?

There are two reasons:
1) According to base/basictypes.h, int64 was already deprecated and using
int64_t is recommended.
2) This overrides WebFrameClient::serviceWorkerID newly defined in my blink-side
patch. I think new interface shouldn't depend on the deprecated data type.

[Michael van Ouwerkerk, 2014-10-28 12:17:21]

https://codereview.chromium.org/669303003/diff/80001/content/renderer/render_...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/669303003/diff/80001/content/renderer/render_...
content/renderer/render_frame_impl.cc:3272: int64_t
RenderFrameImpl::serviceWorkerID(WebDataSource& data_source) {
On 2014/10/28 12:08:54, nhiroki wrote:
> On 2014/10/28 11:16:00, Michael van Ouwerkerk wrote:
> > Drive-by nit: why not just int64 like in the rest of this file?
> 
> There are two reasons:
> 1) According to base/basictypes.h, int64 was already deprecated and using
> int64_t is recommended.
> 2) This overrides WebFrameClient::serviceWorkerID newly defined in my
blink-side
> patch. I think new interface shouldn't depend on the deprecated data type.

Cool, thanks!

[Michael van Ouwerkerk, 2014-10-28 12:17:30]

mvanouwerkerk@chromium.org changed reviewers:
- mvanouwerkerk@chromium.org

[nasko, 2014-10-28 22:30:08]

LGTM with a nit.

https://codereview.chromium.org/669303003/diff/80001/content/renderer/render_...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/669303003/diff/80001/content/renderer/render_...
content/renderer/render_frame_impl.cc:3279: return
kInvalidServiceWorkerVersionId;
nit: an empty line above will make it a bit more readable

[jamesr, 2014-10-28 22:31:37]

content/renderer/ lgtm % nasko's feedback

https://codereview.chromium.org/669303003/diff/80001/content/renderer/render_...
File content/renderer/render_frame_impl.h (right):

https://codereview.chromium.org/669303003/diff/80001/content/renderer/render_...
content/renderer/render_frame_impl.h:461: virtual int64_t
serviceWorkerID(blink::WebDataSource&);
in chromium land (which this is) all parameters get a name

[nhiroki, 2014-10-29 03:10:57]

Patchset #3 (id:100001) has been deleted

[nhiroki, 2014-10-29 03:11:52]

Thank you. Updated.

https://codereview.chromium.org/669303003/diff/80001/content/renderer/render_...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/669303003/diff/80001/content/renderer/render_...
content/renderer/render_frame_impl.cc:3279: return
kInvalidServiceWorkerVersionId;
On 2014/10/28 22:30:08, nasko wrote:
> nit: an empty line above will make it a bit more readable

Done.

https://codereview.chromium.org/669303003/diff/80001/content/renderer/render_...
File content/renderer/render_frame_impl.h (right):

https://codereview.chromium.org/669303003/diff/80001/content/renderer/render_...
content/renderer/render_frame_impl.h:461: virtual int64_t
serviceWorkerID(blink::WebDataSource&);
On 2014/10/28 22:31:37, jamesr wrote:
> in chromium land (which this is) all parameters get a name

Done.

[nhiroki, 2014-10-29 03:11:57]

The CQ bit was checked by nhiroki@chromium.org

[commit-bot: I haz the power, 2014-10-29 03:12:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/669303003/120001

[commit-bot: I haz the power, 2014-10-29 04:03:00]

Committed patchset #3 (id:120001)

[commit-bot: I haz the power, 2014-10-29 04:03:51]

Patchset 3 (id:??) landed as
https://crrev.com/febadeedcd8bf2339a2718b4fbeaaab35cf1fce9
Cr-Commit-Position: refs/heads/master@{#301782}