New issue
Advanced search Search tips

Issue 875859 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 24
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocking:
issue 853794



Sign in to add a comment

youtube_pinch_2018 consistently failing on N5X and pixel2 bots

Project Member Reported by perezju@chromium.org, Aug 20

Issue description

Stories:

smoothness.tough_pinch_zoom_cases/youtube_pinch_2018
smoothness.gpu_rasterization.tough_pinch_zoom_cases/youtube_pinch_2018

Are consistently failing on N5X failing as of:
https://ci.chromium.org/buildbot/chromium.perf/android-nexus5x-perf/227

Also failing on android-pixel2_webview-perf and android-pixel2-perf bots.

Proceeding to disable and kick off functional bisect.

+cc'ing benchmark owners
 
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 20

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

commit 076b591c55fcefba3e4f947da12e3d8e5a565e28
Author: Juan Antonio Navarro Perez <perezju@chromium.org>
Date: Mon Aug 20 14:46:02 2018

[tools/perf] Disable youtube_pinch_2018 in smoothness benchmarks

Stories have been consinstently failing on N5X and Pixel 2 bots.

TBR=nednguyen@google.com
NOTRY=true

Bug:  875859 
Change-Id: I9d41fa293a90e6961eabc6b29711d8b824d469ce
Reviewed-on: https://chromium-review.googlesource.com/1181140
Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584440}
[modify] https://crrev.com/076b591c55fcefba3e4f947da12e3d8e5a565e28/tools/perf/expectations.config

Cc: fhorschig@chromium.org
Owner: fhorschig@chromium.org
Status: Assigned (was: Untriaged)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/16d7bb9e640000

[Android] Make keyboard accessory views independent from BottomContainer by fhorschig@chromium.org
https://chromium.googlesource.com/chromium/src/+/f7f385986a69d648d77b0b9f7821e7901df5c90f
0 → 1 (+1)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Labels: -Pri-3 Pri-2
fhorschig@ it looks like your CL caused one of the user stories in smoothness benchmarks to fail.

After your CL the story fails with a timeout waiting for window.__pinchActionDone to be true.

The relevant story code where the pinch action is started is here:
https://cs.chromium.org/chromium/src/tools/perf/page_sets/rendering/tough_pinch_zoom_cases.py?rcl=8a43f14b01b3c9663cc07e475e5dcd0401ae9004&l=39

And the corresponding pinch action is implemented in these:
https://cs.chromium.org/chromium/src/third_party/catapult/telemetry/telemetry/internal/actions/pinch.py
https://cs.chromium.org/chromium/src/third_party/catapult/telemetry/telemetry/internal/actions/pinch.js

Status: Started (was: Assigned)
I am having a look.
Blocking: 853794
Components: UI>Browser>Passwords
Labels: OS-Android
Can confirm the regression: the issue is a keyboard popping open because a search field is focused when starting up youtube in the test. Details in the linked CL.

It's easier to fix than to revert: https://crrev.com/c/1184847
(Still checking some interactions but it looks like this undoes the regression.)
Awesome, thanks for quickly diagnosing the issue!

After the fix lands you can assign back to me to get the test re-enabled again.
Project Member

Comment 9 by bugdroid1@chromium.org, Aug 23

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

commit 760cc2256b4d27f9a01c6102a3392f7840df7a4f
Author: Friedrich Horschig <fhorschig@chromium.org>
Date: Thu Aug 23 12:01:46 2018

[Android][Performance] Prevent accessory from over-openening keyboard

This CL prevents the keyboard accessory from triggering a keyboard that
hasn't been requested.

Example: Open youtube or chrome://flags
Before this patch:
  The keyboard accessory controller detects that there are passwords for
  this site. Because the search field is focused by default, it will
  request openening the keyboard.
Why this is wrong:
  The controller wrongfully assumed that a focused input field always
  triggers the keyboard. This is wrong, esp. for pageloads.
After this patch:
  The controller requests to open the keyboard iff the accessory sheet
  is opened.

Why has this become a problem now?
Before https://crrev.com/c/1177361, the controller would just request
the accessory to be closed. This caused nasty artifacts and bugs.
Therefore, it requested to open keyboard to dismiss the sheet gently.
This resulted in opening the keyboard too often.

What is potentially impacted by this:
 - Performance tests with search bars: Confirmed
   --> smoothness.*/youtube_pinch_2018 not failing consistently
 - Health tests: Likely confirmed
   system_health.memory_mobile/browse.tools.maps debug run changed
   memory:chrome:all_processes:reported_by_chrome:effective_size
   by 7.7%
 - Startup with previously opened sites: Confirmed
   Following instructions in issue 876228 don't reproduce error.

Bug:  875859 , 876025, 876228
Change-Id: Ic5873c07cb8652bc0da5edcca8324f2680baac11
Reviewed-on: https://chromium-review.googlesource.com/1184847
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Friedrich Horschig [CEST] <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585454}
[modify] https://crrev.com/760cc2256b4d27f9a01c6102a3392f7840df7a4f/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingCoordinator.java
[modify] https://crrev.com/760cc2256b4d27f9a01c6102a3392f7840df7a4f/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/ManualFillingMediator.java
[modify] https://crrev.com/760cc2256b4d27f9a01c6102a3392f7840df7a4f/chrome/android/java/src/org/chromium/chrome/browser/autofill/keyboard_accessory/PasswordAccessoryBridge.java
[modify] https://crrev.com/760cc2256b4d27f9a01c6102a3392f7840df7a4f/chrome/browser/android/password_manager/password_accessory_view_android.cc
[modify] https://crrev.com/760cc2256b4d27f9a01c6102a3392f7840df7a4f/chrome/browser/android/password_manager/password_accessory_view_android.h
[modify] https://crrev.com/760cc2256b4d27f9a01c6102a3392f7840df7a4f/chrome/browser/password_manager/password_accessory_controller.cc
[modify] https://crrev.com/760cc2256b4d27f9a01c6102a3392f7840df7a4f/chrome/browser/password_manager/password_accessory_controller_unittest.cc
[modify] https://crrev.com/760cc2256b4d27f9a01c6102a3392f7840df7a4f/chrome/browser/password_manager/password_accessory_view_interface.h

Owner: perezju@chromium.org
Status: Assigned (was: Started)
I hope that fixed it, if not, please just assign back to me.
Thanks for taking care of the rest!
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 23

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

commit c1f7098f68d058afae0d36c52af1eba29d74fadb
Author: Juan Antonio Navarro Pérez <perezju@chromium.org>
Date: Thu Aug 23 13:14:22 2018

Revert "[tools/perf] Disable youtube_pinch_2018 in smoothness benchmarks"

This reverts commit 076b591c55fcefba3e4f947da12e3d8e5a565e28.

Reason for revert: Root cause has been fixed.

Original change's description:
> [tools/perf] Disable youtube_pinch_2018 in smoothness benchmarks
> 
> Stories have been consinstently failing on N5X and Pixel 2 bots.
> 
> TBR=nednguyen@google.com
> NOTRY=true
> 
> Bug:  875859 
> Change-Id: I9d41fa293a90e6961eabc6b29711d8b824d469ce
> Reviewed-on: https://chromium-review.googlesource.com/1181140
> Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
> Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#584440}

TBR=perezju@chromium.org,nednguyen@google.com

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

Bug:  875859 
Change-Id: Id8caf496237be5e80d9d0d6dbe36554e722f6689
Reviewed-on: https://chromium-review.googlesource.com/1186481
Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585463}
[modify] https://crrev.com/c1f7098f68d058afae0d36c52af1eba29d74fadb/tools/perf/expectations.config

Status: Fixed (was: Assigned)
Thanks! This looks fixed now.

Sign in to add a comment