Removes BackgroundShellApplicationLoader from application_manager tests

Removes BackgroundShellApplicationLoader from application_manager tests

BackgroundShellApplicationLoader is android specific. I'm removing use
from application_manager_unittests so that I can move
BackgroundShellApplicationLoader (see
https://codereview.chromium.org/788243007/ ). There is only one test
in application_manager_unittests that relied on
BackgroundShellApplicationLoader: BDeleted.

BDeleted needed BackgroundShellApplicationLoader because resetting the
loader (BackgroundShellApplicationLoader) triggered shutting down
the background thread and background message loop (owned by
BackgroundShellApplicationLoader). Shutting down the MessageLoop
closes any pipes. As A/B are InterfaceImpls they are destroyed when
the pipe closes. Removing BackgroundShellApplicationLoader breaks
BDeleted because resetting the loader no longer triggers closing
pipes. The fix is to make Tester own the TestAImpls it creates. This
way when the test resets the loader the TestAImpls are destroyed so
that the other side (TestBimpl) sees the connection error and cleans
up.

BUG=none
TEST=none
R=qsr@chromium.org, davemoore@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/dd035f53e654d1c22fd30ffdca777f378e591ca6

[sky, 2014-12-17 18:57:29]

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

[sky, 2014-12-17 18:58:05]

I split this out from https://codereview.chromium.org/788243007/ to make it
easier to review.

Dave, I'm sending your way as you are in my time zone. If you prefer qsr to
review that's fine.

[qsr, 2014-12-18 08:58:35]

lgtm

https://codereview.chromium.org/812083002/diff/20001/mojo/application_manager...
File mojo/application_manager/application_manager_unittest.cc (right):

https://codereview.chromium.org/812083002/diff/20001/mojo/application_manager...
mojo/application_manager/application_manager_unittest.cc:327: struct
TestABinding {
I'm perfectly fine with this, but you could have obtained the same result
without using this class by having a scoped_vector of TestAImpl and either:
- use InterfaceImpl and use WeakBindToRequest so that you kept ownership of the
implementation.
- Have TestAImpl owns the Binding and takes an InterfaceRequest in its
constructor.

[sky, 2014-12-18 16:46:55]

https://codereview.chromium.org/812083002/diff/20001/mojo/application_manager...
File mojo/application_manager/application_manager_unittest.cc (right):

https://codereview.chromium.org/812083002/diff/20001/mojo/application_manager...
mojo/application_manager/application_manager_unittest.cc:327: struct
TestABinding {
On 2014/12/18 08:58:34, qsr wrote:
> I'm perfectly fine with this, but you could have obtained the same result
> without using this class by having a scoped_vector of TestAImpl and either:
> - use InterfaceImpl and use WeakBindToRequest so that you kept ownership of
the
> implementation.
> - Have TestAImpl owns the Binding and takes an InterfaceRequest in its
> constructor.

Good point. I went with your second suggestion.

[sky, 2014-12-18 16:47:20]

Committed patchset #4 (id:60001) manually as
dd035f53e654d1c22fd30ffdca777f378e591ca6 (presubmit successful).