ServiceWorker: plumbing for ClientQueryOptions (1/2, blink)

ServiceWorker: plumbing for ClientQueryOptions (1/2, blink)

To support Clients.matchAll(options)

* Add WebServiceWorkerClientQueryOptions to public interface
* Change getClients() method to take the options param
* Add clientType to WebServiceWorkerClientInfo so that embedder can start returning non-window clients at some point

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: THIS PATCH
2/2: https://codereview.chromium.org/970693003/ (chromium)

Tests will be added later after we implement embedder code.

BUG=455241, 460415
TEST=to be added

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

[kinuko, 2015-03-03 00:08:56]

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

[falken, 2015-03-03 02:11:25]

I didn't get an email for this but it showed up in my incoming reviews... not
sure if you want me to review yet.

FYI since we had trouble keeping track of changes in previous milestones, I
think in M43 we're planning to put web-exposed changes behind a feature flag.

https://codereview.chromium.org/958933004/diff/20001/public/platform/WebServi...
File public/platform/WebServiceWorkerClientType.h (right):

https://codereview.chromium.org/958933004/diff/20001/public/platform/WebServi...
public/platform/WebServiceWorkerClientType.h:13:
WebServiceWorkerClientTypeShaedWorker,
Shared

[kinuko, 2015-03-03 08:10:51]

Now it's ready for review :)

falken> I think in M43 we're planning to put web-exposed changes behind a
feature flag.

This won't change API surfaces but once we add impl it will stop returning
errors when non-default ClientQueryOptions is given. Do we still need to
implement it behind a flag?

https://codereview.chromium.org/958933004/diff/20001/public/platform/WebServi...
File public/platform/WebServiceWorkerClientType.h (right):

https://codereview.chromium.org/958933004/diff/20001/public/platform/WebServi...
public/platform/WebServiceWorkerClientType.h:13:
WebServiceWorkerClientTypeShaedWorker,
On 2015/03/03 02:11:25, falken wrote:
> Shared

Done.

[falken, 2015-03-04 06:53:36]

> This won't change API surfaces but once we add impl it will stop returning
> errors when non-default ClientQueryOptions is given. Do we still need to
> implement it behind a flag?

I think we should do an intent to ship for it. Actually we already deprecated
getAll and added matchAll without an intent to ship, and we plan to remove
getAll in M43. So we should do an intent to ship mentioning all changes:
1) add matchAll
2) remove getAll
3) support ClientQueryOptions.type
4) support ClientQueryOptions.includeUncontrolled

Would you be willing to do an "intent to implement and ship" to blink-dev for
these changes? Then you don't need a flag.

Otherwise, you could implement behind a flag, and then we could include the
changes in the next intent to ship iteration.

[falken, 2015-03-04 06:58:20]

code change lgtm

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

On 2015/03/04 06:53:36, falken wrote:
> > This won't change API surfaces but once we add impl it will stop returning
> > errors when non-default ClientQueryOptions is given. Do we still need to
> > implement it behind a flag?
> 
> I think we should do an intent to ship for it. Actually we already deprecated
> getAll and added matchAll without an intent to ship, and we plan to remove
> getAll in M43. So we should do an intent to ship mentioning all changes:
> 1) add matchAll
> 2) remove getAll
> 3) support ClientQueryOptions.type
> 4) support ClientQueryOptions.includeUncontrolled
> 
> Would you be willing to do an "intent to implement and ship" to blink-dev for
> these changes? Then you don't need a flag.

I can, but if we think sending out Intent email in a batch would make things
easier for us I can just keep these behind a flag. (Let me talk to you later)

> Otherwise, you could implement behind a flag, and then we could include the
> changes in the next intent to ship iteration.

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

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

[kinuko, 2015-03-05 02:31:16]

Kent-san, could you review public/ and Source/web changes?

[tkent, 2015-03-05 02:33:51]

lgtm

[kinuko, 2015-03-05 03:11:00]

The CQ bit was checked by kinuko@chromium.org

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

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

[commit-bot: I haz the power, 2015-03-05 04:30:23]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=191336