DevTools: land service worker handler (patch 1).

DevTools: land service worker handler (patch 1).

BUG=463892
Committed: https://crrev.com/6c5f620da30cbc976f25757f9922c137a68a30e8
Cr-Commit-Position: refs/heads/master@{#319330}

[pfeldman, 2015-03-05 16:05:46]

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

[pfeldman, 2015-03-05 17:07:53]

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

[dgozman, 2015-03-05 20:17:06]

lgtm modulo comments

https://codereview.chromium.org/976353002/diff/40001/content/browser/devtools...
File content/browser/devtools/protocol/service_worker_handler.cc (right):

https://codereview.chromium.org/976353002/diff/40001/content/browser/devtools...
content/browser/devtools/protocol/service_worker_handler.cc:29: Disable();
This may lead to double |RemoveObserver|. Is it safe?

https://codereview.chromium.org/976353002/diff/40001/content/browser/devtools...
content/browser/devtools/protocol/service_worker_handler.cc:100:
attached_hosts_.erase(agent_host->GetId());
DCHECK it's there

https://codereview.chromium.org/976353002/diff/40001/content/browser/devtools...
content/browser/devtools/protocol/service_worker_handler.cc:101:
WorkerDestroyed(agent_host);
Won't this lead to double WorkerDestroyed notification?

https://codereview.chromium.org/976353002/diff/40001/content/browser/devtools...
content/browser/devtools/protocol/service_worker_handler.cc:119: }  // namespace
worker
service_worker

https://codereview.chromium.org/976353002/diff/40001/content/browser/devtools...
File content/browser/devtools/protocol/service_worker_handler.h (right):

https://codereview.chromium.org/976353002/diff/40001/content/browser/devtools...
content/browser/devtools/protocol/service_worker_handler.h:11: 
nit: remove blank line

https://codereview.chromium.org/976353002/diff/40001/content/browser/devtools...
content/browser/devtools/protocol/service_worker_handler.h:31: // Protocol
'worker domain' implementation.
worker -> service worker

https://codereview.chromium.org/976353002/diff/40001/content/browser/devtools...
content/browser/devtools/protocol/service_worker_handler.h:44: //
DevToolsProtocolClient overrides.
DevToolsAgentHostClient

https://codereview.chromium.org/976353002/diff/40001/content/browser/devtools...
content/browser/devtools/protocol/service_worker_handler.h:58: }  // namespace
worker
service_worker

[pfeldman, 2015-03-05 20:26:26]

https://codereview.chromium.org/976353002/diff/40001/content/browser/devtools...
File content/browser/devtools/protocol/service_worker_handler.cc (right):

https://codereview.chromium.org/976353002/diff/40001/content/browser/devtools...
content/browser/devtools/protocol/service_worker_handler.cc:29: Disable();
On 2015/03/05 20:17:06, dgozman wrote:
> This may lead to double |RemoveObserver|. Is it safe?

It will be. I'll clear attached_hosts_ in Disable as I should have.

https://codereview.chromium.org/976353002/diff/40001/content/browser/devtools...
content/browser/devtools/protocol/service_worker_handler.cc:100:
attached_hosts_.erase(agent_host->GetId());
On 2015/03/05 20:17:06, dgozman wrote:
> DCHECK it's there

Done.

https://codereview.chromium.org/976353002/diff/40001/content/browser/devtools...
content/browser/devtools/protocol/service_worker_handler.cc:101:
WorkerDestroyed(agent_host);
On 2015/03/05 20:17:06, dgozman wrote:
> Won't this lead to double WorkerDestroyed notification?

It will! I'll delegate there and make sure it is only called once.

https://codereview.chromium.org/976353002/diff/40001/content/browser/devtools...
content/browser/devtools/protocol/service_worker_handler.cc:101:
WorkerDestroyed(agent_host);
On 2015/03/05 20:17:06, dgozman wrote:
> Won't this lead to double WorkerDestroyed notification?

Done.

https://codereview.chromium.org/976353002/diff/40001/content/browser/devtools...
content/browser/devtools/protocol/service_worker_handler.cc:119: }  // namespace
worker
On 2015/03/05 20:17:06, dgozman wrote:
> service_worker

Done.

https://codereview.chromium.org/976353002/diff/40001/content/browser/devtools...
File content/browser/devtools/protocol/service_worker_handler.h (right):

https://codereview.chromium.org/976353002/diff/40001/content/browser/devtools...
content/browser/devtools/protocol/service_worker_handler.h:11: 
On 2015/03/05 20:17:06, dgozman wrote:
> nit: remove blank line

Done.

https://codereview.chromium.org/976353002/diff/40001/content/browser/devtools...
content/browser/devtools/protocol/service_worker_handler.h:31: // Protocol
'worker domain' implementation.
On 2015/03/05 20:17:06, dgozman wrote:
> worker -> service worker

Done.

https://codereview.chromium.org/976353002/diff/40001/content/browser/devtools...
content/browser/devtools/protocol/service_worker_handler.h:44: //
DevToolsProtocolClient overrides.
On 2015/03/05 20:17:06, dgozman wrote:
> DevToolsAgentHostClient

Done.

https://codereview.chromium.org/976353002/diff/40001/content/browser/devtools...
content/browser/devtools/protocol/service_worker_handler.h:58: }  // namespace
worker
On 2015/03/05 20:17:06, dgozman wrote:
> service_worker

Done.

[pfeldman, 2015-03-05 20:33:37]

The CQ bit was checked by pfeldman@chromium.org

[pfeldman, 2015-03-05 20:33:37]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/976353002/#ps60001 (title:
"review comments addressed")

[commit-bot: I haz the power, 2015-03-05 20:35:02]

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

[commit-bot: I haz the power, 2015-03-05 21:28:01]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-03-05 21:28:34]

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