DevTools: pick sw targets to attach to on the backend side.

DevTools: pick sw targets to attach to on the backend side.

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

[pfeldman, 2015-03-07 17:47:37]

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

[dgozman, 2015-03-08 07:49:56]

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

https://codereview.chromium.org/988003002/diff/40001/content/browser/devtools...
content/browser/devtools/protocol/service_worker_handler.cc:36:
WorkerReadyForInspection(host.get());
You should also detach from the old ones, which are not relevant anymore.

https://codereview.chromium.org/988003002/diff/40001/content/browser/devtools...
File content/browser/devtools/render_frame_devtools_agent_host.cc (right):

https://codereview.chromium.org/988003002/diff/40001/content/browser/devtools...
content/browser/devtools/render_frame_devtools_agent_host.cc:395: void
RenderFrameDevToolsAgentHost::NavigationEntryCommitted(
This method is supposed to do the same url invalidation. Combine them.

https://codereview.chromium.org/988003002/diff/40001/content/browser/devtools...
File content/browser/devtools/service_worker_devtools_manager.cc (right):

https://codereview.chromium.org/988003002/diff/40001/content/browser/devtools...
content/browser/devtools/service_worker_devtools_manager.cc:111:
it->second->Inspect(RenderProcessHost::FromID(worker_process_id)->
it->second   =>  host

https://codereview.chromium.org/988003002/diff/40001/content/browser/devtools...
File content/browser/devtools/worker_devtools_agent_host.cc (left):

https://codereview.chromium.org/988003002/diff/40001/content/browser/devtools...
content/browser/devtools/worker_devtools_agent_host.cc:71:
Inspect(rph->GetBrowserContext());
This seems to regress shared workers.

https://codereview.chromium.org/988003002/diff/40001/content/browser/devtools...
File content/browser/devtools/worker_devtools_agent_host.cc (right):

https://codereview.chromium.org/988003002/diff/40001/content/browser/devtools...
content/browser/devtools/worker_devtools_agent_host.cc:69: state_ =
WORKER_PAUSED_FOR_DEBUG_ON_START;
DCHECK that state is WORKER_TERMINATED or WORKER_UNINSPECTED.

[pfeldman, 2015-03-08 08:30:58]

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

https://codereview.chromium.org/988003002/diff/40001/content/browser/devtools...
content/browser/devtools/protocol/service_worker_handler.cc:36:
WorkerReadyForInspection(host.get());
On 2015/03/08 07:49:56, dgozman wrote:
> You should also detach from the old ones, which are not relevant anymore.

Done.

https://codereview.chromium.org/988003002/diff/40001/content/browser/devtools...
File content/browser/devtools/render_frame_devtools_agent_host.cc (right):

https://codereview.chromium.org/988003002/diff/40001/content/browser/devtools...
content/browser/devtools/render_frame_devtools_agent_host.cc:395: void
RenderFrameDevToolsAgentHost::NavigationEntryCommitted(
On 2015/03/08 07:49:56, dgozman wrote:
> This method is supposed to do the same url invalidation. Combine them.

I now think that I need the one below to fire even earlier. I'll be
experimenting with it, while this one will go once
DevToolsManager::GetInstance()->AgentHostChanged is removed.

https://codereview.chromium.org/988003002/diff/40001/content/browser/devtools...
File content/browser/devtools/service_worker_devtools_manager.cc (right):

https://codereview.chromium.org/988003002/diff/40001/content/browser/devtools...
content/browser/devtools/service_worker_devtools_manager.cc:111:
it->second->Inspect(RenderProcessHost::FromID(worker_process_id)->
On 2015/03/08 07:49:56, dgozman wrote:
> it->second   =>  host

Done.

https://codereview.chromium.org/988003002/diff/40001/content/browser/devtools...
File content/browser/devtools/worker_devtools_agent_host.cc (left):

https://codereview.chromium.org/988003002/diff/40001/content/browser/devtools...
content/browser/devtools/worker_devtools_agent_host.cc:71:
Inspect(rph->GetBrowserContext());
On 2015/03/08 07:49:56, dgozman wrote:
> This seems to regress shared workers.

This code only works for ServiceWorkers.

https://codereview.chromium.org/988003002/diff/40001/content/browser/devtools...
content/browser/devtools/worker_devtools_agent_host.cc:73: DCHECK(IsAttached());
This code only works for SharedWorkers.

We only "reattach" to shared workers,
We only "debug on start" service workers

That's why I am splitting them into different hosts as a follow up. Having said
that, I think that we need to align Shared to mimic Service behavior long term.

https://codereview.chromium.org/988003002/diff/40001/content/browser/devtools...
File content/browser/devtools/worker_devtools_agent_host.cc (right):

https://codereview.chromium.org/988003002/diff/40001/content/browser/devtools...
content/browser/devtools/worker_devtools_agent_host.cc:69: state_ =
WORKER_PAUSED_FOR_DEBUG_ON_START;
On 2015/03/08 07:49:56, dgozman wrote:
> DCHECK that state is WORKER_TERMINATED or WORKER_UNINSPECTED.

In fact, only UNINSPECTED.

[dgozman, 2015-03-08 15:27:08]

lgtm

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

https://codereview.chromium.org/988003002/diff/80001/content/browser/devtools...
content/browser/devtools/render_frame_devtools_agent_host.cc:405: if
(!render_frame_host->GetParent())
To account for OOPIF:
if (render_frame_host_ == render_frame_host)

https://codereview.chromium.org/988003002/diff/80001/content/browser/devtools...
File content/browser/devtools/shared_worker_devtools_manager.h (right):

https://codereview.chromium.org/988003002/diff/80001/content/browser/devtools...
content/browser/devtools/shared_worker_devtools_manager.h:33:
std::vector<scoped_refptr<SharedWorkerDevToolsAgentHost>>* result);
SharedWorkerDevToolsAgentHost::List

[pfeldman, 2015-03-08 15:34:15]

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

https://codereview.chromium.org/988003002/diff/80001/content/browser/devtools...
content/browser/devtools/render_frame_devtools_agent_host.cc:405: if
(!render_frame_host->GetParent())
On 2015/03/08 15:27:08, dgozman wrote:
> To account for OOPIF:
> if (render_frame_host_ == render_frame_host)

Done.

https://codereview.chromium.org/988003002/diff/80001/content/browser/devtools...
File content/browser/devtools/shared_worker_devtools_manager.h (right):

https://codereview.chromium.org/988003002/diff/80001/content/browser/devtools...
content/browser/devtools/shared_worker_devtools_manager.h:33:
std::vector<scoped_refptr<SharedWorkerDevToolsAgentHost>>* result);
On 2015/03/08 15:27:08, dgozman wrote:
> SharedWorkerDevToolsAgentHost::List

Can't do that, the .h dependency is the other way around.

[pfeldman, 2015-03-09 06:16:27]

The CQ bit was checked by pfeldman@chromium.org

[pfeldman, 2015-03-09 06:16:28]

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/988003002/#ps120001
(title: "same w/ basic stop handler.")

[commit-bot: I haz the power, 2015-03-09 06:17:00]

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

[commit-bot: I haz the power, 2015-03-09 06:40:52]

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

[commit-bot: I haz the power, 2015-03-09 06:40:53]

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

[pfeldman, 2015-03-09 14:04:13]

The CQ bit was checked by pfeldman@chromium.org

[commit-bot: I haz the power, 2015-03-09 14:04:37]

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

[commit-bot: I haz the power, 2015-03-09 14:09:23]

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

[commit-bot: I haz the power, 2015-03-09 14:09:24]

Try jobs failed on following builders:
  android_aosp on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)

[pfeldman, 2015-03-09 17:35:05]

The CQ bit was checked by pfeldman@chromium.org

[pfeldman, 2015-03-09 17:35:06]

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/988003002/#ps140001
(title: "rebaselined")

[commit-bot: I haz the power, 2015-03-09 17:35:30]

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

[commit-bot: I haz the power, 2015-03-09 18:26:49]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-03-09 18:27:42]

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