ServiceWorker: Support includeUncontrolled option in clients.matchAll() (1/2, chromium)

ServiceWorker: Support includeUncontrolled option in clients.matchAll() (1/2, chromium)

* Add SWContextCore::GetProviderHostByClientID() method
* Add SWContextCore::GetProviderHostIterator() which takes predicate
* Make ServiceWorkerVersion::OnGetClients return uncontrolled host when specified
* Make ServiceWorkerVersion::On{PostMessage,Focus} work for uncontrolled hosts
  (i.e. now ServiceWorker can have clients that are not controlled by it)

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: blink patch (layout tests will be added here)

BUG=455241
TEST=ServiceWorkerContextTest.GetProviderHostIterator
TEST=layout tests will be added later

Committed: https://crrev.com/04848e8f57c823b674b0a3e03047e281fae44bfd
Cr-Commit-Position: refs/heads/master@{#321722}

[kinuko, 2015-03-15 04:59:45]

Patchset #1 (id:1) has been deleted

[kinuko, 2015-03-15 14:18:07]

Patchset #1 (id:20001) has been deleted

[kinuko, 2015-03-16 04:21:53]

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

[kinuko, 2015-03-16 05:18:51]

ptl

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

https://codereview.chromium.org/1007133002/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_provider_host.cc:99:
context_->UnregisterProviderHostByClientID(client_uuid_);
Somehow I missed to include this in the previous patch...

[nhiroki, 2015-03-16 06:37:59]

https://codereview.chromium.org/1007133002/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_context_core.cc (right):

https://codereview.chromium.org/1007133002/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_context_core.cc:120:
predicate_.Run(provider_host_iterator_->GetCurrentValue()))
nit: Calling ProviderHostIterator::GetProviderHost() would be simpler.

https://codereview.chromium.org/1007133002/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_version.cc (right):

https://codereview.chromium.org/1007133002/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_version.cc:997: if
(controllee_map_.empty() && options.include_uncontrolled) {
"controllee_map_.empty() && !options.include_uncontrolled" seems correct.

https://codereview.chromium.org/1007133002/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_version.cc:1024:
MakeTuple(host->process_id(), host->frame_id(), host->client_uuid()));
We might need to check host->provider_type() and to exclude a host whose type is
SERVICE_WORKER_PROVIDER_FOR_CONTROLLER??

[kinuko, 2015-03-20 08:19:40]

Thanks, updated.

https://codereview.chromium.org/1007133002/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_context_core.cc (right):

https://codereview.chromium.org/1007133002/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_context_core.cc:120:
predicate_.Run(provider_host_iterator_->GetCurrentValue()))
On 2015/03/16 06:37:58, nhiroki wrote:
> nit: Calling ProviderHostIterator::GetProviderHost() would be simpler.

Done.

https://codereview.chromium.org/1007133002/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_version.cc (right):

https://codereview.chromium.org/1007133002/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_version.cc:997: if
(controllee_map_.empty() && options.include_uncontrolled) {
On 2015/03/16 06:37:58, nhiroki wrote:
> "controllee_map_.empty() && !options.include_uncontrolled" seems correct.

Done.

This is how we need tests for every code...

https://codereview.chromium.org/1007133002/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_version.cc:1024:
MakeTuple(host->process_id(), host->frame_id(), host->client_uuid()));
On 2015/03/16 06:37:58, nhiroki wrote:
> We might need to check host->provider_type() and to exclude a host whose type
is
> SERVICE_WORKER_PROVIDER_FOR_CONTROLLER??

That's a good point, hmm yes it looks like. I changed the method to get client
provider hosts and named it 'GetClientProviderHostIterator'.

[nhiroki, 2015-03-20 09:08:06]

lgtm

https://codereview.chromium.org/1007133002/diff/60001/content/browser/service...
File content/browser/service_worker/service_worker_context_unittest.cc (right):

https://codereview.chromium.org/1007133002/diff/60001/content/browser/service...
content/browser/service_worker/service_worker_context_unittest.cc:639:
host3->SetDocumentUrl(kOrigin1);
According to the comment, this should be

host3 -> host4
kOrigin1 -> kOrigin2

[kinuko, 2015-03-20 09:19:13]

https://codereview.chromium.org/1007133002/diff/60001/content/browser/service...
File content/browser/service_worker/service_worker_context_unittest.cc (right):

https://codereview.chromium.org/1007133002/diff/60001/content/browser/service...
content/browser/service_worker/service_worker_context_unittest.cc:639:
host3->SetDocumentUrl(kOrigin1);
On 2015/03/20 09:08:05, nhiroki wrote:
> According to the comment, this should be
> 
> host3 -> host4
> kOrigin1 -> kOrigin2

:O fixed.

[kinuko, 2015-03-20 09:20:32]

The CQ bit was checked by kinuko@chromium.org

[kinuko, 2015-03-20 09:20:32]

The patchset sent to the CQ was uploaded after l-g-t-m from nhiroki@chromium.org
Link to the patchset: https://codereview.chromium.org/1007133002/#ps80001
(title: " ")

[commit-bot: I haz the power, 2015-03-20 09:20:55]

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

[commit-bot: I haz the power, 2015-03-20 10:42:55]

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

[commit-bot: I haz the power, 2015-03-20 10:42:56]

Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[kinuko, 2015-03-22 10:32:07]

The CQ bit was checked by kinuko@chromium.org

[kinuko, 2015-03-22 10:32:08]

The patchset sent to the CQ was uploaded after l-g-t-m from nhiroki@chromium.org
Link to the patchset: https://codereview.chromium.org/1007133002/#ps100001
(title: "adding export")

[commit-bot: I haz the power, 2015-03-22 10:32:17]

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

[kinuko, 2015-03-22 10:32:33]

The CQ bit was unchecked by kinuko@chromium.org

[kinuko, 2015-03-22 10:32:38]

Patchset #4 (id:100001) has been deleted

[kinuko, 2015-03-22 10:33:37]

The CQ bit was checked by kinuko@chromium.org

[kinuko, 2015-03-22 10:33:38]

The patchset sent to the CQ was uploaded after l-g-t-m from nhiroki@chromium.org
Link to the patchset: https://codereview.chromium.org/1007133002/#ps120001
(title: "Add EXPORT")

[commit-bot: I haz the power, 2015-03-22 10:33:45]

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

[commit-bot: I haz the power, 2015-03-22 16:05:41]

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

[commit-bot: I haz the power, 2015-03-22 16:05:43]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[kinuko, 2015-03-22 23:53:37]

The CQ bit was checked by kinuko@chromium.org

[commit-bot: I haz the power, 2015-03-22 23:53:51]

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

[commit-bot: I haz the power, 2015-03-23 00:30:39]

Committed patchset #4 (id:120001)

[commit-bot: I haz the power, 2015-03-23 00:31:13]

Patchset 4 (id:??) landed as
https://crrev.com/04848e8f57c823b674b0a3e03047e281fae44bfd
Cr-Commit-Position: refs/heads/master@{#321722}

[alph, 2015-03-23 09:37:42]

A revert of this CL (patchset #4 id:120001) has been created in
https://codereview.chromium.org/1024383003/ by alph@chromium.org.

The reason for reverting is: Broke
http/tests/serviceworker/chromium/clients-openwindow.html
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/45800.