Perma Windows 7 debug Assertion failure: (aA != 9223372036854775807i64 || aB != (-9223372036854775807i64 - 1) when Gecko 70 merges to Beta on 2019-08-26
Categories
(Core :: DOM: Animation, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox-esr68 | --- | unaffected |
firefox68 | --- | unaffected |
firefox69 | --- | unaffected |
firefox70 | --- | fixed |
firefox71 | --- | verified |
People
(Reporter: CosminS, Assigned: birtles)
References
(Regression)
Details
(Keywords: regression)
Attachments
(2 files, 1 obsolete file)
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
patch
|
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
[Tracking Requested - why for this release]:
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=264489407&repo=try&lineNumber=10240
central rev: https://hg.mozilla.org/mozilla-central/rev/c75d6a0539eb4b2c7961f0c9782fcb364198c6b2
central rev: https://hg.mozilla.org/mozilla-central/rev/993cf82f6bbf09537f4cec8144cb69bfdf26bff3
I assumed this to be fixed by this wpt update: https://hg.mozilla.org/try/rev/6fdb1282586def80e1a0d8c4c85f84e44a860b66 but wasn't and went on starred as an intermittent against bug 1271788.
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Comment 1•5 years ago
|
||
From the wpt side I think we can mark the crash as expected, which might or might not be enough to turn the job green, depending on whether the failing assert independently triggers failure. If that doesn't work then we can disable the test.
But really we want a layout developer to look at this and fix the underlying issue.
Assignee | ||
Comment 2•5 years ago
|
||
I have this bugmail sitting in my inbox to remind to look into this. Hopefully this week.
Comment hidden (Intermittent Failures Robot) |
Comment 4•5 years ago
|
||
Just got similar errors when I push to try for offset-path
:
PROCESS-CRASH | /css/motion/animation/offset-path-interpolation-001.html | application crashed [@ mozilla::TimingParams::Update()]
PID 5696 | Assertion failure: (aA != 9223372036854775807i64 || aB != (-9223372036854775807i64 - 1)) && (aA != (-9223372036854775807i64 - 1) || aB != 9223372036854775807i64) ('Infinity + -Infinity' and '-Infinity + Infinity' are undefined), at z:/build/build/src/obj-firefox/dist/include\mozilla/StickyTimeDuration.h:35
PROCESS-CRASH | /css/motion/animation/offset-path-interpolation-002.html | application crashed [@ mozilla::TimingParams::Update()]
PID 3924 | Assertion failure: (aA != 9223372036854775807i64 || aB != (-9223372036854775807i64 - 1)) && (aA != (-9223372036854775807i64 - 1) || aB != 9223372036854775807i64) ('Infinity + -Infinity' and '-Infinity + Infinity' are undefined), at z:/build/build/src/obj-firefox/dist/include\mozilla/StickyTimeDuration.h:35
PROCESS-CRASH | /css/motion/animation/offset-path-interpolation-003.html | application crashed [@ mozilla::TimingParams::Update()]
PID 6124 | Assertion failure: (aA != 9223372036854775807i64 || aB != (-9223372036854775807i64 - 1)) && (aA != (-9223372036854775807i64 - 1) || aB != 9223372036854775807i64) ('Infinity + -Infinity' and '-Infinity + Infinity' are undefined), at z:/build/build/src/obj-firefox/dist/include\mozilla/StickyTimeDuration.h:35
PROCESS-CRASH | /css/motion/animation/offset-path-interpolation-004.html | application crashed [@ mozilla::TimingParams::Update()]
PID 5324 | Assertion failure: (aA != 9223372036854775807i64 || aB != (-9223372036854775807i64 - 1)) && (aA != (-9223372036854775807i64 - 1) || aB != 9223372036854775807i64) ('Infinity + -Infinity' and '-Infinity + Infinity' are undefined), at z:/build/build/src/obj-firefox/dist/include\mozilla/StickyTimeDuration.h:35
PROCESS-CRASH | /css/motion/animation/offset-path-interpolation-005.html | application crashed [@ mozilla::TimingParams::Update()]
TEST-UNEXPECTED-CRASH | /css/motion/animation/offset-path-interpolation-005.html | expected OK
PID 5824 | Assertion failure: (aA != 9223372036854775807i64 || aB != (-9223372036854775807i64 - 1)) && (aA != (-9223372036854775807i64 - 1) || aB != 9223372036854775807i64) ('Infinity + -Infinity' and '-Infinity + Infinity' are undefined), at z:/build/build/src/obj-firefox/dist/include\mozilla/StickyTimeDuration.h:35
PROCESS-CRASH | /css/motion/animation/offset-rotate-interpolation.html | application crashed [@ mozilla::TimingParams::Update()]
Comment hidden (Intermittent Failures Robot) |
Comment 6•5 years ago
|
||
Brian, can you take a look at this in the coming days, please?
Assignee | ||
Comment 7•5 years ago
|
||
Yes, it's on my list. I'm currently traveling but hope to have a chance to look at this on Friday.
Assignee | ||
Comment 8•5 years ago
|
||
Looking into this as part of bug 1576866 it seems that even if we fix the assertions, these tests will still fail. Instead we should fix the test suite.
Assignee | ||
Comment 9•5 years ago
|
||
Assignee | ||
Comment 10•5 years ago
|
||
(In reply to Brian Birtles (:birtles) from comment #9)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=93291433210584d1b9ab92c0d39962d4cfebe758
There are a few failures there on Windows (which don't reproduce locally for me on a debug Windows build). Interesting they all occur on the Web Animations interpolation test at 0.5. Looking at the code we have:
var animation = target.animate(keyframes, {
fill: 'forwards',
duration: 1,
easing: createEasing(at),
});
animation.pause();
animation.currentTime = 0.5;
And createEasing
has:
function createEasing(y) {
if (y == 0) {
return 'steps(1, end)';
}
if (y == 1) {
return 'steps(1, start)';
}
if (y == 0.5) {
return 'steps(2, end)';
}
// Approximate using a bezier.
var b = (8 * y - 1) / 6;
return 'cubic-bezier(0, ' + b + ', 1, ' + b + ')';
}
Judging by the nature of the test we are hitting the steps(2, end)
case but then presumably there is some floating-point inaccuracy that means when we come to sample the timing function at 0.5 we land just before the cliff and end up sampling the lower value.
This seems like a pretty flaky way to write a test, so we should either:
- use
steps(3, jump-none)
(not widely enough supported yet?), or - drop the special case for 0.5 (is it needed?), or
- just use
linear
.
The latter seems the safest so I will just try that.
Assignee | ||
Comment 11•5 years ago
|
||
Yet again js-format / clang-format completely destroyed my repository when working with mercurial queues. And after disabling them hg qpush
just freezes. You'd think I'd learn. 3 hours lost so far this month to those tools.
Assignee | ||
Comment 12•5 years ago
|
||
Turns out that fix produces a flood of unexpected passes...
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ebb824488dbeea397ca4f82968bd5191d17cbf6c
Assignee | ||
Comment 13•5 years ago
|
||
The border-image-slice-interpolation.html
test still fails but precisely because it is testing at the 50% mark for a discrete animation. There's just some floating point precision issue there. It might be better to make the Web Animations one use a duration of 100s and seek to 50s rather than 1s and seek to 0.5s.
Assignee | ||
Comment 14•5 years ago
|
||
(In reply to Brian Birtles (:birtles) from comment #13)
It might be better to make the Web Animations one use a duration of 100s and seek to 50s rather than 1s and seek to 0.5s.
Actually, it uses a duration of 1ms seeks to 0.5ms.
The alternative would be to simply make the expectFlip
function test at 0.501 instead of precisely at 0.5.
I'm currently running both through try but I'll stick to the former if it works.
Assignee | ||
Comment 15•5 years ago
|
||
So many unexpected passes taking so long to identify and re-annotate. Seems like something in our process let us down here.
Assignee | ||
Comment 16•5 years ago
|
||
This patch fixes a number of issues with the common interpolation testing
functions upstreamed from Blink.
Firstly this test file sets an animation duration of 2,000,000,000s and a delay
of -1,000,000,000s but that does not appear to be necessary since no time
elapses between when the animation is established and when the result is
checked (measure() is called immediately after interpolate() without any
interleaving call to requestAnimationFrame, for example).
Furthermore, this long time will cause some configurations of Firefox on
Windows to treat the time as an infinite time and end up producing discrete
animation. There is nothing in CSS values that requires supporting times of
this magnitude (~63 years) so it seems best not to make the tests rely on it
being supported (the tests are intended to cover interpolation after all, not
timing range) and instead to use a shorter time.
Similarly, for the Web Animations interpolation tests, this utility function
uses a duration of 1ms and seeks to 0.5ms. This can likewise lead to precision
issues, particularly when testing interpolation pairs that fall back to
discrete animation since in that case, the test will seek to precisely 0.5ms
and test that the result is at the top of the step that occurs at that time.
Floating-point error can cause this to fail so this patch changes the duration
and seek time to 100s and 50s respectively.
This patch also includes a few additional tweaks to this file including:
- Replacing the "steps(2, end)" timing function with "linear" since this
avoids floating-point errors at the 0.5 mark. - Adding translation for "float" to "cssFloat".
- Triggering transitions by querying the property being transitioned.
(In theory a UA could skip flushing all styles when accessing the 'left'
property but in practice that is probably not Web-compatible.)
Comment 17•5 years ago
|
||
Comment 20•5 years ago
|
||
Backed out changeset 0626a86d78b1 (Bug 1578125) for causing web platform failures
Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=267784930&resultStatus=testfailed%2Cbusted%2Cexception&revision=0626a86d78b12627264c0d279892e9ff0f98f42a
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=267784930&repo=autoland&lineNumber=38441
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=267784618&repo=autoland&lineNumber=10372
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=267784420&repo=autoland&lineNumber=19447
Backout: https://hg.mozilla.org/integration/autoland/rev/aaa91ca5981cec555473458043cfd37111f5ac8b
Assignee | ||
Comment 21•5 years ago
|
||
There are so many unexpected passes as a result of this. I've spent a whole day just updating test annotations.
Comment 23•5 years ago
|
||
Comment 24•5 years ago
|
||
Comment 25•5 years ago
|
||
(In reply to Brian Birtles (:birtles) from comment #21)
There are so many unexpected passes as a result of this. I've spent a whole day just updating test annotations.
FYI, you can point to a WPT log and do ./mach wpt-udpate <log>
to update those annotations automatically.
Comment 26•5 years ago
|
||
Comment 27•5 years ago
|
||
Comment 28•5 years ago
|
||
Comment 29•5 years ago
|
||
Comment 30•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/73d9659e7715
https://hg.mozilla.org/mozilla-central/rev/9aa5501ed68a
https://hg.mozilla.org/mozilla-central/rev/c36980d14017
https://hg.mozilla.org/mozilla-central/rev/3c1bb6a2bd10
https://hg.mozilla.org/mozilla-central/rev/d45ca6b3be99
https://hg.mozilla.org/mozilla-central/rev/ff6f7d26bb5a
Assignee | ||
Comment 31•5 years ago
|
||
Thanks Emilio for all the follow ups! I had green try runs on Win 7 and Win 10 so I'm not sure what happened there unless something else landed in between.
(In reply to Emilio Cobos Álvarez (:emilio) from comment #25)
(In reply to Brian Birtles (:birtles) from comment #21)
There are so many unexpected passes as a result of this. I've spent a whole day just updating test annotations.
FYI, you can point to a WPT log and do
./mach wpt-udpate <log>
to update those annotations automatically.
Yes, I realised that half way through, but I think I'd need to be on Win 7 / x86 to do that (and I don't have much confidence in my local setup matching CI -- I often get unexpected failures locally due to pixel density differences).
Thanks again!
Comment 32•5 years ago
|
||
Yes, I realised that half way through, but I think I'd need to be on Win 7 / x86 to do that (and I don't have much confidence in my local setup matching CI -- I often get unexpected failures locally due to pixel density differences).
You can always do a try run of just the relevant tests to get the unexpected results for all platforms and update for those logs. I've got a little tool to help download all the logs [1]
We should probably also force the tests to run at single density on desktop like we do on android.
It looks like the upstream PR was merged before this reached central? Which seems like a bug…
Assignee | ||
Comment 33•5 years ago
|
||
(In reply to James Graham [:jgraham] from comment #32)
Yes, I realised that half way through, but I think I'd need to be on Win 7 / x86 to do that (and I don't have much confidence in my local setup matching CI -- I often get unexpected failures locally due to pixel density differences).
You can always do a try run of just the relevant tests to get the unexpected results for all platforms and update for those logs. I've got a little tool to help download all the logs [1]
Yes, I ran the tests on try and went off the Win 7 logs there (and thought I had them all since it was all green). The trouble was more that half of the cases I needed to update the meta file by hg rm
-ing the entire file, and the other half I had to trim the now-passing lines but leave some still-failing entries. In doing that I came across bug 1583012 which made me realise I probably should continue hand editing the files to avoid further corruption.
It looks like the upstream PR was merged before this reached central? Which seems like a bug…
Yes, I noticed that too. I'm not sure what triggered it although it might have been the backout push to autoland that did it.
Nevertheless, what landed in the wpt repository should be correct -- the subsequent pushes didn't touch that part.
Comment hidden (Intermittent Failures Robot) |
Comment 35•5 years ago
|
||
I'm marking this verified fixed as from yesterday's central as beta simulation: https://treeherder.mozilla.org/#/jobs?repo=try&resultStatus=testfailed%2Cbusted%2Cexception&revision=3e07436268e322892f960f046e9dd1f0c555670f&searchStr=windows%2C7%2Cdebug%2C%28wpt&selectedJob=267870334
Updated•5 years ago
|
Updated•5 years ago
|
Comment hidden (Intermittent Failures Robot) |
Comment 38•5 years ago
|
||
Brian, can you request uplift here to fix the failures on mozilla-beta?
Assignee | ||
Comment 39•5 years ago
|
||
(In reply to Andreea Pavel [:apavel] from comment #38)
Brian, can you request uplift here to fix the failures on mozilla-beta?
Yes, I'm working on it now. It might take me a day or so, however.
Comment 40•5 years ago
|
||
No problem. Thank you for giving us updates on this.
Assignee | ||
Comment 41•5 years ago
|
||
I've reworked all the patches to apply to beta. If they seem to work then I'll fold them up and apply for beta approval:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4808bb8f0ea3e3ba0e66254537b9cc1be2412e0c
Assignee | ||
Comment 42•5 years ago
|
||
Hmm, even trying a non artifact build hits some infra error:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=70a4ef6589a95d19681ae481e30fe052cf4b3e0c
Assignee | ||
Comment 43•5 years ago
|
||
Beta/Release Uplift Approval Request
- User impact if declined: None, but tests in CI will fail.
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): It only affects tests.
- String changes made/needed: No
Assignee | ||
Comment 44•5 years ago
|
||
I haven't been able to run the beta patch on CI (see comment 41 and comment 42) so if someone is able to do that before merging, that would be a good idea -- particularly running WPT on Windows 7.
Asking in #sheriffs for someone to take this on.
Comment 46•5 years ago
|
||
Assignee | ||
Comment 47•5 years ago
|
||
Looks like this needs the last patch from bug 1579062 too (to turn on dom.animations-api.implicit-keyframes.enabled
everyone).
Assignee | ||
Comment 48•5 years ago
|
||
Assignee | ||
Comment 49•5 years ago
|
||
Let's see if try works for me this time:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3ba9a29529791a3f561db295eaecd98b924b4747
Assignee | ||
Comment 50•5 years ago
|
||
Comment on attachment 9098412 [details] [diff] [review]
Patch for beta v2
Beta/Release Uplift Approval Request
- User impact if declined: None, but tests in CI will fail.
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): It only affects tests.
- String changes made/needed: None.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Comment 52•5 years ago
|
||
bugherder uplift |
Comment hidden (Intermittent Failures Robot) |
Updated•3 years ago
|
Description
•