Add pause_on_start flag to WorkerProcessMsg_CreateWorker_Params.

Add pause_on_start flag to WorkerProcessMsg_CreateWorker_Params.

DevToolsAgentMsg_PauseWorkerContextOnStart was send to the worker process from the browser process in order to pause the worker context when it starts.
Before http://crbug.com/329786 this message was sent between WorkerProcessMsg_CreateWorker and WorkerMsg_StartWorkerContext.
So the worker was successfully paused before started.
But after http://crbug.com/329786 the worker context may be started before the worker process receives this message because WorkerMsg_StartWorkerContext is no longer sent from the browser process.
So we have to add pause_on_start flag to WorkerProcessMsg_CreateWorker_Params to pause the worker context correctly.

BUG=329786
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=260377

[horo, 2014-03-27 06:20:47]

kinuko@
Could you please review this?

[kinuko, 2014-03-27 08:04:34]

https://codereview.chromium.org/213423008/diff/1/content/browser/devtools/wor...
File content/browser/devtools/worker_devtools_manager.cc (right):

https://codereview.chromium.org/213423008/diff/1/content/browser/devtools/wor...
content/browser/devtools/worker_devtools_manager.cc:297: }
I think adding pause_on_start param in CreateWorker message makes sense, but
when we move DevToolsManager to solely on UI thread this simple sync method
needs to be handled asynchronously-- will we have an async method like void
TerminatedWorkerExist(const Callback<void(bool)>& callback) in the new
architecture then?

[yurys, 2014-03-27 08:37:25]

I like the idea of passing this "pause on start" flag as an argument to
CreateWorker message.

https://codereview.chromium.org/213423008/diff/1/content/browser/devtools/wor...
File content/browser/devtools/worker_devtools_manager.cc (right):

https://codereview.chromium.org/213423008/diff/1/content/browser/devtools/wor...
content/browser/devtools/worker_devtools_manager.cc:283: bool
WorkerDevToolsManager::TerminatedWorkerExist(
We could merge WorkerCreated into this new method to make it clear why the
worker is now in paused_workers_ map.

[horo, 2014-03-27 09:45:18]

On 2014/03/27 08:04:34, kinuko wrote:
>
https://codereview.chromium.org/213423008/diff/1/content/browser/devtools/wor...
> File content/browser/devtools/worker_devtools_manager.cc (right):
> 
>
https://codereview.chromium.org/213423008/diff/1/content/browser/devtools/wor...
> content/browser/devtools/worker_devtools_manager.cc:297: }
> I think adding pause_on_start param in CreateWorker message makes sense, but
> when we move DevToolsManager to solely on UI thread this simple sync method
> needs to be handled asynchronously-- will we have an async method like void
> TerminatedWorkerExist(const Callback<void(bool)>& callback) in the new
> architecture then?

Yes in the new architecture (http://crrev.com/196503005) we have to go to the UI
thread before sending WorkerProcessMsg_CreateWorker message to check the
existence of the terminated worker.

[horo, 2014-03-27 09:45:48]

https://codereview.chromium.org/213423008/diff/1/content/browser/devtools/wor...
File content/browser/devtools/worker_devtools_manager.cc (right):

https://codereview.chromium.org/213423008/diff/1/content/browser/devtools/wor...
content/browser/devtools/worker_devtools_manager.cc:297: }
On 2014/03/27 08:04:34, kinuko wrote:
> I think adding pause_on_start param in CreateWorker message makes sense, but
> when we move DevToolsManager to solely on UI thread this simple sync method
> needs to be handled asynchronously-- will we have an async method like void
> TerminatedWorkerExist(const Callback<void(bool)>& callback) in the new
> architecture then?

Done.

[horo, 2014-03-27 09:47:53]

On 2014/03/27 09:45:48, horo wrote:
>
https://codereview.chromium.org/213423008/diff/1/content/browser/devtools/wor...
> File content/browser/devtools/worker_devtools_manager.cc (right):
> 
>
https://codereview.chromium.org/213423008/diff/1/content/browser/devtools/wor...
> content/browser/devtools/worker_devtools_manager.cc:297: }
> On 2014/03/27 08:04:34, kinuko wrote:
> > I think adding pause_on_start param in CreateWorker message makes sense, but
> > when we move DevToolsManager to solely on UI thread this simple sync method
> > needs to be handled asynchronously-- will we have an async method like void
> > TerminatedWorkerExist(const Callback<void(bool)>& callback) in the new
> > architecture then?
> 
> Done.

Sorry this is wrong response.

[horo, 2014-03-27 09:48:10]

https://codereview.chromium.org/213423008/diff/1/content/browser/devtools/wor...
File content/browser/devtools/worker_devtools_manager.cc (right):

https://codereview.chromium.org/213423008/diff/1/content/browser/devtools/wor...
content/browser/devtools/worker_devtools_manager.cc:283: bool
WorkerDevToolsManager::TerminatedWorkerExist(
On 2014/03/27 08:37:26, yurys wrote:
> We could merge WorkerCreated into this new method to make it clear why the
> worker is now in paused_workers_ map.

Done.

[yurys, 2014-03-27 12:16:35]

lgtm

[kinuko, 2014-03-27 12:26:09]

On 2014/03/27 09:45:18, horo wrote:
> On 2014/03/27 08:04:34, kinuko wrote:
> >
>
https://codereview.chromium.org/213423008/diff/1/content/browser/devtools/wor...
> > File content/browser/devtools/worker_devtools_manager.cc (right):
> > 
> >
>
https://codereview.chromium.org/213423008/diff/1/content/browser/devtools/wor...
> > content/browser/devtools/worker_devtools_manager.cc:297: }
> > I think adding pause_on_start param in CreateWorker message makes sense, but
> > when we move DevToolsManager to solely on UI thread this simple sync method
> > needs to be handled asynchronously-- will we have an async method like void
> > TerminatedWorkerExist(const Callback<void(bool)>& callback) in the new
> > architecture then?
> 
> Yes in the new architecture (http://crrev.com/196503005) we have to go to the
UI
> thread before sending WorkerProcessMsg_CreateWorker message to check the
> existence of the terminated worker.

Hmm.. ok let's see how it looks then.

lgtm

[horo, 2014-03-27 15:20:15]

jochen@
Could you please review content/child/shared_worker_devtools_agent.* in this cl?
Thank you.

[jochen (gone - plz use gerrit), 2014-03-27 15:21:43]

lgtm

[horo, 2014-03-27 15:22:32]

nasko@
Could you please review
 content/common/devtools_messages.h
 content/common/worker_messages.h
in this CL?

Thank you.

[nasko, 2014-03-27 18:02:05]

messages LGTM

[horo, 2014-03-27 22:03:23]

The CQ bit was checked by horo@chromium.org

[commit-bot: I haz the power, 2014-03-27 22:05:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/horo@chromium.org/213423008/20001

[horo, 2014-03-29 04:49:43]

The CQ bit was unchecked by horo@chromium.org

[horo, 2014-03-29 04:49:43]

The CQ bit was checked by horo@chromium.org

[horo, 2014-03-29 05:19:51]

The CQ bit was unchecked by horo@chromium.org

[horo, 2014-03-29 05:29:58]

The CQ bit was checked by horo@chromium.org

[commit-bot: I haz the power, 2014-03-29 05:30:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/horo@chromium.org/213423008/20001

[commit-bot: I haz the power, 2014-03-29 17:29:27]

Change committed as 260377