Have mash_browser_tests recreate BackgroundServiceManager per test

Have mash_browser_tests recreate BackgroundServiceManager per test

Currently one BackgroundServiceManager is created per batch of browser_tests.
This ends up leaving the UI and Ash process running between tests.

This is especially problematic for tests with PRE_test where state is expected
to not be preserved into the following test.

This change delays the initialization of a BackgroundServiceManager until the
child test process has connected to the MojoTestState. Subsequent tests will
tear down the previous BackgroundServiceManager before creating the new one.

This also tears down the UI and Ash processes as desired.

TEST=mash_browser_tests
BUG=678687
Review-Url: https://codereview.chromium.org/2858103002
Cr-Commit-Position: refs/heads/master@{#473897}
Committed: https://chromium.googlesource.com/chromium/src/+/d86012b80944319678d07adcc26b07b873deb5fa

[jonross, 2017-05-03 22:06:39]

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

[jonross, 2017-05-03 22:06:40]

Hey sky@

I realize that you are out for training, so feel free to get to this when you
are back.

This change move the creation of BackgroundServiceManager from initialization of
MojoTestConnector to when the child test process connects to MojoTestState.

This allows for all mojo pipe handles to be generated first, when the test suite
starts. While then tearing down the ServiceManager and active services for each
subsequent test.

This also supports the test of the format PRE_test, then test. Resetting the
environment for each part.

Thanks,
Jon

[sky, 2017-05-04 02:49:50]

https://codereview.chromium.org/2858103002/diff/40001/chrome/test/base/mojo_t...
File chrome/test/base/mojo_test_connector.cc (right):

https://codereview.chromium.org/2858103002/diff/40001/chrome/test/base/mojo_t...
chrome/test/base/mojo_test_connector.cc:206: void MojoTestConnector::Init() {
When multiple tests are run at a time, how many times is this called? It seems
like it's only called once, but I could be wrong.

[jonross, 2017-05-04 13:41:41]

https://codereview.chromium.org/2858103002/diff/40001/chrome/test/base/mojo_t...
File chrome/test/base/mojo_test_connector.cc (right):

https://codereview.chromium.org/2858103002/diff/40001/chrome/test/base/mojo_t...
chrome/test/base/mojo_test_connector.cc:206: void MojoTestConnector::Init() {
On 2017/05/04 02:49:49, sky wrote:
> When multiple tests are run at a time, how many times is this called? It seems
> like it's only called once, but I could be wrong.

This will be only called once.

MashTestLauncherDelegate::PreRunTest is called N times (once per test)
The first call of this builds MojoTestConnector and calls Init once. This gives
us the mojo environment we need.

N MojoTestStates are made, creating the pipe handles for each test. Which is
sent to them via command line as they are forked.

MojoTestConnector::InitBackgroundServerManager will eventually be ran N times.
However it only runs once per test. When the test case connects.

[sky, 2017-05-04 15:23:03]

Excellent. LGTM

[jonross, 2017-05-04 20:46:07]

On 2017/05/04 15:23:03, sky wrote:
> Excellent. LGTM

I'm going to hold off on landing this.

While running small sets locally everything was fine.

However running the full suite on the trybot has found some interesting errors.

One I haven't found the cause for, but there appears to be a race around
resource loading.

The other is that we fail on all test retries. In fact we init Aura as Local.
Investigating.

[sky, 2017-05-04 23:50:33]

+rockot

Ken recently fixed a bug in mash_browser_tests that manifested as Aura
being initted as local. That fix is here:
https://chromium-review.googlesource.com/c/493800/ .

The mas/mus tests set the Env::Mode here:
https://chromium.googlesource.com/chromium/src/+/faf6b1e5ba15004aec259c817b75...
.


On Thu, May 4, 2017 at 1:46 PM, <jonross@chromium.org> wrote:

> On 2017/05/04 15:23:03, sky wrote:
> > Excellent. LGTM
>
> I'm going to hold off on landing this.
>
> While running small sets locally everything was fine.
>
> However running the full suite on the trybot has found some interesting
> errors.
>
> One I haven't found the cause for, but there appears to be a race around
> resource loading.
>
> The other is that we fail on all test retries. In fact we init Aura as
> Local.
> Investigating.
>
> https://codereview.chromium.org/2858103002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jonross, 2017-05-05 14:11:00]

Yeah, I had already pulled that patch.

What appears to be happening is that we are never getting into
MashTestLauncherDelegate::RunTestSuite for the tests being retried. So we
never add the Env::Mode.

I'm going to hunt down why that is the case. We might be dropping
--run-in-mash when entering the retry phase.

On Thu, May 4, 2017 at 7:50 PM Scott Violet <sky@chromium.org> wrote:

> +rockot
>
> Ken recently fixed a bug in mash_browser_tests that manifested as Aura
> being initted as local. That fix is here:
> https://chromium-review.googlesource.com/c/493800/ .
>
> The mas/mus tests set the Env::Mode here:
>
https://chromium.googlesource.com/chromium/src/+/faf6b1e5ba15004aec259c817b75...
> .
>
>
> On Thu, May 4, 2017 at 1:46 PM, <jonross@chromium.org> wrote:
>
>> On 2017/05/04 15:23:03, sky wrote:
>> > Excellent. LGTM
>>
>> I'm going to hold off on landing this.
>>
>> While running small sets locally everything was fine.
>>
>> However running the full suite on the trybot has found some interesting
>> errors.
>>
>> One I haven't found the cause for, but there appears to be a race around
>> resource loading.
>>
>> The other is that we fail on all test retries. In fact we init Aura as
>> Local.
>> Investigating.
>>
>> https://codereview.chromium.org/2858103002/
>>
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sky, 2017-05-05 14:18:30]

That code should be called here:
https://chromium.googlesource.com/chromium/src/+/master/content/public/test/t...
. Maybe the switches are some how wrong?

On Fri, May 5, 2017 at 7:10 AM, Jonathan Ross <jonross@chromium.org> wrote:

> Yeah, I had already pulled that patch.
>
> What appears to be happening is that we are never getting into
> MashTestLauncherDelegate::RunTestSuite for the tests being retried. So we
> never add the Env::Mode.
>
> I'm going to hunt down why that is the case. We might be dropping
> --run-in-mash when entering the retry phase.
>
> On Thu, May 4, 2017 at 7:50 PM Scott Violet <sky@chromium.org> wrote:
>
>> +rockot
>>
>> Ken recently fixed a bug in mash_browser_tests that manifested as Aura
>> being initted as local. That fix is here: https://chromium-review.
>> googlesource.com/c/493800/ .
>>
>> The mas/mus tests set the Env::Mode here: https://chromium.
>> googlesource.com/chromium/src/+/faf6b1e5ba15004aec259c817b7550
>> 9df9639c88/chrome/test/base/mash_browser_tests_main.cc#98 .
>>
>>
>> On Thu, May 4, 2017 at 1:46 PM, <jonross@chromium.org> wrote:
>>
>>> On 2017/05/04 15:23:03, sky wrote:
>>> > Excellent. LGTM
>>>
>>> I'm going to hold off on landing this.
>>>
>>> While running small sets locally everything was fine.
>>>
>>> However running the full suite on the trybot has found some interesting
>>> errors.
>>>
>>> One I haven't found the cause for, but there appears to be a race around
>>> resource loading.
>>>
>>> The other is that we fail on all test retries. In fact we init Aura as
>>> Local.
>>> Investigating.
>>>
>>> https://codereview.chromium.org/2858103002/
>>>
>>
>>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jonross, 2017-05-12 13:56:57]

On 2017/05/05 14:18:30, sky wrote:
> That code should be called here:
>
https://chromium.googlesource.com/chromium/src/+/master/content/public/test/t...
> . Maybe the switches are some how wrong?
> 
> On Fri, May 5, 2017 at 7:10 AM, Jonathan Ross <mailto:jonross@chromium.org>
wrote:
> 
> > Yeah, I had already pulled that patch.
> >
> > What appears to be happening is that we are never getting into
> > MashTestLauncherDelegate::RunTestSuite for the tests being retried. So we
> > never add the Env::Mode.
> >
> > I'm going to hunt down why that is the case. We might be dropping
> > --run-in-mash when entering the retry phase.
> >
> > On Thu, May 4, 2017 at 7:50 PM Scott Violet <mailto:sky@chromium.org> wrote:
> >
> >> +rockot
> >>
> >> Ken recently fixed a bug in mash_browser_tests that manifested as Aura
> >> being initted as local. That fix is here: https://chromium-review.
> >> googlesource.com/c/493800/ .
> >>
> >> The mas/mus tests set the Env::Mode here: https://chromium.
> >> googlesource.com/chromium/src/+/faf6b1e5ba15004aec259c817b7550
> >> 9df9639c88/chrome/test/base/mash_browser_tests_main.cc#98 .
> >>
> >>
> >> On Thu, May 4, 2017 at 1:46 PM, <mailto:jonross@chromium.org> wrote:
> >>
> >>> On 2017/05/04 15:23:03, sky wrote:
> >>> > Excellent. LGTM
> >>>
> >>> I'm going to hold off on landing this.
> >>>
> >>> While running small sets locally everything was fine.
> >>>
> >>> However running the full suite on the trybot has found some interesting
> >>> errors.
> >>>
> >>> One I haven't found the cause for, but there appears to be a race around
> >>> resource loading.
> >>>
> >>> The other is that we fail on all test retries. In fact we init Aura as
> >>> Local.
> >>> Investigating.
> >>>
> >>> https://codereview.chromium.org/2858103002/
> >>>
> >>
> >>
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Hey sky@,
I've made some changes since you last reviewed this to address two types of test
errors that the initial patch uncovered.

This first is that child processes were starting with Aura in Local mode. I
updated ChromeTestLauncher to detect the existence of the mojo pipe to set Mus
mode.

The other is that UI Service and Aura Init were crashing when resource files we
unavailable. I've updated them to  instead exit the resource loading and to log
the error state. In the tests it can occur when services are shutting down.

Could you take another look?

Thanks,
Jon

[sky, 2017-05-12 17:16:47]

https://codereview.chromium.org/2858103002/diff/140001/chrome/test/base/mash_...
File chrome/test/base/mash_browser_tests_main.cc (right):

https://codereview.chromium.org/2858103002/diff/140001/chrome/test/base/mash_...
chrome/test/base/mash_browser_tests_main.cc:121: bool result =
ChromeTestLauncherDelegate::AdjustChildProcessCommandLine(
It doesn't seem that this override is necessary. Can it be removed?

https://codereview.chromium.org/2858103002/diff/140001/services/ui/service.cc
File services/ui/service.cc (right):

https://codereview.chromium.org/2858103002/diff/140001/services/ui/service.cc...
services/ui/service.cc:114: if (!loader.OpenFiles(std::move(directory),
resource_paths)) {
The reason this CHECKs is because to do otherwise leaves the client in a very
bad state, a state that they are likely to crash on soon. For example,
Resourcebundle is not registered if you early return so that any access to it is
going to CHECK. In other words, I think an early return here is just going to
push the crash around.

What client is being started after the filesystem shuts down?

https://codereview.chromium.org/2858103002/diff/140001/ui/views/mus/aura_init.cc
File ui/views/mus/aura_init.cc (right):

https://codereview.chromium.org/2858103002/diff/140001/ui/views/mus/aura_init...
ui/views/mus/aura_init.cc:113: if (!loader.OpenFiles(std::move(directory),
resource_paths)) {
Same comment here.

[jonross, 2017-05-12 18:08:02]

https://codereview.chromium.org/2858103002/diff/140001/chrome/test/base/mash_...
File chrome/test/base/mash_browser_tests_main.cc (right):

https://codereview.chromium.org/2858103002/diff/140001/chrome/test/base/mash_...
chrome/test/base/mash_browser_tests_main.cc:121: bool result =
ChromeTestLauncherDelegate::AdjustChildProcessCommandLine(
On 2017/05/12 17:16:47, sky wrote:
> It doesn't seem that this override is necessary. Can it be removed?

Whoops, missed this when I was removing my debugging. Will remove

https://codereview.chromium.org/2858103002/diff/140001/services/ui/service.cc
File services/ui/service.cc (right):

https://codereview.chromium.org/2858103002/diff/140001/services/ui/service.cc...
services/ui/service.cc:114: if (!loader.OpenFiles(std::move(directory),
resource_paths)) {
On 2017/05/12 17:16:47, sky wrote:
> The reason this CHECKs is because to do otherwise leaves the client in a very
> bad state, a state that they are likely to crash on soon. For example,
> Resourcebundle is not registered if you early return so that any access to it
is
> going to CHECK. In other words, I think an early return here is just going to
> push the crash around.
> 
> What client is being started after the filesystem shuts down?

There appears to be a race on short run tests which don't bring up visuals.

As the UI Service wouldn't be done initializing by the time the ServiceContext
for the filesystem has been destroyed.

So while UI service is completely useless without these resources, it would
subsequently be notified of teardown. And cleanly shutdown.

Would it be more appropriate to use this as an additional signal to start the
shutdown?

[jonross, 2017-05-16 18:44:01]

Patchset #12 (id:220001) has been deleted

[jonross, 2017-05-16 21:54:45]

https://codereview.chromium.org/2858103002/diff/140001/chrome/test/base/mash_...
File chrome/test/base/mash_browser_tests_main.cc (right):

https://codereview.chromium.org/2858103002/diff/140001/chrome/test/base/mash_...
chrome/test/base/mash_browser_tests_main.cc:121: bool result =
ChromeTestLauncherDelegate::AdjustChildProcessCommandLine(
On 2017/05/12 18:08:02, jonross wrote:
> On 2017/05/12 17:16:47, sky wrote:
> > It doesn't seem that this override is necessary. Can it be removed?
> 
> Whoops, missed this when I was removing my debugging. Will remove

Done.

https://codereview.chromium.org/2858103002/diff/260001/services/ui/service.cc
File services/ui/service.cc (right):

https://codereview.chromium.org/2858103002/diff/260001/services/ui/service.cc...
services/ui/service.cc:164: if (!InitializeResources(context()->connector()))
We talked offline about shutting down in response to no resources. Currently the
services are started by a ServiceContext, which awaits connection errors before
shutting down. There isn't a direct shutdown path within them.

Do we want to adjust this? Have Service::OnStart also begin returning a bool,
and having the ServiceContext also close out in that case?

[sky, 2017-05-17 16:58:59]

https://codereview.chromium.org/2858103002/diff/260001/services/ui/service.cc
File services/ui/service.cc (right):

https://codereview.chromium.org/2858103002/diff/260001/services/ui/service.cc...
services/ui/service.cc:164: if (!InitializeResources(context()->connector()))
On 2017/05/16 21:54:45, jonross wrote:
> We talked offline about shutting down in response to no resources. Currently
the
> services are started by a ServiceContext, which awaits connection errors
before
> shutting down. There isn't a direct shutdown path within them.
> 
> Do we want to adjust this? Have Service::OnStart also begin returning a bool,
> and having the ServiceContext also close out in that case?

I don't think we should change OnStart(), but could we instead make this code
stop the service itself? Basically call QuitNow() on the context.

[jonross, 2017-05-18 19:23:28]

Both ui/service and aura_init have been updated to quit the ServiceContext in
the event of no resource access.

https://codereview.chromium.org/2858103002/diff/260001/services/ui/service.cc
File services/ui/service.cc (right):

https://codereview.chromium.org/2858103002/diff/260001/services/ui/service.cc...
services/ui/service.cc:164: if (!InitializeResources(context()->connector()))
On 2017/05/17 16:58:59, sky wrote:
> On 2017/05/16 21:54:45, jonross wrote:
> > We talked offline about shutting down in response to no resources. Currently
> the
> > services are started by a ServiceContext, which awaits connection errors
> before
> > shutting down. There isn't a direct shutdown path within them.
> > 
> > Do we want to adjust this? Have Service::OnStart also begin returning a
bool,
> > and having the ServiceContext also close out in that case?
> 
> I don't think we should change OnStart(), but could we instead make this code
> stop the service itself? Basically call QuitNow() on the context.

Done.

[jonross, 2017-05-18 19:23:30]

Both ui/service and aura_init have been updated to quit the ServiceContext in
the event of no resource access.

[sky, 2017-05-18 19:47:35]

I'm ok with making the ui service quit, but I don't think AuraInit should do the
same. Instead it should return something to the callers and let them make the
decision.

How about keeping this patch at just updating the ui service?

[sky, 2017-05-18 19:52:58]

The reason for not making AuraInit do the quit is that I suspect it isn't
that easy. Code that uses AuraInit may expect certain things to happen so
that if AuraInit fails then the calling code needs to know and react
differently.

On Thu, May 18, 2017 at 12:47 PM, <sky@chromium.org> wrote:

> I'm ok with making the ui service quit, but I don't think AuraInit should
> do the
> same. Instead it should return something to the callers and let them make
> the
> decision.
>
> How about keeping this patch at just updating the ui service?
>
> https://codereview.chromium.org/2858103002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jonross, 2017-05-18 19:59:22]

On 2017/05/18 19:52:58, sky wrote:
> The reason for not making AuraInit do the quit is that I suspect it isn't
> that easy. Code that uses AuraInit may expect certain things to happen so
> that if AuraInit fails then the calling code needs to know and react
> differently.
> 
> On Thu, May 18, 2017 at 12:47 PM, <mailto:sky@chromium.org> wrote:
> 
> > I'm ok with making the ui service quit, but I don't think AuraInit should
> > do the
> > same. Instead it should return something to the callers and let them make
> > the
> > decision.
> >
> > How about keeping this patch at just updating the ui service?
> >
> > https://codereview.chromium.org/2858103002/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

I've reverted having AuraInit quit the ServiceContext. I'll take a closer look
at handling that appropriately in a subsequent CL

[sky, 2017-05-18 20:30:28]

https://codereview.chromium.org/2858103002/diff/300001/services/ui/service.cc
File services/ui/service.cc (right):

https://codereview.chromium.org/2858103002/diff/300001/services/ui/service.cc...
services/ui/service.cc:116: LOG(ERROR) << "Service failed to open resource
files.\n";
no need for the \n here.

https://codereview.chromium.org/2858103002/diff/300001/ui/views/mus/aura_init.cc
File ui/views/mus/aura_init.cc (right):

https://codereview.chromium.org/2858103002/diff/300001/ui/views/mus/aura_init...
ui/views/mus/aura_init.cc:116: LOG(ERROR) << "AuraInit failed to open resource
files.\n";
no \n here either.

[jonross, 2017-05-18 20:40:25]

As discussed offline all AuraInit changes are being pushed to a later patch.

https://codereview.chromium.org/2858103002/diff/300001/services/ui/service.cc
File services/ui/service.cc (right):

https://codereview.chromium.org/2858103002/diff/300001/services/ui/service.cc...
services/ui/service.cc:116: LOG(ERROR) << "Service failed to open resource
files.\n";
On 2017/05/18 20:30:27, sky wrote:
> no need for the \n here.

Done.

https://codereview.chromium.org/2858103002/diff/300001/ui/views/mus/aura_init.cc
File ui/views/mus/aura_init.cc (right):

https://codereview.chromium.org/2858103002/diff/300001/ui/views/mus/aura_init...
ui/views/mus/aura_init.cc:116: LOG(ERROR) << "AuraInit failed to open resource
files.\n";
On 2017/05/18 20:30:28, sky wrote:
> no \n here either.

reverted.

[sky, 2017-05-18 21:07:14]

LGTM

[jonross, 2017-05-18 21:07:50]

The CQ bit was checked by jonross@chromium.org

[commit-bot: I haz the power, 2017-05-18 21:08:49]

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-05-18 21:27:29]

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

[commit-bot: I haz the power, 2017-05-18 21:27:30]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[jonross, 2017-05-19 13:22:45]

The CQ bit was checked by jonross@chromium.org

[commit-bot: I haz the power, 2017-05-19 13:22:57]

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-05-19 13:46:12]

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

[commit-bot: I haz the power, 2017-05-19 13:46:13]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[jonross, 2017-05-19 13:52:27]

The CQ bit was checked by jonross@chromium.org

[jonross, 2017-05-19 13:52:28]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2858103002/#ps340001
(title: "fix mac compile")

[commit-bot: I haz the power, 2017-05-19 13:52: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-05-19 15:43:01]

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

[commit-bot: I haz the power, 2017-05-19 15:43:02]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[jonross, 2017-05-19 17:35:38]

The CQ bit was checked by jonross@chromium.org

[commit-bot: I haz the power, 2017-05-19 17:36:30]

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-05-19 20:07:52]

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

[commit-bot: I haz the power, 2017-05-19 20:07:53]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[jonross, 2017-05-23 13:41:21]

The CQ bit was checked by jonross@chromium.org

[commit-bot: I haz the power, 2017-05-23 13:41:42]

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-05-23 13:43:51]

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

[commit-bot: I haz the power, 2017-05-23 13:43:53]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[jonross, 2017-05-23 13:57:09]

The CQ bit was checked by jonross@chromium.org

[jonross, 2017-05-23 13:57:09]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2858103002/#ps360001
(title: "Rebase")

[commit-bot: I haz the power, 2017-05-23 13:57:27]

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-05-23 15:06:04]

CQ is committing da patch.

Bot data: {"patchset_id": 360001, "attempt_start_ts": 1495547829291840,
"parent_rev": "c84859811406738d0392874930908f0040ecaffa", "commit_rev":
"d86012b80944319678d07adcc26b07b873deb5fa"}

[commit-bot: I haz the power, 2017-05-23 15:06:12]

Description was changed from

==========
Have mash_browser_tests recreate BackgroundServiceManager per test

Currently one BackgroundServiceManager is created per batch of browser_tests.
This ends up leaving the UI and Ash process running between tests.

This is especially problematic for tests with PRE_test where state is expected
to not be preserved into the following test.

This change delays the initialization of a BackgroundServiceManager until the
child test process has connected to the MojoTestState. Subsequent tests will
tear down the previous BackgroundServiceManager before creating the new one.

This also tears down the UI and Ash processes as desired.

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

to

==========
Have mash_browser_tests recreate BackgroundServiceManager per test

Currently one BackgroundServiceManager is created per batch of browser_tests.
This ends up leaving the UI and Ash process running between tests.

This is especially problematic for tests with PRE_test where state is expected
to not be preserved into the following test.

This change delays the initialization of a BackgroundServiceManager until the
child test process has connected to the MojoTestState. Subsequent tests will
tear down the previous BackgroundServiceManager before creating the new one.

This also tears down the UI and Ash processes as desired.

TEST=mash_browser_tests
BUG=678687

Review-Url: https://codereview.chromium.org/2858103002
Cr-Commit-Position: refs/heads/master@{#473897}
Committed:
https://chromium.googlesource.com/chromium/src/+/d86012b80944319678d07adcc26b...
==========

[commit-bot: I haz the power, 2017-05-23 15:06:15]

Committed patchset #18 (id:360001) as
https://chromium.googlesource.com/chromium/src/+/d86012b80944319678d07adcc26b...