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

Issue 664515 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug

Blocking:
issue 664505



Sign in to add a comment

System health common crashing on browse:social:twitter

Project Member Reported by perezju@chromium.org, Nov 11 2016

Issue description

The browse:social:twitter story appears to be crashing somewhat consistently at least on:

- Android Nexus5 Perf (2)
- Android Nexus5X WebView Perf (2)
- Android Nexus6 WebView Perf (1)

Crash logs coming soon.
 
https://uberchromegw.corp.google.com/i/chromium.perf/builders/Android%20Nexus7v2%20Perf%20%282%29/builds/3244/steps/system_health.common_mobile/logs/stdio

(test only appears to flake on this bot, fails about 40% of the time)
twitter_nexus7v2.log
219 KB View Download
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 11 2016

Cc: hpayer@chromium.org
Labels: Hotlist-MemoryInfra
Owner: hjd@chromium.org
hjd@ I really need your help here! 

Comment 7 by hjd@chromium.org, Nov 22 2016

Status: Started (was: Untriaged)

Comment 8 by hjd@chromium.org, Nov 22 2016

hmm, interesting...

Attempted to repo locally but it worked for me.

Two of those logs (nexus5x, nexus6) include stack traces like this:
content::SyntheticGestureTargetBase::DispatchInputEventToPlatform(...)
Which is the same stack trace you get if you attempt to run a benchmark when the
device screen is turned off.
Is it possible the twitter stories were shared to a device in a bad state? (is that even how sharding works?)

The other two have stack traces in v8/src/runtime/runtime-simd.cc not sure if that is relevant or not.

Attempting a try job here to see what errors we get:
https://codereview.chromium.org/2522593007

Comment 9 by hjd@chromium.org, Nov 23 2016

So of those builds:
2 successes
* android_one_perf_bisect
* android_nexus9_perf_bisect

1 unrelated failure (failed both with and without the patch)
* android_s5_perf_bisect

3 failures in v8/src/runtime/runtime-simd.cc
* android_nexus6_perf_bisect
* android_nexus5_perf_bisect
* android_fyi_perf_bisect

4 failures in content::SyntheticGestureTargetBase::DispatchInputEventToPlatform
* android_nexus5X_perf_bisect
* android_webview_arm64_aosp_perf_bisect
* android_webview_nexus6_aosp_perf_bisect
* staging_android_nexus5X_perf_bisect

Logging into the bots the devices do seem to be awake.

Comment 10 by hjd@chromium.org, Nov 23 2016

Okay managed to repo locally the content::SyntheticGestureTargetBase::DispatchInputEventToPlatform failures are caused by trying to click on an element that isn't in the viewport. 4/10ish repo rate. I'm guessing this happens because a signin banner is added in javascript in between calculating the scroll amount and actually scrolling? I have added a wait in between hitting back and scrolling the main page which seem to fix this locally, I will run a try job on this change.

Comment 11 by hjd@chromium.org, Nov 23 2016

Narrowed down the issue, it isn't that the TapElement is out of bounds it is that the scroll gesture (https://codesearch.chromium.org/chromium/src/tools/perf/page_sets/system_health/browsing_stories.py?l=34) is out of bounds.

That scroll calls to here:
https://cs.corp.google.com/github/catapult-project/catapult/telemetry/telemetry/internal/actions/scroll_to_element.py?l=65
Which calls:
https://cs.corp.google.com/github/catapult-project/catapult/telemetry/telemetry/internal/actions/scroll.py?l=103
Which calls:
https://cs.corp.google.com/github/catapult-project/catapult/telemetry/telemetry/internal/actions/scroll.js?l=128
Which calls:
https://cs.corp.google.com/github/catapult-project/catapult/telemetry/telemetry/internal/actions/gesture_common.js?l=53

We calculate the middle of the selected element to scroll on but because getBoundingVisibleRect sometimes returns a negative number for height this calculation gives an out of bounds position.

In this case:
getBoundingRect() = {left: 0, top: -906, width: 412, height: 604}
so:
rect.height += rect.top;
gives:
rect.height = -302;

I'm not sure if this is the right behaviour for getBoundingVisibleRect, I guess not?



Cc: nednguyen@chromium.org
Sounds like it should be:

    if (rect.top < 0) {
      rect.height += rect.top;
      rect.top = 0;
      if (rect.height < 0) {
        rect.height = 0;  // the whole of the element is out of screen.
      }
    }

And something similar for the left/width calculation below.

Still, I'm baffled as to why a js error here causes Chrome to crash?
#12: I think it's because there are a bunch of CHECKS() in the synthetic input code that ensure we're not trying to send events outside the screen.
Cc: rnep...@chromium.org
Project Member

Comment 15 by bugdroid1@chromium.org, Nov 25 2016

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

commit c60f3321df06eb2945441f7953e1747efadb943a
Author: catapult-deps-roller <catapult-deps-roller@chromium.org>
Date: Fri Nov 25 00:25:59 2016

Roll src/third_party/catapult/ 3276375d2..671a65473 (7 commits).

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/3276375d2563..671a654736c2

$ git log 3276375d2..671a65473 --date=short --no-merges --format='%ad %ae %s'
2016-11-24 simonhatch Bisect - Fix compare_samples to read scalar values.
2016-11-24 hjd Fix getBoundingVisibleRect with offscreen elements
2016-11-24 ulan Helper functions for computing expected queueing time.
2016-11-23 achuith Autotest extension fix.
2016-11-23 benjhayden Document customizeSummaryOptions in how-to-write-metrics.
2016-11-23 fmeawad runtime_stats metric: only display the average in the dashboard
2016-11-23 simonhatch [bisect] - Clearly state bisect type in output.

BUG= 668536 , 664515 ,625701, 654525 , 667813 

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=catapult-sheriff@chromium.org

Review-Url: https://codereview.chromium.org/2530973002
Cr-Commit-Position: refs/heads/master@{#434413}

[modify] https://crrev.com/c60f3321df06eb2945441f7953e1747efadb943a/DEPS

Comment 16 by hjd@chromium.org, Dec 6 2016

Status: Fixed (was: Started)
thanks! did you re-enable the stories in #5?
Or is something else required?

Comment 18 by hjd@chromium.org, Dec 6 2016

Re-enabling happened in crrev.com/2522593007 I managed to mess up the bug on that CL though so it didn't get linked :(
Project Member

Comment 20 by bugdroid1@chromium.org, Dec 6 2016

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

commit d8bf23963e9c7f9df10c01771192c6608dcff5cf
Author: hjd <hjd@chromium.org>
Date: Tue Dec 06 12:41:51 2016

Re-enable browse:social:twitter smoke test

BUG= 664515 
R=nednguyen

Review-Url: https://codereview.chromium.org/2552153004
Cr-Commit-Position: refs/heads/master@{#436579}

[modify] https://crrev.com/d8bf23963e9c7f9df10c01771192c6608dcff5cf/tools/perf/benchmarks/system_health_smoke_test.py

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

commit cb34998d5bc07f8dbbf9dfdce122eafe7cf6c9f3
Author: hjd <hjd@chromium.org>
Date:   Fri Nov 25 15:27:42 2016

Reenable system_health browse:social:twitter story

BUG= chromium:259147 

Review-Url: https://codereview.chromium.org/2522593007
Cr-Commit-Position: refs/heads/master@{#434507}

(Your friendly primiano-droid :P)
Components: Internals>Instrumentation>Memory
Project Member

Comment 23 by bugdroid1@chromium.org, Jan 17 2017

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

commit b7e693f47396d9d20d205e8e1a53736d4bb1aa0b
Author: hjd <hjd@chromium.org>
Date: Tue Jan 17 11:19:51 2017

Stop browse:social:twitter scrolling wrong element

At the moment the browse:social:twitter system health story
thinks that it is scrolling the body element but is actually
scrolling something else (since the body element is off-screen
its bounding rect is clipped to have zero height so we scroll
(width/2, 0/2) which is some other div). This happens to work
but is unexpected. We want to make scrolling a zero area div
an error however that would break this story so we first need
to update the story to use a manually specified container.

Depends on: https://codereview.chromium.org/2617503002/

BUG= chromium:664515 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq

Review-Url: https://codereview.chromium.org/2612053002
Cr-Commit-Position: refs/heads/master@{#444024}

[modify] https://crrev.com/b7e693f47396d9d20d205e8e1a53736d4bb1aa0b/tools/perf/page_sets/system_health/browsing_stories.py

Project Member

Comment 24 by bugdroid1@chromium.org, Jan 19 2017

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

commit 6ab584fe173d30622049bf8b0c230384b10b41c1
Author: catapult-deps-roller <catapult-deps-roller@chromium.org>
Date: Thu Jan 19 15:36:50 2017

Roll src/third_party/catapult/ cfcae9b97..1e05d2f84 (15 commits).

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/cfcae9b972f2..1e05d2f8401c

$ git log cfcae9b97..1e05d2f84 --date=short --no-merges --format='%ad %ae %s'
2017-01-19 hjd Revert of [telemetry] Bullet proof scroll.js (patchset #4 id:60001 of https://codereview.chromium.org/2532443002/ )
2017-01-19 perezju [catapult] Remove Telemetry's android_command_line_backend
2017-01-18 dtu [pinpoint] Config changes to make /new work.
2017-01-18 dtu [pinpoint] Remove "blocked" from Attempt and Execution.
2017-01-18 benjhayden Translate Statistics methods to python.
2017-01-18 dtu [pinpoint] Centralize failure and completion handling in Executions.
2017-01-18 benjhayden Translate PercentToString to python.
2017-01-18 benjhayden Translate RunningStatistics to python.
2017-01-18 benjhayden Translate Range to python.
2017-01-18 jreck Make extract_thread_list slightly smarter
2017-01-18 jessimb Fixed alert count to match display.
2017-01-18 simonhatch This avoids getting into weird circular situations like A dupes on B which duped against A already. Kept it simple, if the other bug is already a duplicate, don't merge. Still cc the author and culprit though.
2017-01-18 perezju [telemetry] Provide new API to evaluate JavaScript code
2017-01-18 perezju [devil] Upgrade parser/serializer for command line flags
2017-01-18 hjd [telemetry] Bullet proof scroll.js

BUG= 664515 , 679814 , 664515 

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=catapult-sheriff@chromium.org

Review-Url: https://codereview.chromium.org/2642083002
Cr-Commit-Position: refs/heads/master@{#444740}

[modify] https://crrev.com/6ab584fe173d30622049bf8b0c230384b10b41c1/DEPS

Sign in to add a comment