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

Issue 841757 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

android-pixel2_webivew perf failing, unable to find android-webview browser

Project Member Reported by eyaich@chromium.org, May 10 2018

Issue description

Our current telemetry tests on pixel2 webview are failing due to being unable to find the webview browser:

https://chrome-swarming.appspot.com/task?id=3d5e0638fbdd1f10&refresh=10&show_raw=1


It looks like telemetry is looking for Monochrome.apk which we don't currently build in our isolate:

https://cs.chromium.org/chromium/src/third_party/catapult/telemetry/telemetry/internal/backends/android_browser_backend_settings.py?q=SystemWebview.apk&sq=package:chromium&dr=C&l=131

This is a tracking bug ot get this working for pixel2.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 10 2018

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

commit 301bc682ed113402e6490d8bd680551a43d39d95
Author: Emily Hanley <eyaich@google.com>
Date: Thu May 10 15:52:50 2018

Adding monochrome_public_apk to the perf isolate

Bug: 841757
Change-Id: I423f9dd3854eefdd5255f0a4524255820b0cbde5
Reviewed-on: https://chromium-review.googlesource.com/1053752
Commit-Queue: Emily Hanley <eyaich@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#557532}
[modify] https://crrev.com/301bc682ed113402e6490d8bd680551a43d39d95/tools/perf/chrome_telemetry_build/BUILD.gn

Project Member

Comment 2 by bugdroid1@chromium.org, May 10 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/26ea6d781a8b7d4cae2891ab4c34ed10436606ac

commit 26ea6d781a8b7d4cae2891ab4c34ed10436606ac
Author: Emily Hanley <eyaich@google.com>
Date: Thu May 10 18:21:57 2018

Adding monochrome_public_apk to the extra compile targets for the FYI builder

Bug:841757
Change-Id: I44f400de04ea62bb870a96d21fc16112e6328c51
Reviewed-on: https://chromium-review.googlesource.com/1053755
Reviewed-by: David Tu <dtu@chromium.org>
Commit-Queue: Emily Hanley <eyaich@chromium.org>

[modify] https://crrev.com/26ea6d781a8b7d4cae2891ab4c34ed10436606ac/scripts/slave/recipe_modules/chromium_tests/chromium_perf_fyi.py

Comment 3 by eyaich@chromium.org, May 11 2018

Cc: bpastene@chromium.org martiniss@chromium.org
Ok so I have both added the monochrom_public_apk to my isolate and added it to the compile targets for our builder (https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/chromium_tests/chromium_perf_fyi.py?l=66) although I am definitely not sure why they compile targets are needed there.  From a bug with martiniss@ crbug.com/639530 it looks like it is an artifact of the android recipe we migrated from.  Can someone clarify the need for those in the recipe?

We are still seeing the same error that it is unable to find the browser: https://chrome-swarming.appspot.com/task?id=3d682666799a0c10&refresh=10&show_raw=1

This is the first time we have run webview on >=N (so using monochrome) but Juan was able to successfully get it running on a webview gobo device (https://uberchromegw.corp.google.com/i/internal.client.clank/builders/perf-go-webview-phone/builds/1024), so I don't think it is a telemetry issue. See logs of that run here where it successfully installs the apk:

https://logs.chromium.org/v/?s=chrome%2Fbb%2Finternal.client.clank%2Fperf-go-webview-phone%2F1024%2F%2B%2Frecipes%2Fsteps%2Fsystem_health.webview_startup%2F0%2Fstdout

(INFO) 2018-05-09 17:34:02,841 browser_finder.FindBrowser:123  Chose browser: PossibleAndroidBrowser(browser_type=android-webview)
(WARNING) 2018-05-09 17:34:02,841 android_browser_finder.UpdateExecutableIfNeeded:239  Installing /b/build/slave/perf-go-webview-phone/build/src/out/Release/apks/Monochrome.apk on device if needed.


Therefore, I am assuming it is something in the recipe that I am missing.  

Stephen, Ben or John do any of you any insight into the recipe and why I would be not finding this browser?


Cc: eyaich@chromium.org
Owner: perezju@chromium.org
For Juan's case of internal clank waterfall, the name of the apk to use was Monochrome.apk

For us, it's MonochromePublic.apk. So I think we need to update Telemetry to look for either monochrome or monochromepublic.

Juan: can you help with this since you're most familiar with apk selection logic in Telemetry?
So, this is what each of our android --browser's install on the device, c.f. [1]

- android-chromium: 
  - ChromePublic.apk (always)

- android-chrome:
  - Monochrome.apk (N and above)
  - Chrome.apk (otherwise: pre N)

- android-webview:
  - SystemWebView.apk (in AOSP)
  - Monochrome.apk (N and above)
  - SystemWebViewGoogle.apk (othewise: pre N, non-AOSP)

I think that, similar to chrome/chromium, we should split webview into the "official" and "public" cases. So that instead each one installs:

- android-webview:
  - Monochrome.apk (N and above)
  - SystemWebViewGoogle.apk (othewise: pre N)

- android-webview-public:
  - MonochromePublic.apk (N and above)
  - SystemWebView.apk (othewise: pre N)

And switch all upstream webview bots to this new browser flag.

Alternatively we could use android-webview-google for the "offical" browser and keep android-webview for the "public" one.

Additionally, we should also probably make android-chromium install MonochromePublic.apk in N and above.

Does that sound right? Any preference on the names for webview browsers?

[1]: https://cs.chromium.org/chromium/src/third_party/catapult/telemetry/telemetry/internal/backends/android_browser_backend_settings.py

Comment 6 by eyaich@chromium.org, May 11 2018

Good catch Ned and thanks Juan.

I have a slight preference for android-webview-google and android-webview but that means we will have to break things for a bit while we update the new flag.
Cc: simonhatch@chromium.org
I think it's mostly doable without breaking things as follows:

1) CL to add android-webview-google with the new behavior documented in #5, existing android-webview still works the same as today.

2) Switch internal perf and bisect bots to explicitly use android-webview-google instead of android-webview.  --- +simonhatch FYI

3) CL to switch android-webview to the new behavior, i.e. install MonochromePublic.apk.

+ some announcement on Telemetry list to warn about the change.

I think the tricky bit might be internal webview bisects across the boundary. Simon, is there something we could do on that front?
I can probably hack something terrible in for the time-being to choose based on commit position. This is for recipe, right?
Project Member

Comment 9 by bugdroid1@chromium.org, May 14 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/77b812956a7220d25b76bc859aa2c5f26206602a

commit 77b812956a7220d25b76bc859aa2c5f26206602a
Author: Juan Antonio Navarro Perez <perezju@chromium.org>
Date: Mon May 14 16:11:26 2018

[Telemetry] Define --browser android-webview-google

This will be used to install:
- Monochrome.apk (on N and above)
- SystemWebViewGoogle.apk (otherwise)

Bug: chromium:841757
Change-Id: Iccd375ed69dc2eec0bf36d35399e9e14bb79a8b9
Reviewed-on: https://chromium-review.googlesource.com/1056988
Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Reviewed-by: Emily Hanley <eyaich@chromium.org>

[modify] https://crrev.com/77b812956a7220d25b76bc859aa2c5f26206602a/telemetry/telemetry/internal/backends/android_browser_backend_settings.py
[modify] https://crrev.com/77b812956a7220d25b76bc859aa2c5f26206602a/telemetry/telemetry/internal/backends/android_backend_settings_unittest.py

Project Member

Comment 10 by bugdroid1@chromium.org, May 14 2018

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

commit f65e1f5882596a8902409c58d297710284bd7f5f
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Mon May 14 21:24:43 2018

Roll src/third_party/catapult/ 195c52dc7..a67f1510e (6 commits)

https://chromium.googlesource.com/catapult.git/+log/195c52dc7098..a67f1510e710

$ git log 195c52dc7..a67f1510e --date=short --no-merges --format='%ad %ae %s'
2018-05-14 perezju [soundwave] Fetch bug data in parallel
2018-05-14 perezju [Telemetry] Define --browser android-webview-google
2018-05-14 simonhatch Dashboard - Strip dead code from testselection.html
2018-05-14 nednguyen Add script to update Telemetry binary
2018-05-10 perezju [soundwave] Create bugs table if needed
2018-05-11 simonhatch Dashboard - Remove internal_only field from Row

Created with:
  roll-dep src/third_party/catapult
BUG=chromium:841757


The AutoRoll server is located here: https://catapult-roll.skia.org

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

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


TBR=sullivan@chromium.org

Change-Id: Ia81ddecb305d9801c475b922f3d392884f208df6
Reviewed-on: https://chromium-review.googlesource.com/1057903
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#558465}
[modify] https://crrev.com/f65e1f5882596a8902409c58d297710284bd7f5f/DEPS

Owner: simonhatch@chromium.org
Re #8: Yes, this change is needed for internal recipe bisect.

From r558465 onward you can use --browser android-webview-google instead of --browser android-webview on webview bisect bots. (At the moment either of them work and do the same thing.)

Assigning to you Simon for that change. When ready send back to me so I can make the final change needed to give --browser android-webview it's new meaning.
Project Member

Comment 12 by bugdroid1@chromium.org, May 16 2018

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/e295d620dbcdd891a78fc6b52c2f5e9d30943902

commit e295d620dbcdd891a78fc6b52c2f5e9d30943902
Author: Juan Antonio Navarro Perez <perezju@chromium.org>
Date: Wed May 16 09:50:44 2018

Project Member

Comment 13 by bugdroid1@chromium.org, May 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/03db8a6e5bb1eabfcbd88becac24c967597ed5bb

commit 03db8a6e5bb1eabfcbd88becac24c967597ed5bb
Author: Juan Antonio Navarro Perez <perezju@chromium.org>
Date: Thu May 17 14:56:54 2018

[Telemetry] Fix expectations.ANDROID_WEBVIEW

Tests disabled on Android_Webview should be disabled on both:
- android-webview
- android-webview-google

Bug: chromium:841757
Change-Id: I68d7f84883539a9a53e16610b1c9fe36b0a26bfe
Reviewed-on: https://chromium-review.googlesource.com/1064231
Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Ned Nguyen <nednguyen@google.com>

[modify] https://crrev.com/03db8a6e5bb1eabfcbd88becac24c967597ed5bb/telemetry/telemetry/story/expectations_unittest.py
[modify] https://crrev.com/03db8a6e5bb1eabfcbd88becac24c967597ed5bb/telemetry/telemetry/story/expectations.py

Project Member

Comment 14 by bugdroid1@chromium.org, May 17 2018

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

commit 187539dde23ec9addc640a87b56988001de8b1be
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Thu May 17 16:22:55 2018

Roll src/third_party/catapult/ c594085d9..03db8a6e5 (1 commit)

https://chromium.googlesource.com/catapult.git/+log/c594085d996e..03db8a6e5bb1

$ git log c594085d9..03db8a6e5 --date=short --no-merges --format='%ad %ae %s'
2018-05-17 perezju [Telemetry] Fix expectations.ANDROID_WEBVIEW

Created with:
  roll-dep src/third_party/catapult
BUG=chromium:841757


The AutoRoll server is located here: https://catapult-roll.skia.org

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

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


TBR=sullivan@chromium.org

Change-Id: Id604c21c3502604503693bc3bb65c886e273031e
Reviewed-on: https://chromium-review.googlesource.com/1064008
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#559559}
[modify] https://crrev.com/187539dde23ec9addc640a87b56988001de8b1be/DEPS

Status: Assigned (was: Untriaged)
Ok I've got a hack in the recipe to swap out the name to the old name, which bots should be using the -google one now?
All WebView recipe based bots should use -google now if the commit position is 558465 or above.
Project Member

Comment 18 by bugdroid1@chromium.org, May 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/0745901e0114427752d027032417f453b53af6bb

commit 0745901e0114427752d027032417f453b53af6bb
Author: Simon <simonhatch@chromium.org>
Date: Fri May 18 14:52:33 2018

Bisect - Hack to change names for old revisions on webview.

TBR=dtu@chromium.org

Bug: 841757
Change-Id: I6b63ff63b920bf56215076e947c3c78aba029942
Recipe-Manual-Change: build_limited_scripts_slave
Reviewed-on: https://chromium-review.googlesource.com/1064731
Commit-Queue: Simon Hatch <simonhatch@chromium.org>
Reviewed-by: David Tu <dtu@chromium.org>
Reviewed-by: Simon Hatch <simonhatch@chromium.org>

[modify] https://crrev.com/0745901e0114427752d027032417f453b53af6bb/scripts/slave/recipe_modules/auto_bisect/revision_state.py

Project Member

Comment 19 by bugdroid1@chromium.org, May 21 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/5ee9829622bcb6b8d95602f05b490c175e457b27

commit 5ee9829622bcb6b8d95602f05b490c175e457b27
Author: Juan Antonio Navarro Perez <perezju@chromium.org>
Date: Mon May 21 09:34:40 2018

[Telemetry] Update meaning of --browser android-webview

This will now use:
- SystemWebView.apk (pre-N)
- MonochromePublic.apk (N and above)

Use --browser android-webview-google instead if you want to test the
"official" Google builds.

Bug: chromium:841757
Change-Id: I2930c2674259708cfc931c845d5a3518bd65ecbe
Reviewed-on: https://chromium-review.googlesource.com/1065735
Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>

[modify] https://crrev.com/5ee9829622bcb6b8d95602f05b490c175e457b27/telemetry/telemetry/internal/backends/android_browser_backend_settings.py
[modify] https://crrev.com/5ee9829622bcb6b8d95602f05b490c175e457b27/telemetry/telemetry/internal/backends/android_backend_settings_unittest.py

Owner: eyaich@chromium.org
Back to you Emily. I believe this _should_ be trying to install the correct MonochromePublic.apk now. Let me know if it doesn't.
Project Member

Comment 21 by bugdroid1@chromium.org, May 21 2018

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

commit 3a21271cb39d94f36755c9d6aee42a7ebd9eb41a
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Mon May 21 10:44:53 2018

Roll src/third_party/catapult/ 1d08c644f..5ee982962 (1 commit)

https://chromium.googlesource.com/catapult.git/+log/1d08c644fb9a..5ee9829622bc

$ git log 1d08c644f..5ee982962 --date=short --no-merges --format='%ad %ae %s'
2018-05-18 perezju [Telemetry] Update meaning of --browser android-webview

Created with:
  roll-dep src/third_party/catapult
BUG=chromium:841757


The AutoRoll server is located here: https://catapult-roll.skia.org

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

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


TBR=sullivan@chromium.org

Change-Id: I792fdb841da0516a363100591d96d259d80367a0
Reviewed-on: https://chromium-review.googlesource.com/1066075
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#560256}
[modify] https://crrev.com/3a21271cb39d94f36755c9d6aee42a7ebd9eb41a/DEPS

We are still unable to find the browser on the pixel2: 

See failing shard here:
https://chrome-swarming.appspot.com/task?id=3d9b17d4b2de3c10&refresh=10&show_raw=1

I see an info warning :

(INFO) 2018-05-21 10:39:00,907 android_browser_backend_settings.FindLocalApk:76  Picked apk name Monochrome.apk for browser_type android-webview

Should that be MonocrhomePublic.apk now?


Emily: it gonna take some times for Juan's change to take effect in perf waterfall, so I think we should check again tomorrow
Project Member

Comment 24 by bugdroid1@chromium.org, May 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/3c7c79e55318c5ded6a2a45906a355230c7eb24b

commit 3c7c79e55318c5ded6a2a45906a355230c7eb24b
Author: John Budorick <jbudorick@chromium.org>
Date: Wed May 23 03:02:47 2018

Revert "Bisect - Hack to change names for old revisions on webview."

This reverts commit 0745901e0114427752d027032417f453b53af6bb.

Reason for revert: Breaking the roll due to being incompatible with the downstream recipe for which this hack is intended.

Original change's description:
> Bisect - Hack to change names for old revisions on webview.
> 
> TBR=dtu@chromium.org
> 
> Bug: 841757
> Change-Id: I6b63ff63b920bf56215076e947c3c78aba029942
> Recipe-Manual-Change: build_limited_scripts_slave
> Reviewed-on: https://chromium-review.googlesource.com/1064731
> Commit-Queue: Simon Hatch <simonhatch@chromium.org>
> Reviewed-by: David Tu <dtu@chromium.org>
> Reviewed-by: Simon Hatch <simonhatch@chromium.org>

TBR=dtu@chromium.org,simonhatch@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 841757
Change-Id: I17ed62f520e4a7824ec46fe0b51a98f7f278242e
Reviewed-on: https://chromium-review.googlesource.com/1068690
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: John Budorick <jbudorick@chromium.org>

[modify] https://crrev.com/3c7c79e55318c5ded6a2a45906a355230c7eb24b/scripts/slave/recipe_modules/auto_bisect/revision_state.py

Cc: dtu@chromium.org
Hmm.. the revert in #24 would leave us without clank bisects for webview. Since Simon is OOO, +dtu could you try to give a stab at fixing and relanding that CL?

(Otherwise if it's too complicated I guess we'll just wait until Simon and me we're both back.)

Sign in to add a comment