Intermittent /css/cssom-view/elementScroll.html | Element scrollBy test (one argument) - assert_equals: increment of scrollTop should be 15 expected 90 but got 89

RESOLVED FIXED in Firefox 69

Status

()

defect
P5
normal
RESOLVED FIXED
3 months ago
2 months ago

People

(Reporter: intermittent-bug-filer, Assigned: hiro)

Tracking

(Regression, {intermittent-failure, regression})

unspecified
mozilla69
Points:
---

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox67 unaffected, firefox68 unaffected, firefox69 fixed)

Details

Attachments

(1 attachment)

Filed by: aciure [at] mozilla.com
Parsed log: https://treeherder.mozilla.org/logviewer.html#?job_id=248072946&repo=mozilla-central
Full log: https://queue.taskcluster.net/v1/task/KO0CmD5NT9e-axyIuYpBOQ/runs/0/artifacts/public/logs/live_backing.log


[task 2019-05-23T23:21:14.924Z] 23:21:14 INFO - TEST-PASS | /css/cssom-view/elementScroll.html | Element scrollBy test (two arguments)
[task 2019-05-23T23:21:14.924Z] 23:21:14 INFO - TEST-UNEXPECTED-FAIL | /css/cssom-view/elementScroll.html | Element scrollBy test (one argument) - assert_equals: increment of scrollTop should be 15 expected 90 but got 89
[task 2019-05-23T23:21:14.925Z] 23:21:14 INFO - window.onload/<@http://web-platform.test:8000/css/cssom-view/elementScroll.html:137:13
[task 2019-05-23T23:21:14.925Z] 23:21:14 INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1587:25
[task 2019-05-23T23:21:14.925Z] 23:21:14 INFO - test@http://web-platform.test:8000/resources/testharness.js:544:30
[task 2019-05-23T23:21:14.925Z] 23:21:14 INFO - window.onload@http://web-platform.test:8000/css/cssom-view/elementScroll.html:128:9
[task 2019-05-23T23:21:15.146Z] 23:21:15 INFO - .
[task 2019-05-23T23:21:15.146Z] 23:21:15 INFO - TEST-OK | /css/cssom-view/elementScroll.html | took 600ms
[task 2019-05-23T23:21:15.887Z] 23:21:15 INFO - Closing logging queue
[task 2019-05-23T23:21:15.887Z] 23:21:15 INFO - queue closed
[task 2019-05-23T23:21:15.903Z] 23:21:15 INFO - Setting up ssl
[task 2019-05-23T23:21:15.919Z] 23:21:15 INFO - certutil |
[task 2019-05-23T23:21:15.934Z] 23:21:15 INFO - certutil |
[task 2019-05-23T23:21:15.950Z] 23:21:15 INFO - certutil |
[task 2019-05-23T23:21:15.950Z] 23:21:15 INFO - Certificate Nickname Trust Attributes
[task 2019-05-23T23:21:15.950Z] 23:21:15 INFO - SSL,S/MIME,JAR/XPI
[task 2019-05-23T23:21:15.950Z] 23:21:15 INFO -
[task 2019-05-23T23:21:15.950Z] 23:21:15 INFO - web-platform-tests CT,,
[task 2019-05-23T23:21:15.950Z] 23:21:15 INFO -
[task 2019-05-23T23:21:17.679Z] 23:21:17 INFO - adb Granting important runtime permissions to org.mozilla.geckoview.test
[task 2019-05-23T23:21:18.928Z] 23:21:18 INFO - adb launch_application: am start -W -n org.mozilla.geckoview.test/org.mozilla.geckoview.test.TestRunnerActivity -a android.intent.action.MAIN --es env9 MOZ_PROCESS_LOG=/tmp/tmp5gTuGtpidlog --es env8 MOZ_CRASHREPORTER_NO_REPORT=1 --es args "-no-remote -profile /sdcard/tests/profile --marionette about:blank" --es env3 STYLO_THREADS=4 --es env2 MOZ_HIDE_RESULTS_TABLE=1 --es env1 R_LOG_VERBOSE=1 --es env0 MOZ_CRASHREPORTER=1 --es env7 MOZ_DISABLE_NONLOCAL_CONNECTIONS=1 --es env6 R_LOG_DESTINATION=stderr --es env5 MOZ_CRASHREPORTER_SHUTDOWN=1 --es env4 MOZ_LOG=signaling:3,mtransport:4,DataChannel:4,jsep:4 --ez use_multiprocess True --es env10 R_LOG_LEVEL=6
[task 2019-05-23T23:21:20.768Z] 23:21:20 INFO - Starting runner

Flags: needinfo?(hikezoe)
Regressed by: 1553022

Sure.

Assignee: nobody → hikezoe
Status: NEW → ASSIGNED
Flags: needinfo?(hikezoe)

Hmm, weird. I can't reproduce the failure locally at all. Initially I tried on android-emulator-x86, but after that, I noticed our CI uses android-emulator-x86-64, so I also tried on the 64bits emulator, but can't reproduce either. Then I suspected that the device screen size is not the same as CI's, but as per a log in our CI, it's exactly the same.

hw.lcd.width = 800
hw.lcd.height = 1280
hw.lcd.depth = 16
hw.lcd.density = 320

I can replicate the failure on local linux box with layout.css.devPixelsPerPx=1.61. And it turns out it's an underlying issue. And there is another issue that I just filed (bug 1554604).

Anyway, the problem here is;

  1. We store the scroll position as nsPoint
  2. We clamp the scroll position to the layer pixels in ClampAndAlignWithLayerPixels
  3. We get the current scroll position in app units in ScrollByCSSPixels and add the given delta value to the current value in app units

So, it's possible that the added value is slightly far from the ideal position in CSS pixels, and also it's possible that the added value is clamped to positions where it's farther from the ideal position in ClampAndAlignWithLayerPixels.

To prevent this calculation error, we should get the current position in CSS pixels in ScrollByCSSPixels.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8b9ff110c940cc7d40db862fa6e8e29ca40fecac

The try looks good. The test in question in wpt5, all wpt5s are orange but it's due to bug 1551382.

Otherwise clamped positions in layer pixels might cause 1-pixel difference
in CSS pixels on Android.

See Also: → 1554024
See Also: → 1554023

For future references;

Though this failure caused by bug 1553022, but the real cause is bug 1556685 which is a pre-existing issue on the layer-pixel alignment, and the reason why we hadn't realized it is that we've been incorrectly applying the layer-pixel alignment (bug 1556683) on Fennec, to be more precise on non-E10S.

Also note that I've noticed that window.scrollBy clamps the given double value in CSS pixels, but it doesn't use NStoIntRound() so that it might be possible there are some edge cases where the clamped value is different from what element.scrollBy does.

See Also: → 1556685
Pushed by hikezoe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bdb43da73c35
Use the current position in CSS pixels for the start point of ScrollBy. r=botond,mstange
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
You need to log in before you can comment on or make changes to this bug.