DevTools: Added service workers to remote debugging targets

DevTools: Added service workers to remote debugging targets

This one contains the required API changes to include service workers in remote debugging.
For now service workers will show up as "untitled" as they do not have a title to speak of.

Split off https://codereview.chromium.org/349033009/

BUG=389454
TBR= mnaganov@chromium.org (android_webview/native/aw_dev_tools_server.cc)

Committed: https://crrev.com/e1133eb041af5ae5fa9587fc72e315ab7ae8caaf
Cr-Commit-Position: refs/heads/master@{#291676}

[yurys, 2014-08-12 12:14:45]

As agreed offline let's add GetType(web contents, shared worker, service
worker), GetTitle, GetUrl, Close and Activate methods to DevToolsAgentHost.
After that we should be able to create targets having a DTAH. Target enumeration
will be simple wrapping of all DTAHs into targets.

https://codereview.chromium.org/459403002/diff/20001/chrome/browser/devtools/...
File chrome/browser/devtools/devtools_target_impl.cc (right):

https://codereview.chromium.org/459403002/diff/20001/chrome/browser/devtools/...
chrome/browser/devtools/devtools_target_impl.cc:373: for
(DevToolsAgentHost::List::iterator it = agents.begin();
Is it possible to use something like
ServiceWorkerService::GetInstance()->GetWorkers() to get all service workers
instead of relying on the fact that everything except workers and webcontents is
service workers?

[vkuzkokov, 2014-08-13 06:35:19]

Added methods to DTAH and used them in embedders.

https://codereview.chromium.org/459403002/diff/20001/chrome/browser/devtools/...
File chrome/browser/devtools/devtools_target_impl.cc (right):

https://codereview.chromium.org/459403002/diff/20001/chrome/browser/devtools/...
chrome/browser/devtools/devtools_target_impl.cc:373: for
(DevToolsAgentHost::List::iterator it = agents.begin();
On 2014/08/12 12:14:45, yurys wrote:
> Is it possible to use something like
> ServiceWorkerService::GetInstance()->GetWorkers() to get all service workers
> instead of relying on the fact that everything except workers and webcontents
is
> service workers?

Newly added DTAH::GetType() used instead.

[yurys, 2014-08-13 07:33:47]

https://codereview.chromium.org/459403002/diff/80001/chrome/browser/devtools/...
File chrome/browser/devtools/devtools_target_impl.cc (right):

https://codereview.chromium.org/459403002/diff/80001/chrome/browser/devtools/...
chrome/browser/devtools/devtools_target_impl.cc:328: void
DevToolsTargetImpl::EnumerateWorkerTargets(Callback callback) {
We don't need this method anymore.

https://codereview.chromium.org/459403002/diff/80001/content/browser/devtools...
File content/browser/devtools/embedded_worker_devtools_agent_host.cc (right):

https://codereview.chromium.org/459403002/diff/80001/content/browser/devtools...
content/browser/devtools/embedded_worker_devtools_agent_host.cc:46: return "";
 shared_worker_ has both name and url so you should be able to retrieve them.
Also it might make sense to have to descendants for service and shared worker to
avoid repetitive ifs.

https://codereview.chromium.org/459403002/diff/80001/content/browser/devtools...
content/browser/devtools/embedded_worker_devtools_agent_host.cc:58: return
false;
I guess we can easily implement this method.

https://codereview.chromium.org/459403002/diff/80001/content/public/browser/d...
File content/public/browser/devtools_agent_host.h (right):

https://codereview.chromium.org/459403002/diff/80001/content/public/browser/d...
content/public/browser/devtools_agent_host.h:22: extern const char
kAgentHostTypePage[];
Let's make it an enum.

https://codereview.chromium.org/459403002/diff/80001/content/public/browser/d...
content/public/browser/devtools_agent_host.h:98: static void DetachAllClients();
We shouldn't be detaching clients that we don't control. On the other hand,
having a reference to DTAH we should be able to close/detach client from it
without this method.

https://codereview.chromium.org/459403002/diff/80001/content/public/browser/d...
content/public/browser/devtools_agent_host.h:103: static void
AddAgentStateCallback(const AgentStateCallback& callback);
I don't think it should be a part of this CL.

[vkuzkokov, 2014-08-13 08:21:06]

https://codereview.chromium.org/459403002/diff/80001/chrome/browser/devtools/...
File chrome/browser/devtools/devtools_target_impl.cc (right):

https://codereview.chromium.org/459403002/diff/80001/chrome/browser/devtools/...
chrome/browser/devtools/devtools_target_impl.cc:328: void
DevToolsTargetImpl::EnumerateWorkerTargets(Callback callback) {
On 2014/08/13 07:33:46, yurys wrote:
> We don't need this method anymore.

We do.
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/dev...

https://codereview.chromium.org/459403002/diff/80001/content/browser/devtools...
File content/browser/devtools/embedded_worker_devtools_agent_host.cc (right):

https://codereview.chromium.org/459403002/diff/80001/content/browser/devtools...
content/browser/devtools/embedded_worker_devtools_agent_host.cc:46: return "";
On 2014/08/13 07:33:46, yurys wrote:
>  shared_worker_ has both name and url so you should be able to retrieve them.
> Also it might make sense to have to descendants for service and shared worker
to
> avoid repetitive ifs.

Implemented GetTitle() and GetURL() for shared workers.
I'd prefer a separate patch for splitting EWDTAH into 2 implementations.

https://codereview.chromium.org/459403002/diff/80001/content/browser/devtools...
content/browser/devtools/embedded_worker_devtools_agent_host.cc:58: return
false;
On 2014/08/13 07:33:46, yurys wrote:
> I guess we can easily implement this method.

Easily - for shared workers. For service workers it requires modifications in
EmbeddedWorkerInstance (as seen in crrev.com/349033009)

https://codereview.chromium.org/459403002/diff/80001/content/public/browser/d...
File content/public/browser/devtools_agent_host.h (right):

https://codereview.chromium.org/459403002/diff/80001/content/public/browser/d...
content/public/browser/devtools_agent_host.h:22: extern const char
kAgentHostTypePage[];
On 2014/08/13 07:33:47, yurys wrote:
> Let's make it an enum.

We still need string constants to serialize these types and it doesn't make much
sense to repeat them in every embedder.

https://codereview.chromium.org/459403002/diff/80001/content/public/browser/d...
content/public/browser/devtools_agent_host.h:98: static void DetachAllClients();
On 2014/08/13 07:33:47, yurys wrote:
> We shouldn't be detaching clients that we don't control. On the other hand,
> having a reference to DTAH we should be able to close/detach client from it
> without this method.

It's part of an unsuccessful rebase. Removed.

https://codereview.chromium.org/459403002/diff/80001/content/public/browser/d...
content/public/browser/devtools_agent_host.h:103: static void
AddAgentStateCallback(const AgentStateCallback& callback);
On 2014/08/13 07:33:47, yurys wrote:
> I don't think it should be a part of this CL.

Ditto.

[yurys, 2014-08-13 12:34:55]

lgtm given that the problem with string constants in content/ namespace is
resolved.

https://codereview.chromium.org/459403002/diff/100001/content/public/browser/...
File content/public/browser/devtools_agent_host.h (right):

https://codereview.chromium.org/459403002/diff/100001/content/public/browser/...
content/public/browser/devtools_agent_host.h:21: extern const char
kAgentHostTypePage[];
These should be moved inside DevToolsAgentHost or some namespace.

[vkuzkokov, 2014-08-13 13:31:54]

https://codereview.chromium.org/459403002/diff/100001/content/public/browser/...
File content/public/browser/devtools_agent_host.h (right):

https://codereview.chromium.org/459403002/diff/100001/content/public/browser/...
content/public/browser/devtools_agent_host.h:21: extern const char
kAgentHostTypePage[];
On 2014/08/13 12:34:55, yurys wrote:
> These should be moved inside DevToolsAgentHost or some namespace.

Done.

[dgozman, 2014-08-13 15:57:03]

https://codereview.chromium.org/459403002/diff/120001/content/browser/devtool...
File content/browser/devtools/embedded_worker_devtools_agent_host.cc (right):

https://codereview.chromium.org/459403002/diff/120001/content/browser/devtool...
content/browser/devtools/embedded_worker_devtools_agent_host.cc:52: return
shared_worker_ ? shared_worker_->url() : GURL();
Can we get URL for shared worker?

https://codereview.chromium.org/459403002/diff/120001/content/browser/devtool...
content/browser/devtools/embedded_worker_devtools_agent_host.cc:64: return
false;
Why can't we close shared worker?

https://codereview.chromium.org/459403002/diff/120001/content/browser/devtool...
File content/browser/devtools/render_view_devtools_agent_host.cc (right):

https://codereview.chromium.org/459403002/diff/120001/content/browser/devtool...
content/browser/devtools/render_view_devtools_agent_host.cc:401:
web_contents->GetRenderViewHost()->ClosePage();
Use |render_view_host_| instead, this call may return wrong RVH.

https://codereview.chromium.org/459403002/diff/120001/content/public/browser/...
File content/public/browser/devtools_agent_host.h (right):

https://codereview.chromium.org/459403002/diff/120001/content/public/browser/...
content/public/browser/devtools_agent_host.h:61: // Returns all possible
DevToolsAgentHosts
nit: full stop please

[vkuzkokov, 2014-08-14 07:48:17]

https://codereview.chromium.org/459403002/diff/120001/content/browser/devtool...
File content/browser/devtools/embedded_worker_devtools_agent_host.cc (right):

https://codereview.chromium.org/459403002/diff/120001/content/browser/devtool...
content/browser/devtools/embedded_worker_devtools_agent_host.cc:52: return
shared_worker_ ? shared_worker_->url() : GURL();
On 2014/08/13 15:57:02, dgozman wrote:
> Can we get URL for shared worker?

Service worker, you mean? It requires changes in EmbeddedWorkerInstance which
belong in another CL.

https://codereview.chromium.org/459403002/diff/120001/content/browser/devtool...
content/browser/devtools/embedded_worker_devtools_agent_host.cc:64: return
false;
On 2014/08/13 15:57:02, dgozman wrote:
> Why can't we close shared worker?

Ditto.

https://codereview.chromium.org/459403002/diff/120001/content/browser/devtool...
File content/browser/devtools/render_view_devtools_agent_host.cc (right):

https://codereview.chromium.org/459403002/diff/120001/content/browser/devtool...
content/browser/devtools/render_view_devtools_agent_host.cc:401:
web_contents->GetRenderViewHost()->ClosePage();
On 2014/08/13 15:57:02, dgozman wrote:
> Use |render_view_host_| instead, this call may return wrong RVH.

Done.

https://codereview.chromium.org/459403002/diff/120001/content/public/browser/...
File content/public/browser/devtools_agent_host.h (right):

https://codereview.chromium.org/459403002/diff/120001/content/public/browser/...
content/public/browser/devtools_agent_host.h:61: // Returns all possible
DevToolsAgentHosts
On 2014/08/13 15:57:02, dgozman wrote:
> nit: full stop please

Done.

[vkuzkokov, 2014-08-14 07:50:53]

jam, PTAL at:
chrome/browser/android/dev_tools_server.cc
content/public/browser/devtools_agent_host.h

pfeldman, PTAL at:
content/shell/browser/shell_devtools_delegate.cc

[dgozman, 2014-08-14 08:15:33]

https://codereview.chromium.org/459403002/diff/120001/content/browser/devtool...
File content/browser/devtools/embedded_worker_devtools_agent_host.cc (right):

https://codereview.chromium.org/459403002/diff/120001/content/browser/devtool...
content/browser/devtools/embedded_worker_devtools_agent_host.cc:52: return
shared_worker_ ? shared_worker_->url() : GURL();
On 2014/08/14 07:48:17, vkuzkokov wrote:
> On 2014/08/13 15:57:02, dgozman wrote:
> > Can we get URL for shared worker?
> 
> Service worker, you mean? It requires changes in EmbeddedWorkerInstance which
> belong in another CL.

Can we make those changes beforehand? Otherwise you will get disfunctional Close
button as well as no way to differentiate between service workers. I think we
should not expose not yet ready service workers then.

[vkuzkokov, 2014-08-14 12:43:57]

On 2014/08/14 08:15:33, dgozman wrote:
>
https://codereview.chromium.org/459403002/diff/120001/content/browser/devtool...
> File content/browser/devtools/embedded_worker_devtools_agent_host.cc (right):
> 
>
https://codereview.chromium.org/459403002/diff/120001/content/browser/devtool...
> content/browser/devtools/embedded_worker_devtools_agent_host.cc:52: return
> shared_worker_ ? shared_worker_->url() : GURL();
> On 2014/08/14 07:48:17, vkuzkokov wrote:
> > On 2014/08/13 15:57:02, dgozman wrote:
> > > Can we get URL for shared worker?
> > 
> > Service worker, you mean? It requires changes in EmbeddedWorkerInstance
which
> > belong in another CL.
> 
> Can we make those changes beforehand? Otherwise you will get disfunctional
Close
> button as well as no way to differentiate between service workers. I think we
> should not expose not yet ready service workers then.

OK, let's land https://codereview.chromium.org/470923004/ first.

[jam, 2014-08-15 17:35:46]

https://codereview.chromium.org/459403002/diff/140001/content/public/browser/...
File content/public/browser/devtools_agent_host.h (right):

https://codereview.chromium.org/459403002/diff/140001/content/public/browser/...
content/public/browser/devtools_agent_host.h:32: static const char
kTypeServiceWorker[];
why aren't these an enum?

[vkuzkokov, 2014-08-18 14:08:01]

On 2014/08/15 17:35:46, jam wrote:
>
https://codereview.chromium.org/459403002/diff/140001/content/public/browser/...
> File content/public/browser/devtools_agent_host.h (right):
> 
>
https://codereview.chromium.org/459403002/diff/140001/content/public/browser/...
> content/public/browser/devtools_agent_host.h:32: static const char
> kTypeServiceWorker[];
> why aren't these an enum?

We need to serialize these values, eventually.
That means a switch in DevToolsTarget implementation for every embedder.
Or we can return a string straight away.

[jam, 2014-08-18 15:57:57]

On 2014/08/18 14:08:01, vkuzkokov wrote:
> On 2014/08/15 17:35:46, jam wrote:
> >
>
https://codereview.chromium.org/459403002/diff/140001/content/public/browser/...
> > File content/public/browser/devtools_agent_host.h (right):
> > 
> >
>
https://codereview.chromium.org/459403002/diff/140001/content/public/browser/...
> > content/public/browser/devtools_agent_host.h:32: static const char
> > kTypeServiceWorker[];
> > why aren't these an enum?
> 
> We need to serialize these values, eventually.
> That means a switch in DevToolsTarget implementation for every embedder.
> Or we can return a string straight away.

which code specifically will serialize? is it just in content? or if it's in
chrome, why does every embedder need to serialize this?

[vkuzkokov, 2014-08-18 16:22:02]

On 2014/08/18 15:57:57, jam wrote:
> On 2014/08/18 14:08:01, vkuzkokov wrote:
> > On 2014/08/15 17:35:46, jam wrote:
> > >
> >
>
https://codereview.chromium.org/459403002/diff/140001/content/public/browser/...
> > > File content/public/browser/devtools_agent_host.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/459403002/diff/140001/content/public/browser/...
> > > content/public/browser/devtools_agent_host.h:32: static const char
> > > kTypeServiceWorker[];
> > > why aren't these an enum?
> > 
> > We need to serialize these values, eventually.
> > That means a switch in DevToolsTarget implementation for every embedder.
> > Or we can return a string straight away.
> 
> which code specifically will serialize? is it just in content? or if it's in
> chrome, why does every embedder need to serialize this?

Specific code that serializes: DevToolsHttpHandlerImpl::SerializeTarget. It uses
DevToolsTarget's provided by embedder.
Content is not aware of all values possible for DevToolsTarget::GetType in
desktop chrome (like "app" and "background_page") so it's a string.
If we make DevToolsAgentHost::GetType return enum we need to have string
constants for DevToolsTarget::GetType implementaion in every embedder.

[vkuzkokov, 2014-08-20 13:25:10]

Rebased. Now service workers are displayed with URLs and can be remotely closed.

[dgozman, 2014-08-21 11:50:25]

https://codereview.chromium.org/459403002/diff/200001/chrome/browser/devtools...
File chrome/browser/devtools/devtools_target_impl.cc (right):

https://codereview.chromium.org/459403002/diff/200001/chrome/browser/devtools...
chrome/browser/devtools/devtools_target_impl.cc:194: class ServiceWorkerTarget :
public DevToolsTargetImpl {
You should remove this class and update WorkerTarget instead.

https://codereview.chromium.org/459403002/diff/200001/chrome/browser/devtools...
chrome/browser/devtools/devtools_target_impl.cc:329: void
DevToolsTargetImpl::EnumerateWorkerTargets(Callback callback) {
We should remove this method and it's usages in a follow-up patch.

https://codereview.chromium.org/459403002/diff/200001/content/browser/devtool...
File content/browser/devtools/embedded_worker_devtools_agent_host.h (right):

https://codereview.chromium.org/459403002/diff/200001/content/browser/devtool...
content/browser/devtools/embedded_worker_devtools_agent_host.h:52: bool
IsAlive();
I'd rather use IsTerminated.

https://codereview.chromium.org/459403002/diff/200001/content/browser/devtool...
File content/browser/devtools/forwarding_agent_host.cc (right):

https://codereview.chromium.org/459403002/diff/200001/content/browser/devtool...
content/browser/devtools/forwarding_agent_host.cc:41: return "";
You should return something meaningful.

https://codereview.chromium.org/459403002/diff/200001/content/browser/devtool...
File content/browser/devtools/render_view_devtools_agent_host.cc (right):

https://codereview.chromium.org/459403002/diff/200001/content/browser/devtool...
content/browser/devtools/render_view_devtools_agent_host.cc:387: if
(WebContents* web_contents = GetWebContents())
return render_view_host_ ?
render_view_host_->GetMainFrame()->GetLastCommittedURL() : GURL();

https://codereview.chromium.org/459403002/diff/200001/content/browser/devtool...
content/browser/devtools/render_view_devtools_agent_host.cc:393: if
(WebContents* web_contents = GetWebContents()) {
if (render_view_host_) {
  render_view_host_->GetDelegate()->Activate();
  return true;
}

https://codereview.chromium.org/459403002/diff/200001/content/public/browser/...
File content/public/browser/devtools_agent_host.h (right):

https://codereview.chromium.org/459403002/diff/200001/content/public/browser/...
content/public/browser/devtools_agent_host.h:28: static const char
kTypeWebContents[];
I think we should use enum and let embedder serialize it.

https://codereview.chromium.org/459403002/diff/200001/content/public/browser/...
content/public/browser/devtools_agent_host.h:31: static const char
kTypeWorker[];
kTypeSharedWorker

[vkuzkokov, 2014-08-21 14:17:53]

https://codereview.chromium.org/459403002/diff/200001/chrome/browser/devtools...
File chrome/browser/devtools/devtools_target_impl.cc (right):

https://codereview.chromium.org/459403002/diff/200001/chrome/browser/devtools...
chrome/browser/devtools/devtools_target_impl.cc:194: class ServiceWorkerTarget :
public DevToolsTargetImpl {
On 2014/08/21 11:50:24, dgozman wrote:
> You should remove this class and update WorkerTarget instead.

Done.

https://codereview.chromium.org/459403002/diff/200001/chrome/browser/devtools...
chrome/browser/devtools/devtools_target_impl.cc:329: void
DevToolsTargetImpl::EnumerateWorkerTargets(Callback callback) {
On 2014/08/21 11:50:24, dgozman wrote:
> We should remove this method and it's usages in a follow-up patch.

Acknowledged.

https://codereview.chromium.org/459403002/diff/200001/content/browser/devtool...
File content/browser/devtools/embedded_worker_devtools_agent_host.h (right):

https://codereview.chromium.org/459403002/diff/200001/content/browser/devtool...
content/browser/devtools/embedded_worker_devtools_agent_host.h:52: bool
IsAlive();
On 2014/08/21 11:50:24, dgozman wrote:
> I'd rather use IsTerminated.

Done.

https://codereview.chromium.org/459403002/diff/200001/content/browser/devtool...
File content/browser/devtools/forwarding_agent_host.cc (right):

https://codereview.chromium.org/459403002/diff/200001/content/browser/devtool...
content/browser/devtools/forwarding_agent_host.cc:41: return "";
On 2014/08/21 11:50:24, dgozman wrote:
> You should return something meaningful.

Done.

https://codereview.chromium.org/459403002/diff/200001/content/browser/devtool...
File content/browser/devtools/render_view_devtools_agent_host.cc (right):

https://codereview.chromium.org/459403002/diff/200001/content/browser/devtool...
content/browser/devtools/render_view_devtools_agent_host.cc:387: if
(WebContents* web_contents = GetWebContents())
On 2014/08/21 11:50:24, dgozman wrote:
> return render_view_host_ ?
> render_view_host_->GetMainFrame()->GetLastCommittedURL() : GURL();

Done.

https://codereview.chromium.org/459403002/diff/200001/content/browser/devtool...
content/browser/devtools/render_view_devtools_agent_host.cc:393: if
(WebContents* web_contents = GetWebContents()) {
On 2014/08/21 11:50:24, dgozman wrote:
> if (render_view_host_) {
>   render_view_host_->GetDelegate()->Activate();
>   return true;
> }

Done.

https://codereview.chromium.org/459403002/diff/200001/content/public/browser/...
File content/public/browser/devtools_agent_host.h (right):

https://codereview.chromium.org/459403002/diff/200001/content/public/browser/...
content/public/browser/devtools_agent_host.h:28: static const char
kTypeWebContents[];
On 2014/08/21 11:50:24, dgozman wrote:
> I think we should use enum and let embedder serialize it.

Done.

https://codereview.chromium.org/459403002/diff/200001/content/public/browser/...
content/public/browser/devtools_agent_host.h:31: static const char
kTypeWorker[];
On 2014/08/21 11:50:24, dgozman wrote:
> kTypeSharedWorker

Done.

[jam, 2014-08-21 17:35:20]

lgtm

sorry for the delay, just saw this update

[dgozman, 2014-08-22 08:38:28]

lgtm

https://codereview.chromium.org/459403002/diff/220001/android_webview/native/...
File android_webview/native/aw_dev_tools_server.cc (right):

https://codereview.chromium.org/459403002/diff/220001/android_webview/native/...
android_webview/native/aw_dev_tools_server.cc:53: {} // Do nothing.
break;

https://codereview.chromium.org/459403002/diff/220001/chrome/browser/android/...
File chrome/browser/android/dev_tools_server.cc (left):

https://codereview.chromium.org/459403002/diff/220001/chrome/browser/android/...
chrome/browser/android/dev_tools_server.cc:234: if (TabModelList::begin() ==
TabModelList::end()) {
Please don't regress this.

https://codereview.chromium.org/459403002/diff/220001/chrome/browser/devtools...
File chrome/browser/devtools/devtools_target_impl.cc (left):

https://codereview.chromium.org/459403002/diff/220001/chrome/browser/devtools...
chrome/browser/devtools/devtools_target_impl.cc:190:
set_description(base::StringPrintf("Worker pid:%d",
Leave set_description in this constructor.

https://codereview.chromium.org/459403002/diff/220001/content/browser/devtool...
File content/browser/devtools/embedded_worker_devtools_agent_host.cc (right):

https://codereview.chromium.org/459403002/diff/220001/content/browser/devtool...
content/browser/devtools/embedded_worker_devtools_agent_host.cc:174:
HostClosed();
This should go to a separate patch.

https://codereview.chromium.org/459403002/diff/220001/content/public/browser/...
File content/public/browser/devtools_agent_host.h (right):

https://codereview.chromium.org/459403002/diff/220001/content/public/browser/...
content/public/browser/devtools_agent_host.h:29: AGENT_HOST_WEB_CONTENTS,
I'd say this should be TYPE_WEB_CONTENTS.

[vkuzkokov, 2014-08-22 11:01:48]

https://codereview.chromium.org/459403002/diff/220001/android_webview/native/...
File android_webview/native/aw_dev_tools_server.cc (right):

https://codereview.chromium.org/459403002/diff/220001/android_webview/native/...
android_webview/native/aw_dev_tools_server.cc:53: {} // Do nothing.
On 2014/08/22 08:38:28, dgozman wrote:
> break;

Done.

https://codereview.chromium.org/459403002/diff/220001/chrome/browser/android/...
File chrome/browser/android/dev_tools_server.cc (left):

https://codereview.chromium.org/459403002/diff/220001/chrome/browser/android/...
chrome/browser/android/dev_tools_server.cc:234: if (TabModelList::begin() ==
TabModelList::end()) {
On 2014/08/22 08:38:28, dgozman wrote:
> Please don't regress this.

Done.

https://codereview.chromium.org/459403002/diff/220001/chrome/browser/devtools...
File chrome/browser/devtools/devtools_target_impl.cc (left):

https://codereview.chromium.org/459403002/diff/220001/chrome/browser/devtools...
chrome/browser/devtools/devtools_target_impl.cc:190:
set_description(base::StringPrintf("Worker pid:%d",
On 2014/08/22 08:38:28, dgozman wrote:
> Leave set_description in this constructor.

Done.

https://codereview.chromium.org/459403002/diff/220001/content/browser/devtool...
File content/browser/devtools/embedded_worker_devtools_agent_host.cc (right):

https://codereview.chromium.org/459403002/diff/220001/content/browser/devtool...
content/browser/devtools/embedded_worker_devtools_agent_host.cc:174:
HostClosed();
On 2014/08/22 08:38:28, dgozman wrote:
> This should go to a separate patch.

Done.

https://codereview.chromium.org/459403002/diff/220001/content/public/browser/...
File content/public/browser/devtools_agent_host.h (right):

https://codereview.chromium.org/459403002/diff/220001/content/public/browser/...
content/public/browser/devtools_agent_host.h:29: AGENT_HOST_WEB_CONTENTS,
On 2014/08/22 08:38:28, dgozman wrote:
> I'd say this should be TYPE_WEB_CONTENTS.

Done.

[vkuzkokov, 2014-08-22 11:03:32]

The CQ bit was checked by vkuzkokov@chromium.org

[commit-bot: I haz the power, 2014-08-22 11:04:09]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vkuzkokov@chromium.org/459403002/240001

[vkuzkokov, 2014-08-22 11:10:32]

The CQ bit was unchecked by vkuzkokov@chromium.org

[vkuzkokov, 2014-08-22 11:12:09]

The CQ bit was checked by vkuzkokov@chromium.org

[commit-bot: I haz the power, 2014-08-22 11:13:12]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vkuzkokov@chromium.org/459403002/260001

[commit-bot: I haz the power, 2014-08-22 12:39:31]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-22 12:46:58]

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

[commit-bot: I haz the power, 2014-08-22 12:46:59]

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

[vkuzkokov, 2014-08-25 11:59:27]

The CQ bit was checked by vkuzkokov@chromium.org

[commit-bot: I haz the power, 2014-08-25 11:59:48]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vkuzkokov@chromium.org/459403002/300001

[commit-bot: I haz the power, 2014-08-25 13:03:38]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[commit-bot: I haz the power, 2014-08-25 13:33:40]

Committed patchset #16 (300001) as 29eb413f7ce65210c9855442a80474b9a673d940

[commit-bot: I haz the power, 2014-09-10 02:34:19]

Patchset 16 (id:??) landed as
https://crrev.com/e1133eb041af5ae5fa9587fc72e315ab7ae8caaf
Cr-Commit-Position: refs/heads/master@{#291676}