Fix memory leak when errors happens in NaClProcessHost::Launch. Allow Launch to

Fix memory leak when errors happens in NaClProcessHost::Launch. Allow Launch to
be asynchronous. This is needed to support manifest autodetection for nacl-gdb.
Fix problems with Broker::OnLoaderDied due to new correct error handling in
Launch.

BUG=115392
TEST= browser_tests.exe --gtest_filter=NaClGdbTest.*, manual testing.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=128201

[Mark Seaborn, 2012-03-21 20:41:29]

LGTM with the changes below

http://codereview.chromium.org/9748012/diff/3003/chrome/browser/nacl_host/nac...
File chrome/browser/nacl_host/nacl_process_host.cc (right):

http://codereview.chromium.org/9748012/diff/3003/chrome/browser/nacl_host/nac...
chrome/browser/nacl_host/nacl_process_host.cc:49: static const char
dummy[1<<16]="1";
Left in by mistake?

http://codereview.chromium.org/9748012/diff/3003/chrome/browser/nacl_host/nac...
chrome/browser/nacl_host/nacl_process_host.cc:287: if (!RunningOnWOW64()) {
On merging with HEAD this becomes "if (debug_context_ != NULL)"

http://codereview.chromium.org/9748012/diff/3003/chrome/browser/nacl_host/nac...
chrome/browser/nacl_host/nacl_process_host.cc:321: void
NaClProcessHost::SendNaClLaunchError() {
This method only has one call site, doesn't it?  You could just inline it.

http://codereview.chromium.org/9748012/diff/3003/chrome/browser/nacl_host/nac...
File chrome/browser/nacl_host/nacl_process_host.h (right):

http://codereview.chromium.org/9748012/diff/3003/chrome/browser/nacl_host/nac...
chrome/browser/nacl_host/nacl_process_host.h:40: // Initialize the new NaCl
process, returning true on success. On success,
Update the comment.  Launch() no longer returns true on success.  Remove the 'On
success'/'On failure' parts.

http://codereview.chromium.org/9748012/diff/3003/chrome/browser/nacl_host/nac...
chrome/browser/nacl_host/nacl_process_host.h:83: // This field becames true when
the broker successfully launched
'becomes'.  Actually the comment is rather unnecessary since the variable name
is descriptive enough.

[halyavin, 2012-03-22 08:35:47]

https://chromiumcodereview.appspot.com/9748012/diff/3003/chrome/browser/nacl_...
File chrome/browser/nacl_host/nacl_process_host.cc (right):

https://chromiumcodereview.appspot.com/9748012/diff/3003/chrome/browser/nacl_...
chrome/browser/nacl_host/nacl_process_host.cc:49: static const char
dummy[1<<16]="1";
On 2012/03/21 20:41:29, Mark Seaborn wrote:
> Left in by mistake?
Sorry, I was doing an experiment in this working copy and forgot to revert the
change.

https://chromiumcodereview.appspot.com/9748012/diff/3003/chrome/browser/nacl_...
chrome/browser/nacl_host/nacl_process_host.cc:287: if (!RunningOnWOW64()) {
On 2012/03/21 20:41:29, Mark Seaborn wrote:
> On merging with HEAD this becomes "if (debug_context_ != NULL)"

Done.

https://chromiumcodereview.appspot.com/9748012/diff/3003/chrome/browser/nacl_...
chrome/browser/nacl_host/nacl_process_host.cc:321: void
NaClProcessHost::SendNaClLaunchError() {
On 2012/03/21 20:41:29, Mark Seaborn wrote:
> This method only has one call site, doesn't it?  You could just inline it.

Done.

https://chromiumcodereview.appspot.com/9748012/diff/3003/chrome/browser/nacl_...
File chrome/browser/nacl_host/nacl_process_host.h (right):

https://chromiumcodereview.appspot.com/9748012/diff/3003/chrome/browser/nacl_...
chrome/browser/nacl_host/nacl_process_host.h:40: // Initialize the new NaCl
process, returning true on success. On success,
On 2012/03/21 20:41:29, Mark Seaborn wrote:
> Update the comment.  Launch() no longer returns true on success.  Remove the
'On
> success'/'On failure' parts.

Done.

https://chromiumcodereview.appspot.com/9748012/diff/3003/chrome/browser/nacl_...
chrome/browser/nacl_host/nacl_process_host.h:83: // This field becames true when
the broker successfully launched
On 2012/03/21 20:41:29, Mark Seaborn wrote:
> 'becomes'.  Actually the comment is rather unnecessary since the variable name
> is descriptive enough.

Done.

[commit-bot: I haz the power, 2012-03-22 11:38:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/halyavin@google.com/9748012/12003

[commit-bot: I haz the power, 2012-03-22 13:08:38]

Change committed as 128201