Hide WorkerProcessHost from chrome.

Hide WorkerProcessHost from chrome.

The code in chrome didn't need to know about all of WorkerProcessHost, which has many methods/getters especially counting WorkerInstance. Instead I added simple getters for the list of running workers, and the ability to terminate a worker.

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

[Andrew T Wilson (Slow), 2012-02-01 05:51:36]

Generally looks good. A few nits, and one potential bug, but LGTM after you
resolve them. Do you want to have dslomov or dimich take a look also, since
they've had their fingers in the code more recently than I have?

http://codereview.chromium.org/9317026/diff/7006/chrome/browser/chrome_conten...
File chrome/browser/chrome_content_browser_client.cc (right):

http://codereview.chromium.org/9317026/diff/7006/chrome/browser/chrome_conten...
chrome/browser/chrome_content_browser_client.cc:802: i->first, i->second, url,
name, display_name, !allow));
this line is indented one space too many

http://codereview.chromium.org/9317026/diff/7006/chrome/browser/chrome_conten...
chrome/browser/chrome_content_browser_client.cc:824: i->first, i->second, url,
!allow));
indent is off

http://codereview.chromium.org/9317026/diff/7006/chrome/browser/chrome_conten...
chrome/browser/chrome_content_browser_client.cc:826: 
remove blank line

http://codereview.chromium.org/9317026/diff/7006/chrome/browser/debugger/devt...
File chrome/browser/debugger/devtools_sanity_unittest.cc (right):

http://codereview.chromium.org/9317026/diff/7006/chrome/browser/debugger/devt...
chrome/browser/debugger/devtools_sanity_unittest.cc:363:
MessageLoop::QuitClosure());
The original code had a return statement in the loop. I actually don't think we
actually need a loop - just grab the first element? Or am I misreading this?

http://codereview.chromium.org/9317026/diff/7006/content/browser/worker_host/...
File content/browser/worker_host/worker_process_host.cc (right):

http://codereview.chromium.org/9317026/diff/7006/content/browser/worker_host/...
content/browser/worker_host/worker_process_host.cc:84:
std::vector<std::pair<int, int> > result;
How many copies of the vector does this end up creating? Is the compiler smart
enough to avoid copying into the return value? The alternative would be to pass
in a vector*. Not a huge deal, though - I'm OK as-is since in general this is a
very small vector.

Is there a reason why this isn't a member function on WorkerProcessHost?

Also, can |process| be a const in-param?

http://codereview.chromium.org/9317026/diff/7006/content/public/browser/conte...
File content/public/browser/content_browser_client.h (right):

http://codereview.chromium.org/9317026/diff/7006/content/public/browser/conte...
content/public/browser/content_browser_client.h:200: // Allow the embedder to
control if access to file system by a shared worker
Is this really only for shared workers?

[jam, 2012-02-01 16:53:53]

re reviewers, I'll keep that in mind for next time. this was sufficiently
straightforward change in that it shouldn't change functionality so I figure
multiple reviewers are not necessary

http://codereview.chromium.org/9317026/diff/7006/chrome/browser/chrome_conten...
File chrome/browser/chrome_content_browser_client.cc (right):

http://codereview.chromium.org/9317026/diff/7006/chrome/browser/chrome_conten...
chrome/browser/chrome_content_browser_client.cc:802: i->first, i->second, url,
name, display_name, !allow));
On 2012/02/01 05:51:37, Andrew T Wilson wrote:
> this line is indented one space too many

Done.

http://codereview.chromium.org/9317026/diff/7006/chrome/browser/chrome_conten...
chrome/browser/chrome_content_browser_client.cc:824: i->first, i->second, url,
!allow));
On 2012/02/01 05:51:37, Andrew T Wilson wrote:
> indent is off

Done.

http://codereview.chromium.org/9317026/diff/7006/chrome/browser/debugger/devt...
File chrome/browser/debugger/devtools_sanity_unittest.cc (right):

http://codereview.chromium.org/9317026/diff/7006/chrome/browser/debugger/devt...
chrome/browser/debugger/devtools_sanity_unittest.cc:363:
MessageLoop::QuitClosure());
On 2012/02/01 05:51:37, Andrew T Wilson wrote:
> The original code had a return statement in the loop. I actually don't think
we
> actually need a loop - just grab the first element? Or am I misreading this?

oops, thanks. took out loop

http://codereview.chromium.org/9317026/diff/7006/content/browser/worker_host/...
File content/browser/worker_host/worker_process_host.cc (right):

http://codereview.chromium.org/9317026/diff/7006/content/browser/worker_host/...
content/browser/worker_host/worker_process_host.cc:84:
std::vector<std::pair<int, int> > result;
On 2012/02/01 05:51:37, Andrew T Wilson wrote:
> How many copies of the vector does this end up creating? Is the compiler smart
> enough to avoid copying into the return value?

yep: see http://en.wikipedia.org/wiki/Return_value_optimization

> The alternative would be to pass
> in a vector*. Not a huge deal, though - I'm OK as-is since in general this is
a
> very small vector.
> 
> Is there a reason why this isn't a member function on WorkerProcessHost?

no reason. i'll just make it a member function

> 
> Also, can |process| be a const in-param?

http://codereview.chromium.org/9317026/diff/7006/content/public/browser/conte...
File content/public/browser/content_browser_client.h (right):

http://codereview.chromium.org/9317026/diff/7006/content/public/browser/conte...
content/public/browser/content_browser_client.h:200: // Allow the embedder to
control if access to file system by a shared worker
On 2012/02/01 05:51:37, Andrew T Wilson wrote:
> Is this really only for shared workers?

yeah, now that only shared workers run in a separate process (and so are hosted
by WorkerProcessHost)