Closed
Bug 1324691
Opened 7 years ago
Closed 7 years ago
stylo: several tests non-fatally assert with "stylo: Servo-backed style system should not be using EffectCompositor" and "stylo: ServoRestyleManager does not support animations yet"
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: heycam, Assigned: boris)
References
(Blocks 1 open bug)
Details
Attachments
(5 files)
59 bytes,
text/x-review-board-request
|
heycam
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
hiro
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
heycam
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
heycam
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
xidorn
:
review+
|
Details |
###!!! ASSERTION: stylo: Servo-backed style system should not be using EffectCompositor: 'Error', file /home/worker/workspace/build/src/dom/animation/EffectCompositor.cpp, line 279 ###!!! ASSERTION: stylo: ServoRestyleManager does not support animations yet: 'Error', file /home/worker/workspace/build/src/layout/generic/nsFrame.cpp, line 710 dom/animation/test/crashtests/1216842-3.html dom/animation/test/crashtests/1216842-4.html dom/animation/test/crashtests/1278485-1.html
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → boris.chiou
Assignee | ||
Comment 1•7 years ago
|
||
OK, I can remove these NS_ERROR after Bug 1317209 is merged.
Assignee | ||
Comment 2•7 years ago
|
||
1. ASSERTION: stylo: Servo-backed style system should not be using EffectCompositor * We need to support animation on compositor. However, I think that needs much effort, so we can create a bug for that and use NS_WARNING, instead of NS_ERROR. 2. ASSERTION: stylo: ServoRestyleManager does not support animations yet * This is to destroy frames because no frame for the element (i.e. display: none). I am trying to do the same thing for ServoRestyleManager.
Assignee | ||
Updated•7 years ago
|
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Reporter | ||
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8831052 [details] Bug 1324691 - Part 1: Use warnings instead of assertions for unsupported OMTA. https://reviewboard.mozilla.org/r/107706/#review108846
Attachment #8831052 -
Flags: review?(cam) → review+
Reporter | ||
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8831054 [details] Bug 1324691 - Part 3: Support AnimationsWithDestroyedFrame for ServoRestyleManager. https://reviewboard.mozilla.org/r/107710/#review108848 ::: layout/base/ServoRestyleManager.cpp:325 (Diff revision 1) > } > > + // Create a AnimationsWithDestroyedFrame during restyling process to > + // stop animations and transitions on elements that have no frame at the end > + // of the restyling process. > + RestyleManagerBase::AnimationsWithDestroyedFrame You should be able to remove the "RestyleManagerBase::", since AnimationsWithDestroyedFrame should be in scope here.
Attachment #8831054 -
Flags: review?(cam) → review+
Reporter | ||
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8831055 [details] Bug 1324691 - Part 4: Remove conditions on some crashtests. https://reviewboard.mozilla.org/r/107712/#review108852
Attachment #8831055 -
Flags: review?(cam) → review+
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8831053 [details] Bug 1324691 - Part 2: Move AnimationsWithDestroyedFrame into RestyleManagerBase. https://reviewboard.mozilla.org/r/107708/#review108854 Looks good to me! I am really hoping MozReview can track moving code in different files.
Attachment #8831053 -
Flags: review?(hikezoe) → review+
Assignee | ||
Comment 11•7 years ago
|
||
Note: We only stop CSS animations/transitions and compositor-driven animations that have no frames [1]. (So I cannot test this feature by script animations now.) [1] http://searchfox.org/mozilla-central/rev/7da3c9dcf467964f2fb82f3a4c63972ee79bf696/layout/base/RestyleManager.cpp#229-230
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 14•7 years ago
|
||
Oops, got these failed. I should fix the assertion numbers in part 4. (Reducing the number is so happy. :)) REFTEST TEST-UNEXPECTED-PASS | file:///home/worker/workspace/build/tests/reftest/tests/dom/animation/test/crashtests/1244595-1.html | assertion count 2 is less than expected 3 assertions REFTEST TEST-UNEXPECTED-PASS | file:///home/worker/workspace/build/tests/reftest/tests/dom/animation/test/crashtests/1216842-1.html | assertion count 0 is less than expected 3 to 10 assertions REFTEST TEST-UNEXPECTED-PASS | file:///home/worker/workspace/build/tests/reftest/tests/dom/animation/test/crashtests/1216842-2.html | assertion count 0 is less than expected 3 to 10 assertions REFTEST TEST-UNEXPECTED-PASS | file:///home/worker/workspace/build/tests/reftest/tests/dom/animation/test/crashtests/1216842-5.html | assertion count 0 is less than expected 4 to 10 assertions REFTEST TEST-UNEXPECTED-PASS | file:///home/worker/workspace/build/tests/reftest/tests/dom/animation/test/crashtests/1216842-6.html | assertion count 0 is less than expected 4 to 10 assertions REFTEST TEST-UNEXPECTED-PASS | file:///home/worker/workspace/build/tests/reftest/tests/dom/animation/test/crashtests/1277272-1.html | assertion count 10 is less than expected 31 assertions REFTEST TEST-UNEXPECTED-PASS | file:///home/worker/workspace/build/tests/reftest/tests/dom/animation/test/crashtests/1290535-1.html | assertion count 0 is less than expected 2 assertions REFTEST TEST-UNEXPECTED-PASS | file:///home/worker/workspace/build/tests/reftest/tests/layout/style/crashtests/1282076-2.html | assertion count 2 is less than expected 4 to 64 assertions REFTEST TEST-UNEXPECTED-PASS | file:///home/worker/workspace/build/tests/reftest/tests/layout/style/crashtests/1290994-1.html | assertion count 0 is less than expected 2 assertions REFTEST TEST-UNEXPECTED-PASS | file:///home/worker/workspace/build/tests/reftest/tests/layout/style/crashtests/1290994-2.html | assertion count 0 is less than expected 2 assertions REFTEST TEST-UNEXPECTED-PASS | file:///home/worker/workspace/build/tests/reftest/tests/layout/style/crashtests/1290994-3.html | assertion count 0 is less than expected 1 assertions
Reporter | ||
Comment 15•7 years ago
|
||
That's OK, I'll fix them up when merging.
Assignee | ||
Comment 16•7 years ago
|
||
(In reply to Cameron McCormack (:heycam) (away 27 Jan–1 Feb) from comment #15) > That's OK, I'll fix them up when merging. OK, Thanks.
Comment hidden (obsolete) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 23•7 years ago
|
||
try stylo: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4cdfdb0c5e33c3d852b09bee8217745418319946
Comment 24•7 years ago
|
||
Pushed by bchiou@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9734ac70df3f Part 1: Use warnings instead of assertions for unsupported OMTA. r=heycam https://hg.mozilla.org/integration/autoland/rev/e2d79699222d Part 2: Move AnimationsWithDestroyedFrame into RestyleManagerBase. r=hiro https://hg.mozilla.org/integration/autoland/rev/b401d8f83eb1 Part 3: Support AnimationsWithDestroyedFrame for ServoRestyleManager. r=heycam https://hg.mozilla.org/integration/autoland/rev/e6f1e92c7afb Part 4: Remove conditions on some crashtests. r=heycam
Comment 25•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9734ac70df3f https://hg.mozilla.org/mozilla-central/rev/e2d79699222d https://hg.mozilla.org/mozilla-central/rev/b401d8f83eb1 https://hg.mozilla.org/mozilla-central/rev/e6f1e92c7afb
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment hidden (mozreview-request) |
Comment 29•7 years ago
|
||
mozreview-review |
Comment on attachment 8906469 [details] Bug 1324691 followup - Remove the last condition on crashtest of this bug. https://reviewboard.mozilla.org/r/178222/#review183118
Attachment #8906469 -
Flags: review+
Comment 30•7 years ago
|
||
Pushed by xquan@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2e429e3f2b3f followup - Remove the last condition on crashtest of this bug. r=xidorn
Comment 31•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2e429e3f2b3f
You need to log in
before you can comment on or make changes to this bug.
Description
•