Title: [258823] trunk
- Revision
- 258823
- Author
- [email protected]
- Date
- 2020-03-22 15:57:22 -0700 (Sun, 22 Mar 2020)
Log Message
[ Mac ] imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events-replacement.html is flaky failing.
https://bugs.webkit.org/show_bug.cgi?id=209239
<rdar://problem/60591358>
Patch by Antoine Quint <[email protected]> on 2020-03-22
Reviewed by Simon Fraser.
Source/WebCore:
This test was made flaky by r257417, the initial fix for webkit.org/b/208069. A new, appropriate fix for that bug is in the works. In the
meantime we revert r257417 in this patch.
The reason this test became flaky is that it features the following code:
animB.timeline = new DocumentTimeline({
originTime:
document.timeline.currentTime - 100 * MS_PER_SEC - animB.startTime,
});
In this case the only reference to the created DocumentTimeline is through `animB.timeline`. But because r257417 made the timeline reference from
WebAnimation a weak reference, in some cases, if GC kicks in, the timeline would be dereferenced and the test would fail. We restore that relationship
to its previous state, which is a strong reference.
* animation/WebAnimation.cpp:
(WebCore::WebAnimation::setTimeline):
(WebCore::WebAnimation::setTimelineInternal):
(WebCore::WebAnimation::enqueueAnimationEvent):
(WebCore::WebAnimation::acceleratedStateDidChange):
(WebCore::WebAnimation::timeline const): Deleted.
* animation/WebAnimation.h:
(WebCore::WebAnimation::timeline const):
LayoutTests:
* platform/mac/TestExpectations:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (258822 => 258823)
--- trunk/LayoutTests/ChangeLog 2020-03-22 20:33:05 UTC (rev 258822)
+++ trunk/LayoutTests/ChangeLog 2020-03-22 22:57:22 UTC (rev 258823)
@@ -1,3 +1,13 @@
+2020-03-22 Antoine Quint <[email protected]>
+
+ [ Mac ] imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events-replacement.html is flaky failing.
+ https://bugs.webkit.org/show_bug.cgi?id=209239
+ <rdar://problem/60591358>
+
+ Reviewed by Simon Fraser.
+
+ * platform/mac/TestExpectations:
+
2020-03-22 Diego Pino Garcia <[email protected]>
[GTK][WPE] Gardening, update TestExpectations
Modified: trunk/LayoutTests/platform/mac/TestExpectations (258822 => 258823)
--- trunk/LayoutTests/platform/mac/TestExpectations 2020-03-22 20:33:05 UTC (rev 258822)
+++ trunk/LayoutTests/platform/mac/TestExpectations 2020-03-22 22:57:22 UTC (rev 258823)
@@ -2032,8 +2032,6 @@
webkit.org/b/207150 platform/mac/webrtc/captureCanvas-webrtc-software-encoder.html [ Pass Failure ]
-webkit.org/b/209239 imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events-replacement.html [ Pass Failure ]
-
# The line breaking rules changed in ICU 66. We've updated the tests to match, but old platforms won't get updated line breaking rules.
webkit.org/b/209250 [ Sierra+ ] imported/w3c/web-platform-tests/css/css-text/line-break/line-break-normal-015.xht [ ImageOnlyFailure ]
webkit.org/b/209250 [ Sierra+ ] imported/w3c/web-platform-tests/css/css-text/line-break/line-break-strict-015.xht [ ImageOnlyFailure ]
Modified: trunk/Source/WebCore/ChangeLog (258822 => 258823)
--- trunk/Source/WebCore/ChangeLog 2020-03-22 20:33:05 UTC (rev 258822)
+++ trunk/Source/WebCore/ChangeLog 2020-03-22 22:57:22 UTC (rev 258823)
@@ -1,3 +1,34 @@
+2020-03-22 Antoine Quint <[email protected]>
+
+ [ Mac ] imported/w3c/web-platform-tests/web-animations/timing-model/timelines/update-and-send-events-replacement.html is flaky failing.
+ https://bugs.webkit.org/show_bug.cgi?id=209239
+ <rdar://problem/60591358>
+
+ Reviewed by Simon Fraser.
+
+ This test was made flaky by r257417, the initial fix for webkit.org/b/208069. A new, appropriate fix for that bug is in the works. In the
+ meantime we revert r257417 in this patch.
+
+ The reason this test became flaky is that it features the following code:
+
+ animB.timeline = new DocumentTimeline({
+ originTime:
+ document.timeline.currentTime - 100 * MS_PER_SEC - animB.startTime,
+ });
+
+ In this case the only reference to the created DocumentTimeline is through `animB.timeline`. But because r257417 made the timeline reference from
+ WebAnimation a weak reference, in some cases, if GC kicks in, the timeline would be dereferenced and the test would fail. We restore that relationship
+ to its previous state, which is a strong reference.
+
+ * animation/WebAnimation.cpp:
+ (WebCore::WebAnimation::setTimeline):
+ (WebCore::WebAnimation::setTimelineInternal):
+ (WebCore::WebAnimation::enqueueAnimationEvent):
+ (WebCore::WebAnimation::acceleratedStateDidChange):
+ (WebCore::WebAnimation::timeline const): Deleted.
+ * animation/WebAnimation.h:
+ (WebCore::WebAnimation::timeline const):
+
2020-03-22 Zalan Bujtas <[email protected]>
[LFC] Introduce InitialContainingBox class
Modified: trunk/Source/WebCore/animation/WebAnimation.cpp (258822 => 258823)
--- trunk/Source/WebCore/animation/WebAnimation.cpp 2020-03-22 20:33:05 UTC (rev 258822)
+++ trunk/Source/WebCore/animation/WebAnimation.cpp 2020-03-22 22:57:22 UTC (rev 258823)
@@ -189,11 +189,6 @@
invalidateEffect();
}
-AnimationTimeline* WebAnimation::timeline() const
-{
- return m_timeline.get();
-}
-
void WebAnimation::setEffectInternal(RefPtr<AnimationEffect>&& newEffect, bool doNotRemoveFromTimeline)
{
if (m_effect == newEffect)
@@ -232,7 +227,7 @@
// https://drafts.csswg.org/web-animations-1/#setting-the-timeline
// 2. If new timeline is the same object as old timeline, abort this procedure.
- if (timeline == m_timeline.get())
+ if (timeline == m_timeline)
return;
// 4. If the animation start time of animation is resolved, make animation's hold time unresolved.
@@ -257,7 +252,7 @@
auto protectedThis = makeRef(*this);
setTimelineInternal(WTFMove(timeline));
- setSuspended(is<DocumentTimeline>(m_timeline.get()) && downcast<DocumentTimeline>(*m_timeline).animationsAreSuspended());
+ setSuspended(is<DocumentTimeline>(m_timeline) && downcast<DocumentTimeline>(*m_timeline).animationsAreSuspended());
// 5. Run the procedure to update an animation's finished state for animation with the did seek flag set to false,
// and the synchronously notify flag set to false.
@@ -268,13 +263,13 @@
void WebAnimation::setTimelineInternal(RefPtr<AnimationTimeline>&& timeline)
{
- if (m_timeline.get() == timeline)
+ if (m_timeline == timeline)
return;
if (m_timeline)
m_timeline->removeAnimation(*this);
- m_timeline = makeWeakPtr(timeline.get());
+ m_timeline = WTFMove(timeline);
if (m_effect)
m_effect->animationTimelineDidChange(m_timeline.get());
@@ -683,7 +678,7 @@
void WebAnimation::enqueueAnimationEvent(Ref<AnimationEventBase>&& event)
{
- if (is<DocumentTimeline>(m_timeline.get())) {
+ if (is<DocumentTimeline>(m_timeline)) {
// If animation has a document for timing, then append event to its document for timing's pending animation event queue along
// with its target, animation. If animation is associated with an active timeline that defines a procedure to convert timeline times
// to origin-relative time, let the scheduled event time be the result of applying that procedure to timeline time. Otherwise, the
@@ -1239,7 +1234,7 @@
void WebAnimation::acceleratedStateDidChange()
{
- if (is<DocumentTimeline>(m_timeline.get()))
+ if (is<DocumentTimeline>(m_timeline))
downcast<DocumentTimeline>(*m_timeline).animationAcceleratedRunningStateDidChange(*this);
}
Modified: trunk/Source/WebCore/animation/WebAnimation.h (258822 => 258823)
--- trunk/Source/WebCore/animation/WebAnimation.h 2020-03-22 20:33:05 UTC (rev 258822)
+++ trunk/Source/WebCore/animation/WebAnimation.h 2020-03-22 22:57:22 UTC (rev 258823)
@@ -68,7 +68,7 @@
AnimationEffect* effect() const { return m_effect.get(); }
void setEffect(RefPtr<AnimationEffect>&&);
- AnimationTimeline* timeline() const;
+ AnimationTimeline* timeline() const { return m_timeline.get(); }
virtual void setTimeline(RefPtr<AnimationTimeline>&&);
Optional<Seconds> currentTime() const;
@@ -186,7 +186,7 @@
void applyPendingPlaybackRate();
RefPtr<AnimationEffect> m_effect;
- WeakPtr<AnimationTimeline> m_timeline;
+ RefPtr<AnimationTimeline> m_timeline;
UniqueRef<ReadyPromise> m_readyPromise;
UniqueRef<FinishedPromise> m_finishedPromise;
Markable<Seconds, Seconds::MarkableTraits> m_previousCurrentTime;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
