ServiceWorker: plumbing for ClientQueryOptions (2/2, chromium)

ServiceWorker: plumbing for ClientQueryOptions (2/2, chromium)

To support Clients.matchAll(options)

* Add ServiceWorkerClientQueryOptions param to GetClients IPC msg (renderer -> browser)
* Add client_type field to ServiceWorkerClientInfo for DidGetClients IPC msg (browser -> renderer)

Spec links:
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#serviceworker-client-query-options-dictionary
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#clients-getall

1/2: https://codereview.chromium.org/958933004/ (blink)
2/2: THIS PATCH

BUG=455241, 460415
TEST=to be added after we add actual impl

Committed: https://crrev.com/4042259c2be1dd127ec0bb10b4f116b7a0548500
Cr-Commit-Position: refs/heads/master@{#319588}

[kinuko, 2015-03-03 07:56:28]

Patchset #2 (id:20001) has been deleted

[kinuko, 2015-03-03 07:56:38]

Patchset #1 (id:1) has been deleted

[kinuko, 2015-03-03 08:11:18]

kinuko@chromium.org changed reviewers:
+ falken@chromium.org

[kinuko, 2015-03-03 08:11:30]

Chromium-side patch.

[falken, 2015-03-04 07:16:43]

I think this looks good.

https://codereview.chromium.org/970693003/diff/40001/content/browser/service_...
File content/browser/service_worker/service_worker_provider_host.cc (right):

https://codereview.chromium.org/970693003/diff/40001/content/browser/service_...
content/browser/service_worker/service_worker_provider_host.cc:50:
blink::WebServiceWorkerClientTypeWindow);
maybe I'm lost, how do we know this is a window and not a shared worker?

https://codereview.chromium.org/970693003/diff/40001/content/common/service_w...
File content/common/service_worker/service_worker_messages.h (right):

https://codereview.chromium.org/970693003/diff/40001/content/common/service_w...
content/common/service_worker/service_worker_messages.h:237: // Asks the browser
to retrieve clients controlled by the sender
I guess "controlled" is now wrong too, since there's include_uncontrolled.

[kinuko, 2015-03-05 02:30:23]

https://codereview.chromium.org/970693003/diff/40001/content/browser/service_...
File content/browser/service_worker/service_worker_provider_host.cc (right):

https://codereview.chromium.org/970693003/diff/40001/content/browser/service_...
content/browser/service_worker/service_worker_provider_host.cc:50:
blink::WebServiceWorkerClientTypeWindow);
On 2015/03/04 07:16:42, falken wrote:
> maybe I'm lost, how do we know this is a window and not a shared worker?

We collect these clients using render_frame_id, which is -1 for worker cases.

[falken, 2015-03-05 14:23:02]

ok lgtm

https://codereview.chromium.org/970693003/diff/40001/content/browser/service_...
File content/browser/service_worker/service_worker_provider_host.cc (right):

https://codereview.chromium.org/970693003/diff/40001/content/browser/service_...
content/browser/service_worker/service_worker_provider_host.cc:50:
blink::WebServiceWorkerClientTypeWindow);
On 2015/03/05 02:30:23, kinuko wrote:
> On 2015/03/04 07:16:42, falken wrote:
> > maybe I'm lost, how do we know this is a window and not a shared worker?
> 
> We collect these clients using render_frame_id, which is -1 for worker cases.

Acknowledged.

[kinuko, 2015-03-06 06:03:25]

kinuko@chromium.org changed reviewers:
+ tkent@chromium.org, tsepez@chromium.org

[kinuko, 2015-03-06 06:03:39]

kinuko@chromium.org changed reviewers:
- tkent@chromium.org

[kinuko, 2015-03-06 06:04:12]

kinuko@chromium.org changed reviewers:
+ avi@chromium.org

[kinuko, 2015-03-06 06:04:47]

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

[kinuko, 2015-03-06 06:05:44]

tsepez@: Could you review service_worker_messages.h change?

tkent@: Could you review common/DEPS change for WebServiceWorkerClientType.h,
that is enum-only header file.

avi@: Could you stamp common/DEPS change?

[tkent, 2015-03-06 06:17:22]

content/common/DEPS lgtm

[Avi (use Gerrit), 2015-03-06 16:14:44]

On 2015/03/06 06:17:22, tkent wrote:
> content/common/DEPS lgtm

content/common/DEPS lgtm 2

[Tom Sepez, 2015-03-06 17:53:22]

LGTM with truly minor nit for which I apologize.

https://codereview.chromium.org/970693003/diff/40001/content/common/service_w...
File content/common/service_worker/service_worker_messages.h (right):

https://codereview.chromium.org/970693003/diff/40001/content/common/service_w...
content/common/service_worker/service_worker_messages.h:125:
IPC_STRUCT_TRAITS_MEMBER(include_uncontrolled)
nit: "uncontrolled" is an adjective, I'd like a noun here.  Maybe
include_uncontrolled_clients"?

[kinuko, 2015-03-07 12:11:38]

Thanks!

https://codereview.chromium.org/970693003/diff/40001/content/common/service_w...
File content/common/service_worker/service_worker_messages.h (right):

https://codereview.chromium.org/970693003/diff/40001/content/common/service_w...
content/common/service_worker/service_worker_messages.h:125:
IPC_STRUCT_TRAITS_MEMBER(include_uncontrolled)
On 2015/03/06 17:53:21, Tom Sepez wrote:
> nit: "uncontrolled" is an adjective, I'd like a noun here.  Maybe
> include_uncontrolled_clients"?

Yup I agree that a noun'd be better... while this field name is taken from the
spec [1], so I'd like to stick to it to match with it.

[1]
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#serviceworke...

https://codereview.chromium.org/970693003/diff/40001/content/common/service_w...
content/common/service_worker/service_worker_messages.h:237: // Asks the browser
to retrieve clients controlled by the sender
On 2015/03/04 07:16:43, falken wrote:
> I guess "controlled" is now wrong too, since there's include_uncontrolled.

Done.

[kinuko, 2015-03-07 12:12:06]

The CQ bit was checked by kinuko@chromium.org

[kinuko, 2015-03-07 12:12:07]

The patchset sent to the CQ was uploaded after l-g-t-m from falken@chromium.org,
tsepez@chromium.org, avi@chromium.org, tkent@chromium.org
Link to the patchset: https://codereview.chromium.org/970693003/#ps60001 (title:
"rebase + addressed comments")

[commit-bot: I haz the power, 2015-03-07 12:13:07]

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

[commit-bot: I haz the power, 2015-03-07 13:20:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-07 13:20:15]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_TIMED_OUT, no build
URL)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_TIMED_OUT, no build URL)
Timed out jobs are not retried to avoid causing additional load on the builders
with large pending queues.

[kinuko, 2015-03-08 06:55:28]

The CQ bit was checked by kinuko@chromium.org

[kinuko, 2015-03-08 06:55:29]

The patchset sent to the CQ was uploaded after l-g-t-m from falken@chromium.org,
tsepez@chromium.org, avi@chromium.org, tkent@chromium.org
Link to the patchset: https://codereview.chromium.org/970693003/#ps80001 (title:
"rebase")

[commit-bot: I haz the power, 2015-03-08 06:55:36]

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

[commit-bot: I haz the power, 2015-03-08 07:28:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-08 07:28:11]

Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[kinuko, 2015-03-08 07:54:59]

The CQ bit was checked by kinuko@chromium.org

[kinuko, 2015-03-08 07:54:59]

The patchset sent to the CQ was uploaded after l-g-t-m from falken@chromium.org,
tsepez@chromium.org, avi@chromium.org, tkent@chromium.org
Link to the patchset: https://codereview.chromium.org/970693003/#ps100001
(title: "rebase")

[commit-bot: I haz the power, 2015-03-08 07:55:16]

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

[commit-bot: I haz the power, 2015-03-08 08:28:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-08 08:28:29]

Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[kinuko, 2015-03-08 08:36:06]

The CQ bit was checked by kinuko@chromium.org

[kinuko, 2015-03-08 08:36:07]

The patchset sent to the CQ was uploaded after l-g-t-m from falken@chromium.org,
tsepez@chromium.org, avi@chromium.org, tkent@chromium.org
Link to the patchset: https://codereview.chromium.org/970693003/#ps120001
(title: "rebase build fix...")

[commit-bot: I haz the power, 2015-03-08 08:36:34]

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

[commit-bot: I haz the power, 2015-03-08 09:45:06]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2015-03-08 09:45:47]

Patchset 5 (id:??) landed as
https://crrev.com/4042259c2be1dd127ec0bb10b4f116b7a0548500
Cr-Commit-Position: refs/heads/master@{#319588}