[1/5 chromium] Shows the clients which are controlled by ServiceWorker in DevTools.

[1/5 chromium] Shows the clients which are controlled by ServiceWorker in DevTools.

This cl implements ServiceWorkerHandler::ActivateTarget() to focus the tab which is controlled by ServiceWorker.

Screenshot: https://code.google.com/p/chromium/issues/detail?id=466871#c65

1/5 chromium: This cl.
2/5 blink: https://codereview.chromium.org/1151993003/
3/5 chromium: https://codereview.chromium.org/1149383004/
4/5 blink: https://codereview.chromium.org/1164583002/
5/5 chromium: https://codereview.chromium.org/1143363009/

BUG=466871
Committed: https://crrev.com/938e803e0a762fde0935ad7005e8f84d84f7e36e
Cr-Commit-Position: refs/heads/master@{#333671}

[horo, 2015-05-29 08:55:53]

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

[horo, 2015-05-29 08:55:54]

nhiroki@
Could you please review this?

[nhiroki, 2015-06-01 05:04:09]

LGTM with nits

https://codereview.chromium.org/1160133002/diff/1/content/browser/service_wor...
File content/browser/service_worker/service_worker_utils.h (right):

https://codereview.chromium.org/1160133002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_utils.h:9: #include
"content/browser/service_worker/service_worker_provider_host.h"
nit: not necessary?

https://codereview.chromium.org/1160133002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_utils.h:26: static void
NoOpClientInfoCallback(const ServiceWorkerClientInfo&) {}
Can you move this into service_worker_context_wrapper.cc?

(NoOpStatusCallback is widely used among SW components, but this is not)

[horo, 2015-06-02 03:14:48]

Thank you

https://codereview.chromium.org/1160133002/diff/1/content/browser/service_wor...
File content/browser/service_worker/service_worker_utils.h (right):

https://codereview.chromium.org/1160133002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_utils.h:26: static void
NoOpClientInfoCallback(const ServiceWorkerClientInfo&) {}
On 2015/06/01 05:04:09, nhiroki wrote:
> Can you move this into service_worker_context_wrapper.cc?
> 
> (NoOpStatusCallback is widely used among SW components, but this is not)

Done.

https://codereview.chromium.org/1160133002/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_utils.h:26: static void
NoOpClientInfoCallback(const ServiceWorkerClientInfo&) {}
On 2015/06/01 05:04:09, nhiroki wrote:
> Can you move this into service_worker_context_wrapper.cc?
> 
> (NoOpStatusCallback is widely used among SW components, but this is not)

Done.

[horo, 2015-06-02 03:15:20]

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

[horo, 2015-06-02 03:15:20]

pfeldman@

Could you please review
content/browser/devtools/protocol/service_worker_handler.*?

[horo, 2015-06-05 02:34:54]

Patchset #3 (id:40001) has been deleted

[horo, 2015-06-05 05:52:31]

Patchset #3 (id:60001) has been deleted

[pfeldman, 2015-06-05 12:19:54]

https://codereview.chromium.org/1160133002/diff/80001/content/browser/devtool...
File content/browser/devtools/protocol/service_worker_handler.cc (right):

https://codereview.chromium.org/1160133002/diff/80001/content/browser/devtool...
content/browser/devtools/protocol/service_worker_handler.cc:402: return
Response::InternalError("Not implemented yet");
Why do you need command_id?

https://codereview.chromium.org/1160133002/diff/80001/content/browser/devtool...
content/browser/devtools/protocol/service_worker_handler.cc:408: if
(!agent_host) {
drop {}

[horo, 2015-06-08 02:21:58]

Patchset #4 (id:100001) has been deleted

[horo, 2015-06-08 03:21:53]

https://codereview.chromium.org/1160133002/diff/80001/content/browser/devtool...
File content/browser/devtools/protocol/service_worker_handler.cc (right):

https://codereview.chromium.org/1160133002/diff/80001/content/browser/devtool...
content/browser/devtools/protocol/service_worker_handler.cc:402: return
Response::InternalError("Not implemented yet");
On 2015/06/05 12:19:54, pfeldman wrote:
> Why do you need command_id?

"getTargetInfo" doesn't need to be "async" command.
It should be "sync" command like "GetTargetInfo(const std::string& target_id,
scoped_refptr<TargetInfo>* target_info)".
But I can't use TargetInfo type in service_worker_handler.* before it will be
defined in protocol.json and be used in a command which handler is "browser".

So (1) this cl introduces a dummy "async" method in service_worker_handler.*,
and (2) https://codereview.chromium.org/1151993003/ defines TargetInfo and an
"async" command in protocol.json, and (3)
https://codereview.chromium.org/1149383004/ introduces "sync" command in
service_worker_handler.*, and (4) https://codereview.chromium.org/1164583002/
removes '"async": true' from protocol.json, and (5)
https://codereview.chromium.org/1143363009/ removes "async" method from
service_worker_handler.*.

https://codereview.chromium.org/1160133002/diff/80001/content/browser/devtool...
content/browser/devtools/protocol/service_worker_handler.cc:408: if
(!agent_host) {
On 2015/06/05 12:19:54, pfeldman wrote:
> drop {}

Done.

[horo, 2015-06-09 13:58:54]

pfeldman@
Ping?

[pfeldman, 2015-06-09 14:00:25]

lgtm

https://codereview.chromium.org/1160133002/diff/120001/content/browser/devtoo...
File content/browser/devtools/protocol/service_worker_handler.h (right):

https://codereview.chromium.org/1160133002/diff/120001/content/browser/devtoo...
content/browser/devtools/protocol/service_worker_handler.h:64: Response
FocusTarget(const std::string& target_id);
We should have called this Activate...

[horo, 2015-06-10 02:24:05]

Thank you!

https://codereview.chromium.org/1160133002/diff/120001/content/browser/devtoo...
File content/browser/devtools/protocol/service_worker_handler.h (right):

https://codereview.chromium.org/1160133002/diff/120001/content/browser/devtoo...
content/browser/devtools/protocol/service_worker_handler.h:64: Response
FocusTarget(const std::string& target_id);
On 2015/06/09 14:00:25, pfeldman wrote:
> We should have called this Activate...

Done.

[horo, 2015-06-10 02:24:10]

The CQ bit was checked by horo@chromium.org

[horo, 2015-06-10 02:24:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
nhiroki@chromium.org, pfeldman@chromium.org
Link to the patchset: https://codereview.chromium.org/1160133002/#ps140001
(title: "s/Focus/Activate/")

[commit-bot: I haz the power, 2015-06-10 02:25:10]

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

[horo, 2015-06-10 02:52:45]

The CQ bit was unchecked by horo@chromium.org

[horo, 2015-06-10 03:10:15]

The CQ bit was checked by horo@chromium.org

[commit-bot: I haz the power, 2015-06-10 03:10:57]

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

[commit-bot: I haz the power, 2015-06-10 03:14:18]

Committed patchset #5 (id:140001)

[commit-bot: I haz the power, 2015-06-10 03:16:14]

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