[telemetry] Install PushAppsToBackground apk

[telemetry] Install PushAppsToBackground apk

Install the PushAppsToBackground.apk to Android phones
and reimplement Background() to use this new apk.
This works around an issue where not every Android phone has
the clock app installed which is what we currently use to force
our app into the background.

BUG=chromium:586148
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/809b177fe47870411eaad67a30797ba7ed4ef33d

[hjd, 2016-10-06 15:39:00]

hjd@chromium.org changed reviewers:
+ nednguyen@google.com, perezju@chromium.org

[hjd, 2016-10-06 15:39:00]

hjd@chromium.org changed reviewers:
+ nednguyen@google.com, perezju@chromium.org

[hjd, 2016-10-06 15:39:01]

ptal, thanks :)

[hjd, 2016-10-06 15:39:01]

ptal, thanks :)

[perezju, 2016-10-06 15:51:08]

https://codereview.chromium.org/2398683006/diff/1/telemetry/telemetry/interna...
File telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py
(right):

https://codereview.chromium.org/2398683006/diff/1/telemetry/telemetry/interna...
telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py:112:
self._platform_backend.EnsureBackgroundApkInstalled()
I don't think this should be done when starting tracing; benchmarks may want to
push apps to background, even if they don't do any tracing.

Maybe should be installed during android_platform_backend.__init__? Or in
story_runner.py? Or the shared page state?

Ned, any opinions?

[nednguyen, 2016-10-08 14:31:27]

On 2016/10/06 15:51:08, perezju wrote:
>
https://codereview.chromium.org/2398683006/diff/1/telemetry/telemetry/interna...
> File
telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py
> (right):
> 
>
https://codereview.chromium.org/2398683006/diff/1/telemetry/telemetry/interna...
>
telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py:112:
> self._platform_backend.EnsureBackgroundApkInstalled()
> I don't think this should be done when starting tracing; benchmarks may want
to
> push apps to background, even if they don't do any tracing.
> 
> Maybe should be installed during android_platform_backend.__init__? Or in
> story_runner.py? Or the shared page state?

android_platform_backend.__init__ is the right place to add this. 
> 
> Ned, any opinions?

What Juan says is the truth.

[nednguyen, 2016-10-08 17:41:12]

On 2016/10/08 14:31:27, nednguyen wrote:
> On 2016/10/06 15:51:08, perezju wrote:
> >
>
https://codereview.chromium.org/2398683006/diff/1/telemetry/telemetry/interna...
> > File
> telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py
> > (right):
> > 
> >
>
https://codereview.chromium.org/2398683006/diff/1/telemetry/telemetry/interna...
> >
>
telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py:112:
> > self._platform_backend.EnsureBackgroundApkInstalled()
> > I don't think this should be done when starting tracing; benchmarks may want
> to
> > push apps to background, even if they don't do any tracing.
> > 
> > Maybe should be installed during android_platform_backend.__init__? Or in
> > story_runner.py? Or the shared page state?
> 
> android_platform_backend.__init__ is the right place to add this. 
> > 
> > Ned, any opinions?
> 
> What Juan says is the truth.

Actually let avoid side effect in constructor if possible. You can lazily does
this installation when Background() is first called.

[nednguyen, 2016-10-08 17:45:07]

On 2016/10/08 17:41:12, nednguyen wrote:
> On 2016/10/08 14:31:27, nednguyen wrote:
> > On 2016/10/06 15:51:08, perezju wrote:
> > >
> >
>
https://codereview.chromium.org/2398683006/diff/1/telemetry/telemetry/interna...
> > > File
> > telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py
> > > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2398683006/diff/1/telemetry/telemetry/interna...
> > >
> >
>
telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py:112:
> > > self._platform_backend.EnsureBackgroundApkInstalled()
> > > I don't think this should be done when starting tracing; benchmarks may
want
> > to
> > > push apps to background, even if they don't do any tracing.
> > > 
> > > Maybe should be installed during android_platform_backend.__init__? Or in
> > > story_runner.py? Or the shared page state?
> > 
> > android_platform_backend.__init__ is the right place to add this. 
> > > 
> > > Ned, any opinions?
> > 
> > What Juan says is the truth.
> 
> Actually let avoid side effect in constructor if possible. You can lazily does
> this installation when Background() is first called.

Sorry for change of heart again. Installing an apk can have heavy cost & affect
the perf test, so it's not a great solution either. So I think we should
add a method like Initialize() for platform & initialize it nearby
https://cs.chromium.org/chromium/src/third_party/catapult/telemetry/telemetry...


The downside is now all people who write new shared_state that involve the
BackGround action need to explicitly install the apk themselves, but explicit is
probably better implicit in this case since the installation cost is not trivial
:-/

[perezju, 2016-10-10 09:47:32]

On 2016/10/08 17:45:07, nednguyen wrote:
> On 2016/10/08 17:41:12, nednguyen wrote:
> > On 2016/10/08 14:31:27, nednguyen wrote:
> > > On 2016/10/06 15:51:08, perezju wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/2398683006/diff/1/telemetry/telemetry/interna...
> > > > File
> > >
telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py
> > > > (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2398683006/diff/1/telemetry/telemetry/interna...
> > > >
> > >
> >
>
telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py:112:
> > > > self._platform_backend.EnsureBackgroundApkInstalled()
> > > > I don't think this should be done when starting tracing; benchmarks may
> want
> > > to
> > > > push apps to background, even if they don't do any tracing.
> > > > 
> > > > Maybe should be installed during android_platform_backend.__init__? Or
in
> > > > story_runner.py? Or the shared page state?
> > > 
> > > android_platform_backend.__init__ is the right place to add this. 
> > > > 
> > > > Ned, any opinions?
> > > 
> > > What Juan says is the truth.
> > 
> > Actually let avoid side effect in constructor if possible. You can lazily
does
> > this installation when Background() is first called.
> 
> Sorry for change of heart again. Installing an apk can have heavy cost &
affect
> the perf test, so it's not a great solution either. So I think we should
> add a method like Initialize() for platform & initialize it nearby
>
https://cs.chromium.org/chromium/src/third_party/catapult/telemetry/telemetry...
> 
> 
> The downside is now all people who write new shared_state that involve the
> BackGround action need to explicitly install the apk themselves, but explicit
is
> probably better implicit in this case since the installation cost is not
trivial
> :-/

Agree on better being explicit. The plan sgtm.

[hjd, 2016-10-10 17:46:25]

On 2016/10/10 09:47:32, perezju wrote:
> On 2016/10/08 17:45:07, nednguyen wrote:
> > On 2016/10/08 17:41:12, nednguyen wrote:
> > > On 2016/10/08 14:31:27, nednguyen wrote:
> > > > On 2016/10/06 15:51:08, perezju wrote:
> > > > >
> > > >
> > >
> >
>
https://codereview.chromium.org/2398683006/diff/1/telemetry/telemetry/interna...
> > > > > File
> > > >
> telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py
> > > > > (right):
> > > > > 
> > > > >
> > > >
> > >
> >
>
https://codereview.chromium.org/2398683006/diff/1/telemetry/telemetry/interna...
> > > > >
> > > >
> > >
> >
>
telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py:112:
> > > > > self._platform_backend.EnsureBackgroundApkInstalled()
> > > > > I don't think this should be done when starting tracing; benchmarks
may
> > want
> > > > to
> > > > > push apps to background, even if they don't do any tracing.
> > > > > 
> > > > > Maybe should be installed during android_platform_backend.__init__? Or
> in
> > > > > story_runner.py? Or the shared page state?
> > > > 
> > > > android_platform_backend.__init__ is the right place to add this. 
> > > > > 
> > > > > Ned, any opinions?
> > > > 
> > > > What Juan says is the truth.
> > > 
> > > Actually let avoid side effect in constructor if possible. You can lazily
> does
> > > this installation when Background() is first called.
> > 
> > Sorry for change of heart again. Installing an apk can have heavy cost &
> affect
> > the perf test, so it's not a great solution either. So I think we should
> > add a method like Initialize() for platform & initialize it nearby
> >
>
https://cs.chromium.org/chromium/src/third_party/catapult/telemetry/telemetry...
> > 
> > 
> > The downside is now all people who write new shared_state that involve the
> > BackGround action need to explicitly install the apk themselves, but
explicit
> is
> > probably better implicit in this case since the installation cost is not
> trivial
> > :-/
> 
> Agree on better being explicit. The plan sgtm.

Updated :)

[nednguyen, 2016-10-10 18:42:31]

https://codereview.chromium.org/2398683006/diff/40001/telemetry/telemetry/int...
File telemetry/telemetry/internal/platform/android_platform_backend.py (right):

https://codereview.chromium.org/2398683006/diff/40001/telemetry/telemetry/int...
telemetry/telemetry/internal/platform/android_platform_backend.py:243:
directories = util.GetBuildDirectories()
Hmhh, don't rely on this method. Instead, use dependency_manager to fetch the
apk instead. To do this, you need:
1) Upload the apk to cloud storage.
2) Add an entry to telemetry/telemetry/internal/binary_dependencies.json
3) Switch android_prebuilt_profiler_helper.InstallOnDevice(self._device,
'PushAppsToBackground'):

[hjd, 2016-10-11 17:56:59]

On 2016/10/10 18:42:31, nednguyen wrote:
>
https://codereview.chromium.org/2398683006/diff/40001/telemetry/telemetry/int...
> File telemetry/telemetry/internal/platform/android_platform_backend.py
(right):
> 
>
https://codereview.chromium.org/2398683006/diff/40001/telemetry/telemetry/int...
> telemetry/telemetry/internal/platform/android_platform_backend.py:243:
> directories = util.GetBuildDirectories()
> Hmhh, don't rely on this method. Instead, use dependency_manager to fetch the
> apk instead. To do this, you need:
> 1) Upload the apk to cloud storage.
> 2) Add an entry to telemetry/telemetry/internal/binary_dependencies.json
> 3) Switch android_prebuilt_profiler_helper.InstallOnDevice(self._device,
> 'PushAppsToBackground'):

Thanks :)
Done.

[nednguyen, 2016-10-11 18:55:56]

lgtm

https://codereview.chromium.org/2398683006/diff/60001/telemetry/telemetry/cor...
File telemetry/telemetry/core/android_platform.py (right):

https://codereview.chromium.org/2398683006/diff/60001/telemetry/telemetry/cor...
telemetry/telemetry/core/android_platform.py:18: def Initialize(self):
nits: usually the way we do this is having platform.Initialize call
platform_backend.Initialize(), then the based implementation of
platform_backend.Initialize is doing nothing.

Most of platform in telemetry is supposed to have same API, but we special case
Android because Android is very different from that of the desktop ones..

[perezju, 2016-10-12 10:10:05]

https://codereview.chromium.org/2398683006/diff/80001/telemetry/telemetry/int...
File telemetry/telemetry/internal/platform/android_platform_backend.py (right):

https://codereview.chromium.org/2398683006/diff/80001/telemetry/telemetry/int...
telemetry/telemetry/internal/platform/android_platform_backend.py:246:
self._device, 'push_apps_to_background_apk'):
This doesn't work. It "pushes" the apk to the device, but does not "install" it.

I think you should use the binary_manager to fetch the apk, and then install it
with _device.Install.

[hjd, 2016-10-13 11:09:30]

On 2016/10/12 10:10:05, perezju wrote:
>
https://codereview.chromium.org/2398683006/diff/80001/telemetry/telemetry/int...
> File telemetry/telemetry/internal/platform/android_platform_backend.py
(right):
> 
>
https://codereview.chromium.org/2398683006/diff/80001/telemetry/telemetry/int...
> telemetry/telemetry/internal/platform/android_platform_backend.py:246:
> self._device, 'push_apps_to_background_apk'):
> This doesn't work. It "pushes" the apk to the device, but does not "install"
it.
> 
> I think you should use the binary_manager to fetch the apk, and then install
it
> with _device.Install.

Done, thanks :)

[hjd, 2016-10-13 11:10:07]

Thanks :)

https://codereview.chromium.org/2398683006/diff/60001/telemetry/telemetry/cor...
File telemetry/telemetry/core/android_platform.py (right):

https://codereview.chromium.org/2398683006/diff/60001/telemetry/telemetry/cor...
telemetry/telemetry/core/android_platform.py:18: def Initialize(self):
On 2016/10/11 18:55:56, nednguyen wrote:
> nits: usually the way we do this is having platform.Initialize call
> platform_backend.Initialize(), then the based implementation of
> platform_backend.Initialize is doing nothing.
> 
> Most of platform in telemetry is supposed to have same API, but we special
case
> Android because Android is very different from that of the desktop ones..

Done.

https://codereview.chromium.org/2398683006/diff/80001/telemetry/telemetry/int...
File telemetry/telemetry/internal/platform/android_platform_backend.py (right):

https://codereview.chromium.org/2398683006/diff/80001/telemetry/telemetry/int...
telemetry/telemetry/internal/platform/android_platform_backend.py:246:
self._device, 'push_apps_to_background_apk'):
On 2016/10/12 10:10:05, perezju wrote:
> This doesn't work. It "pushes" the apk to the device, but does not "install"
it.
> 
> I think you should use the binary_manager to fetch the apk, and then install
it
> with _device.Install.

Done.

[hjd, 2016-10-18 11:21:55]

How does it look now Juan? :)

[perezju, 2016-10-18 13:03:07]

On 2016/10/18 11:21:55, hjd wrote:
> How does it look now Juan? :)

lgtm, thanks!

[hjd, 2016-10-19 11:04:52]

The CQ bit was checked by hjd@chromium.org

[hjd, 2016-10-19 11:04:53]

The patchset sent to the CQ was uploaded after l-g-t-m from nednguyen@google.com
Link to the patchset: https://codereview.chromium.org/2398683006/#ps100001
(title: "fix to actually install")

[commit-bot: I haz the power, 2016-10-19 11:04:56]

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, 2016-10-19 11:19:05]

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

[commit-bot: I haz the power, 2016-10-19 11:19:07]

Try jobs failed on following builders:
  Catapult Mac Tryserver on master.tryserver.client.catapult (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Ma...)

[hjd, 2016-10-19 15:26:25]

The CQ bit was checked by hjd@chromium.org

[hjd, 2016-10-19 15:26:26]

The patchset sent to the CQ was uploaded after l-g-t-m from
perezju@chromium.org, nednguyen@google.com
Link to the patchset: https://codereview.chromium.org/2398683006/#ps120001
(title: "add method to test fake")

[commit-bot: I haz the power, 2016-10-19 15:26:36]

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, 2016-10-19 15:46:02]

Description was changed from

==========
[telemetry] Install PushAppsToBackground apk

Install the PushAppsToBackground.apk to Android phones
and reimplement Background() to use this new apk.
This works around an issue where not every Android phone has
the clock app installed which is what we currently use to force
our app into the background.

BUG=chromium:586148
==========

to

==========
[telemetry] Install PushAppsToBackground apk

Install the PushAppsToBackground.apk to Android phones
and reimplement Background() to use this new apk.
This works around an issue where not every Android phone has
the clock app installed which is what we currently use to force
our app into the background.

BUG=chromium:586148

Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2016-10-19 15:46:03]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...