New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 725695 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug

Blocked on:
issue 726515
issue 728404

Blocking:
issue 715565



Sign in to add a comment

Run android webview tests on swarming

Project Member Reported by martiniss@chromium.org, May 23 2017

Issue description

I was able to run android webview tests on swarming: https://chromium-swarm.appspot.com/task?id=365038abad87c310&refresh=10&show_raw=1

This is an isolate from a local release build, so it might be wrong.

Any idea why this isn't passing?
 
"android webview tests" is not very specific, fwiw.

I'd start by looking at "Not allowed to load local resource: file:///android_asset/webkit/android-weberror.png"
Is this test currently run on Webview. The failure is a timed out, so it could just be you have a bad webview build that fails this particular test.


Blocking: 715565
Owner: martiniss@chromium.org
Status: Started (was: Available)
Yeah, I'm not sure how to handle that error.

Let me run a test which is passing on the waterfall though.
Ok, https://chromium-swarm-dev.appspot.com/task?id=3650b63297f14510&refresh=10&show_raw=1 succeeded, as did the test on the main waterfall. So we might be good.

What's a good set of benchmarks I should run on webview, to make sure everything works ok?
Also, I'm not sure if this is correct, but the tests seem to take a lot longer on webview. I could be wrong, or my build of chromium could be unoptimized. I'm not sure.
I would also suggest running the isolate test with your local phone to see what goes wrong. Randy can help with debugging.
Blockedon: 726515
Ok, so I've been able to run the tests on some phones locally. I need to run this before run_benchmark succeeds though:

python build/android/adb_install_apk.py out/64bit/apks/SystemWebView.apk

I probably need to make telemetry run this command as well. I need both SystemWebView.apk and SystemWebViewShell.apk installed on the device to run webview tests, right? Anyone have an idea of how to do this? I looked at the code and it looks doable, if a bit complicated.
Cc: perezju@chromium.org
cc-ing perezju@ if you have any input.
Cc: boliu@chromium.org
boliu@: do you know if we need both SystemWebView.apk & SystemWebViewShell.apk to run webview tests?

Comment 11 by boliu@chromium.org, May 30 2017

These are perf tests? Then yes, you need both.

SystemWebView.apk is the (public version of) the thing we ship to the play store. Note the public version is only meant to run on aosp devices though. The actual production version is under a different package name, and the name of package used as the webview implementation is semi-hardcoded. So just installing SystemWebView.apk on device does not mean it's actually being used as the webview implementation.

SystemWebViewShell.apk is a small app that uses the system webview, whatever implementation it is. I think this is the thing telemetry talks to.
Thanks for the answer, Bo Liu@!

#8: for Telemetry to install both apk, you can update the code in https://github.com/catapult-project/catapult/blob/master/telemetry/telemetry/internal/backends/chrome/android_browser_finder.py#L152

Probably an optional _embedder_apk field, so that PossibleAndroidBrowser .UpdateExecutableIfNeeded works like this:

if HaveLocalAPK():
   self.platform.InstallApplication(self._local_apk)
if HaveEmbedderAPK():
   self.platform.InstallApplication(self._local_embedder_apk)
Sorry for the back n forth on direction, now I think about it, we just happen to use SystemWebViewShell.apk for our benchmark. Theoretically, if people want to use SystemWebView.apk with AndroidSearchApp.apk or GmailApp.apk, it should also be possible. 

So the way we implement this in Telemetry should be it support a --webview-embedder-apk-path flag, and have PossibleAndroidBrowser installs finder_options.webview_embedder_apk_path when its browser type is webview.
It's a bit more complicated than that, mikecase started a discussion at issue 711073 to improve on the current situation.

As far as I understand, setting up WebView on a device consists of:

1. Removing the stock webview implementation from the device (involves restarting the device)
  e.g.: https://github.com/catapult-project/catapult/blob/deb4a2aa8f11cb520e9b8bc7413296d899a3af47/devil/devil/android/tools/provision_devices.py#L375

2. Install apks: SystemWebView.apk (or SystemWebViewGoogle.apk on internal bots) and SystemWebViewShell.apk.

3. Select the correct webview implementation
   e.g. https://goto.google.com/rnokl (internal link)

As I mentioned in that issue, I'm not sure how much of this should be responsibility of Telemetry, and how much should be done on some previous device provisioning step.
Cc: bpastene@chromium.org
Wow, this seems a lot harder than I thought it gonna be. How does webview correctness testing on Android swarming work now? Probably best to mimic the solution there.

+Ben if you know the answer to this.

Comment 16 by boliu@chromium.org, May 31 2017

> How does webview correctness testing on Android swarming work now? Probably best to mimic the solution there.

That's based on the webview instrumentation test shell, target name webview_instrumentation_apk. That's a standalone apk that includes most (but not all) webview production code, plus fake implementations of private APIs that webview needs. It's fine for correctness testing (for code that's in there anyway), but not ideal for end-to-end perf testing, because part of that reimplemented private API is android compositor API, so it doesn't necessarily reflect production behavior.
Oh I see. For the comment in #14: does the step of removing the stock webview implementation only need to be done once, or do we need to do it for every test that uses a new webview apk?

If that's the former, it's probably be handled by infra, and not Telemetry. (2) & (3) seems to be in Telemetry realm to me.
We could make the infra/swarming side of things remove the default webview. (A lot of what swarming does is just a subset of provision_devices, so it would make sense to add the logic there.)

As for replacing it with a custom SystemWebView.apk, that should probably be done by the tests, yeah?


I talked with jbudorick@ about this offline. We thought that it would be good to make a small wrapper script which would do uninstallation, and optional re-installation of particular packages, and add that to devil.

Then our tests would call that wrapper, and have them not reinstall the package (to save on cycle time)

This is also useful for other teams, AFAIK.

I'm writing a small design doc about this. I'll post the link here in a bit when I have a first draft.
martiniss and I talked about this earlier today -- we need a generic mechanism for disabling/removing/hiding system packages in the scope of a task. (The VR folks want to do something similar for some of their functional tests.)

I don't think this is something we want to bake into swarming's provisioning logic, as it's not something we always want to do. (provision_devices currently handles it w/ a flag.)
I am supportive of making it general for other tests! However, can you file a another bug & mark it as blocker for this bug?
Blockedon: 728404
Project Member

Comment 23 by bugdroid1@chromium.org, Jun 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9ff9c3196bb331d8b97089e61b19888bbe75cfb5

commit 9ff9c3196bb331d8b97089e61b19888bbe75cfb5
Author: catapult-deps-roller@chromium.org <catapult-deps-roller@chromium.org>
Date: Tue Jun 13 00:38:37 2017

Roll src/third_party/catapult/ e663166b9..ceedebe21 (3 commits)

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/e663166b9cf3..ceedebe21711

$ git log e663166b9..ceedebe21 --date=short --no-merges --format='%ad %ae %s'
2017-06-12 martiniss Add webview embedded apk flag.
2017-06-12 mikecase Pass output-dir argument to stack script.
2017-06-12 mikecase Adds option to not have SetLogLevel add a log handler.

Created with:
  roll-dep src/third_party/catapult
BUG= 725695 ,721684


Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls


CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel
TBR=sullivan@chromium.org

Change-Id: I0eba150a1a220ca44e6ef346dd6c1ec08df3526b
Reviewed-on: https://chromium-review.googlesource.com/531837
Reviewed-by: <catapult-deps-roller@chromium.org>
Commit-Queue: <catapult-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#478843}
[modify] https://crrev.com/9ff9c3196bb331d8b97089e61b19888bbe75cfb5/DEPS

Project Member

Comment 24 by bugdroid1@chromium.org, Jun 14 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infradata/master-manager/+/4a022df7daf26ff6b5a2dbdaadc945cf32f0686d

commit 4a022df7daf26ff6b5a2dbdaadc945cf32f0686d
Author: Stephen Martinis <martiniss@google.com>
Date: Wed Jun 14 00:36:33 2017

Project Member

Comment 25 by bugdroid1@chromium.org, Jun 14 2017

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infradata/master-manager/+/4a022df7daf26ff6b5a2dbdaadc945cf32f0686d

commit 4a022df7daf26ff6b5a2dbdaadc945cf32f0686d
Author: Stephen Martinis <martiniss@google.com>
Date: Wed Jun 14 00:36:33 2017

Project Member

Comment 26 by bugdroid1@chromium.org, Jun 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/76218ac69794372fe5d575fffcd5b60dd6596688

commit 76218ac69794372fe5d575fffcd5b60dd6596688
Author: Stephen Martinis <martiniss@chromium.org>
Date: Thu Jun 15 01:29:05 2017

//tools/perf: Fix typo-ed device name

I misread the bot name in http://crbug.com/732898. This CL fixes the
typo

Bug:  725695 
Change-Id: I516a8732b7ec7a734e94ef24b7f62288d87ae822
Reviewed-on: https://chromium-review.googlesource.com/536174
Commit-Queue: Stephen Martinis <martiniss@chromium.org>
Commit-Queue: Ned Nguyen <nednguyen@google.com>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#479569}
[modify] https://crrev.com/76218ac69794372fe5d575fffcd5b60dd6596688/testing/buildbot/chromium.perf.fyi.json
[modify] https://crrev.com/76218ac69794372fe5d575fffcd5b60dd6596688/tools/perf/core/benchmark_sharding_map.json
[modify] https://crrev.com/76218ac69794372fe5d575fffcd5b60dd6596688/tools/perf/core/perf_data_generator.py

Project Member

Comment 27 by bugdroid1@chromium.org, Jun 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b2846d28f16b5d28625b4efbd66cf01ce9bf8d37

commit b2846d28f16b5d28625b4efbd66cf01ce9bf8d37
Author: Stephen Martinis <martiniss@chromium.org>
Date: Fri Jun 16 01:45:49 2017

//tools/perf: Fix pool dimension for fyi bot

I need to add a check for this as well :/

TBR=nednguyen@google.com
NOTRY=true

Bug:  725695 
Change-Id: Ibf1cee2c5ca0074e6b1b3d54c7bcc64a8218ad5e
Reviewed-on: https://chromium-review.googlesource.com/537733
Commit-Queue: Stephen Martinis <martiniss@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#479888}
[modify] https://crrev.com/b2846d28f16b5d28625b4efbd66cf01ce9bf8d37/testing/buildbot/chromium.perf.fyi.json
[modify] https://crrev.com/b2846d28f16b5d28625b4efbd66cf01ce9bf8d37/tools/perf/core/perf_data_generator.py

The webview builder is cycling green here: https://build.chromium.org/p/chromium.perf.fyi/builders/Android%20Nexus5X%20WebView%20Perf/builds/170

Awesome work!
The webview bot is indeed green, per #28. I'll plan to convert the webview bots on Monday. I'll make up the CLs tomorrow.

Do we want to compare any data between the existing webview bot and the new swarming bot?

Also, I don't see the new fyi webview bot on the perf dashboard :/ I can click on links from the buildbot log, but I don't see it in the auto complete.
I'm seeing it in the auto complete:


Screen Shot 2017-06-22 at 11.35.35 PM.png
95.7 KB View Download
#29: I think we can just compare:
1) The runtime of system_health or loading
2) The result of system_health.memory_mobile 

1) The run time is comparable
Graphs: https://chromeperf.appspot.com/report?sid=10a0399f2d19eec7feef61831e2d7b6a28b9446e2e8299d4f5cd5d0a1b65f615


2) The memory measurement is not comparable. In fact, there is *NO* memory metric output on Swarming Webview Nexus 5X at the moment!

browse:news:cnn trace of Android Webview Nexus 5X: https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/trace-file-id_13-2017-04-18_03-10-32-10769.html


browse:news:cnn trace of FYI Android Webview Nexus 5X Swarming:
https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/trace-file-id_9-2017-06-26_12-42-26-9270.html -> there is only memory data on Browser process?


Juan: can you help with investigating what could have been wrong here? 
Cc: sullivan@chromium.org
That is expected, webview typically has a single process so you should only see a single process (i.e. browser). Also, on chromeperf, you should be looking for metrics named "memory:webview:*".

For example:

https://chromeperf.appspot.com/report?sid=335afd92dc4b6a2b9c1b6ef16cb76a98f64722691ac1da34890cd044a38c9b57

shows that the metrics on two bots are comparable.

I guess we could ask for all "memory:chrome:*" metrics on webview bots to be removed to avoid confusion. +Annie, would that be possible?
Juan: I guess that make sense. Though how do we explain that the 1st trace and 2nd trace in #32 are so different?
I'm guessing that the first trace comes from a time where we were (wrongly) running tests using Chrome on that Webview bot.
#35: ah, that makes sense. Thanks Juan!

Stephen: I think it's safe to move forward with webview swarming now. :-)
Project Member

Comment 37 by bugdroid1@chromium.org, Jun 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/40c248d5a7227fc63bb4203c22ae4947f6d5bf30

commit 40c248d5a7227fc63bb4203c22ae4947f6d5bf30
Author: Stephen Martinis <martiniss@chromium.org>
Date: Wed Jun 28 23:02:23 2017

//tools/perf: Convert webview bots to swarming

This CL also removes the existing FYI webview bot, since there's a name
conflict between the FYI bot and the regular bot.

NOTRY=true

Bug:  725695 
Change-Id: I190a593186b3044cc2531cb496974fef91a28b41
Reviewed-on: https://chromium-review.googlesource.com/548997
Commit-Queue: Stephen Martinis <martiniss@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#483183}
[modify] https://crrev.com/40c248d5a7227fc63bb4203c22ae4947f6d5bf30/testing/buildbot/chromium.perf.fyi.json
[modify] https://crrev.com/40c248d5a7227fc63bb4203c22ae4947f6d5bf30/testing/buildbot/chromium.perf.json
[modify] https://crrev.com/40c248d5a7227fc63bb4203c22ae4947f6d5bf30/tools/perf/core/benchmark_sharding_map.json
[modify] https://crrev.com/40c248d5a7227fc63bb4203c22ae4947f6d5bf30/tools/perf/core/perf_data_generator.py

Status: Fixed (was: Started)
This works now!
\o/
Awesome!

Sign in to add a comment