[ServiceWorker] Add WebDataSource argument to isControlledByServiceWorker() [1/3 blink]

[ServiceWorker] Add WebDataSource argument to isControlledByServiceWorker() [1/3 blink]

In current implementation, while the new page is loading the request from the existing previous page may refer to the provisional dataSource in isControlledByServiceWorker().
But this is wrong.
We should pass WebDataSource to this method to avoid the ambiguity.

1/3 blink: https://codereview.chromium.org/634813002
2/3 chromium: https://codereview.chromium.org/629953003
3/3 blink: https://codereview.chromium.org/635693003

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

[horo, 2014-10-07 02:21:56]

horo@chromium.org changed reviewers:
+ michaeln@chromium.org

[horo, 2014-10-07 02:21:56]

michaeln@
Could you please review this?

[Mike West, 2014-10-07 15:43:15]

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

[Mike West, 2014-10-07 15:43:16]

core/ and web/ LGTM, but you'll need someone else to sign off on public/.

[michaeln, 2014-10-07 19:28:24]

lgtm

https://codereview.chromium.org/634813002/diff/1/public/web/WebFrameClient.h
File public/web/WebFrameClient.h (right):

https://codereview.chromium.org/634813002/diff/1/public/web/WebFrameClient.h#...
public/web/WebFrameClient.h:572: // Whether the frame is controlled by the
ServiceWorker
Maybe for clarity alter the comment to read 'Whether the document associated
with WebDataSource...' since it's documents that are controlled.

[horo, 2014-10-07 22:47:32]

Thank you!

https://codereview.chromium.org/634813002/diff/1/public/web/WebFrameClient.h
File public/web/WebFrameClient.h (right):

https://codereview.chromium.org/634813002/diff/1/public/web/WebFrameClient.h#...
public/web/WebFrameClient.h:572: // Whether the frame is controlled by the
ServiceWorker
On 2014/10/07 19:28:24, michaeln wrote:
> Maybe for clarity alter the comment to read 'Whether the document associated
> with WebDataSource...' since it's documents that are controlled.

Done.

[horo, 2014-10-07 22:48:23]

horo@chromium.org changed reviewers:
+ tkent@chromium.org

[horo, 2014-10-07 22:48:23]

tkent@
Could you please review public/web/WebFrameClient.h?

[tkent, 2014-10-07 23:58:56]

lgtm

https://codereview.chromium.org/634813002/diff/20001/Source/core/loader/Frame...
File Source/core/loader/FrameLoaderClient.h (right):

https://codereview.chromium.org/634813002/diff/20001/Source/core/loader/Frame...
Source/core/loader/FrameLoaderClient.h:209: virtual bool
isControlledByServiceWorker(DocumentLoader*) = 0;
nit: If the argument is always non-null, please change it to DocumentLoader&.
I couldn't find any callsites of
FrameLoaderClient::isControlledByServiceWorker().

[horo, 2014-10-08 01:33:41]

Thank you!

https://codereview.chromium.org/634813002/diff/20001/Source/core/loader/Frame...
File Source/core/loader/FrameLoaderClient.h (right):

https://codereview.chromium.org/634813002/diff/20001/Source/core/loader/Frame...
Source/core/loader/FrameLoaderClient.h:209: virtual bool
isControlledByServiceWorker(DocumentLoader*) = 0;
On 2014/10/07 23:58:56, tkent wrote:
> nit: If the argument is always non-null, please change it to DocumentLoader&.
> I couldn't find any callsites of
> FrameLoaderClient::isControlledByServiceWorker().

Done.

[horo, 2014-10-08 03:01:21]

The CQ bit was checked by horo@chromium.org

[horo, 2014-10-08 03:01:22]

The CQ bit was unchecked by horo@chromium.org

[horo, 2014-10-08 03:01:44]

The CQ bit was checked by horo@chromium.org

[commit-bot: I haz the power, 2014-10-08 03:02:16]

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

[commit-bot: I haz the power, 2014-10-08 03:36:00]

Committed patchset #3 (id:40001) as 183375