Update AuraInit to handle failed initialization

It is possible for AuraInit to fail initialization. If the remote ServiceManager closes while a Service is starting up, then the synchronous calls to Directory will fail before error states are received.

This change follows up a previous change to AuraInit and QuickLauncher which was addressing these crashes in unittests. It refactors AuraInit to have an explicit initialization method. All callsites are updated to check the success of init, and to close cleanly upon failures.

TBR=sky@chromium.org
TEST=mash_browser_tests
BUG=678687
Review-Url: https://codereview.chromium.org/2967943002
Cr-Commit-Position: refs/heads/master@{#484583}
Committed: https://chromium.googlesource.com/chromium/src/+/14402447b7df16d16126dc49a3a44cf66c318c88

[jonross, 2017-07-04 14:04:49]

Patchset #2 (id:20001) has been deleted

[jonross, 2017-07-04 14:07:19]

Description was changed from

==========
cl format


tidy up


missed some


all targets under browser_Tests

BUG=
==========

to

==========
It is possible for AuraInit to fail initialization. If the remote ServiceManager
closes while a Service is starting up, then the synchronous calls to Directory
will fail before error states are received.

This change follows up a previous change to AuraInit and QuickLauncher which was
addressing these crashes in unittests. It refactors AuraInit to have an explicit
initialization method. All callsites are updated to check the success of init,
and to close cleanly upon failures.


TEST=mash_browser_tests
BUG=678687
==========

[jonross, 2017-07-04 14:17:06]

jonross@chromium.org changed reviewers:
+ sadrul@chromium.org

[jonross, 2017-07-04 14:17:07]

Hey Sadrul,

Could you review this change? I previously updated AuraInit to provide
information on if initialization succeeded. This lead to a fix of a crash in the
test suite.

This change updates AuraInit based on discussions with sky@, and applies that to
all callsite.

Thanks,
Jon

[sadrul, 2017-07-05 14:49:25]

lgtm

https://codereview.chromium.org/2967943002/diff/1/ui/views/mus/aura_init.h
File ui/views/mus/aura_init.h (right):

https://codereview.chromium.org/2967943002/diff/1/ui/views/mus/aura_init.h#ne...
ui/views/mus/aura_init.h:63: bool Init(
What do you think of making the ctor private, and add a static Create() function
(e.g. static std::unique_ptr<AuraInit> Create(....);), which returns null if
initialization failed.

https://codereview.chromium.org/2967943002/diff/1/ui/views/mus/aura_init.h#ne...
ui/views/mus/aura_init.h:77: const std::string& resource_file_200_);
no _ at the end

[jonross, 2017-07-05 14:54:14]

https://codereview.chromium.org/2967943002/diff/1/ui/views/mus/aura_init.h
File ui/views/mus/aura_init.h (right):

https://codereview.chromium.org/2967943002/diff/1/ui/views/mus/aura_init.h#ne...
ui/views/mus/aura_init.h:63: bool Init(
On 2017/07/05 14:49:25, sadrul wrote:
> What do you think of making the ctor private, and add a static Create()
function
> (e.g. static std::unique_ptr<AuraInit> Create(....);), which returns null if
> initialization failed.

SGTM

[jonross, 2017-07-05 17:51:31]

https://codereview.chromium.org/2967943002/diff/1/ui/views/mus/aura_init.h
File ui/views/mus/aura_init.h (right):

https://codereview.chromium.org/2967943002/diff/1/ui/views/mus/aura_init.h#ne...
ui/views/mus/aura_init.h:77: const std::string& resource_file_200_);
On 2017/07/05 14:49:24, sadrul wrote:
> no _ at the end

Done.

[jonross, 2017-07-05 17:51:50]

Description was changed from

==========
It is possible for AuraInit to fail initialization. If the remote ServiceManager
closes while a Service is starting up, then the synchronous calls to Directory
will fail before error states are received.

This change follows up a previous change to AuraInit and QuickLauncher which was
addressing these crashes in unittests. It refactors AuraInit to have an explicit
initialization method. All callsites are updated to check the success of init,
and to close cleanly upon failures.


TEST=mash_browser_tests
BUG=678687
==========

to

==========
It is possible for AuraInit to fail initialization. If the remote ServiceManager
closes while a Service is starting up, then the synchronous calls to Directory
will fail before error states are received.

This change follows up a previous change to AuraInit and QuickLauncher which was
addressing these crashes in unittests. It refactors AuraInit to have an explicit
initialization method. All callsites are updated to check the success of init,
and to close cleanly upon failures.

TBR=sky@chromium.org
TEST=mash_browser_tests
BUG=678687
==========

[jonross, 2017-07-05 17:51:50]

jonross@chromium.org changed reviewers:
+ sky@chromium.org

[jonross, 2017-07-05 17:52:46]

I've switched the initialization format for AuraInit to a Create method.

I'm TBRing sky@ for the callsites, as they are mechanical updates of the
refactor. Based on previously reviewed changes to AuraInit and QuickLauncher.

[jonross, 2017-07-05 17:52:50]

The CQ bit was checked by jonross@chromium.org

[jonross, 2017-07-05 17:52:51]

The patchset sent to the CQ was uploaded after l-g-t-m from sadrul@chromium.org
Link to the patchset: https://codereview.chromium.org/2967943002/#ps40001
(title: "Switch to a Create method for AuraInit")

[commit-bot: I haz the power, 2017-07-05 17:53:14]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-05 18:25:51]

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

[commit-bot: I haz the power, 2017-07-05 18:25:52]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[jonross, 2017-07-05 18:35:18]

The CQ bit was checked by jonross@chromium.org

[commit-bot: I haz the power, 2017-07-05 18:35:43]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-05 19:18:35]

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

[commit-bot: I haz the power, 2017-07-05 19:18:36]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[jonross, 2017-07-06 14:25:31]

The CQ bit was checked by jonross@chromium.org

[commit-bot: I haz the power, 2017-07-06 14:25:48]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-06 14:38:34]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1499351131602930,
"parent_rev": "e1352847922b68e79dd819bb44dbc05e6caaae75", "commit_rev":
"14402447b7df16d16126dc49a3a44cf66c318c88"}

[commit-bot: I haz the power, 2017-07-06 14:38:55]

Description was changed from

==========
It is possible for AuraInit to fail initialization. If the remote ServiceManager
closes while a Service is starting up, then the synchronous calls to Directory
will fail before error states are received.

This change follows up a previous change to AuraInit and QuickLauncher which was
addressing these crashes in unittests. It refactors AuraInit to have an explicit
initialization method. All callsites are updated to check the success of init,
and to close cleanly upon failures.

TBR=sky@chromium.org
TEST=mash_browser_tests
BUG=678687
==========

to

==========
It is possible for AuraInit to fail initialization. If the remote ServiceManager
closes while a Service is starting up, then the synchronous calls to Directory
will fail before error states are received.

This change follows up a previous change to AuraInit and QuickLauncher which was
addressing these crashes in unittests. It refactors AuraInit to have an explicit
initialization method. All callsites are updated to check the success of init,
and to close cleanly upon failures.

TBR=sky@chromium.org
TEST=mash_browser_tests
BUG=678687

Review-Url: https://codereview.chromium.org/2967943002
Cr-Commit-Position: refs/heads/master@{#484583}
Committed:
https://chromium.googlesource.com/chromium/src/+/14402447b7df16d16126dc49a3a4...
==========

[commit-bot: I haz the power, 2017-07-06 14:38:56]

Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/14402447b7df16d16126dc49a3a4...

[Fady Samuel, 2017-07-06 19:32:40]

A revert of this CL (patchset #2 id:40001) has been created in
https://codereview.chromium.org/2973103002/ by fsamuel@chromium.org.

The reason for reverting is: Causing chrome --mash to fail to start

[118236:118236:0706/142624.101546:3509128143561:ERROR:chrome_main_delegate.cc(1187)]
Main process exiting because service ash quit unexpectedly.
.