- Revision
- 258826
- Author
- grao...@webkit.org
- Date
- 2020-03-23 01:17:26 -0700 (Mon, 23 Mar 2020)
Log Message
DocumentTimeline / CSSTransition objects are leaking on CNN.com
https://bugs.webkit.org/show_bug.cgi?id=208069
<rdar://problem/59680143>
Reviewed by Simon Fraser, Geoffrey Garen and Darin Adler.
Source/WebCore:
Test: webanimations/leak-css-animation.html
We add a test feature that lets use query the availability of a given WebAnimation by its "id" property in the WebAnimation::instances list.
We also fix some build issues that appeared with a change in UnifiedSources order.
* animation/ElementAnimationRareData.cpp:
(WebCore::ElementAnimationRareData::setAnimationsCreatedByMarkup):
* animation/ElementAnimationRareData.h:
(WebCore::ElementAnimationRareData::setAnimationsCreatedByMarkup): Deleted.
* animation/WebAnimation.h:
* testing/Internals.cpp:
(WebCore::Internals::animationWithIdExists const):
* testing/Internals.h:
* testing/Internals.idl:
Source/WTF:
If a CSSAnimation is set on an element using the `animation-name` CSS property, and later removed, it will leak due to the ListHashSet<RefPtr<CSSAnimation>>
(aka CSSAnimationCollection) member on ElementAnimationRareData being replaced to the new list, but the old list not being cleared from its members.
We fix the ListHashSet assignment operator to use swap ensuring previously held items are cleared.
* wtf/ListHashSet.h:
(WTF::=):
Tools:
Add a test that checks that a ListHashSet containing RefPtr<> types correctly calls the destructor for those items when the assignment operator is used.
* TestWebKitAPI/Tests/WTF/ListHashSet.cpp:
(TestWebKitAPI::ListHashSetReferencedItem::create):
(TestWebKitAPI::ListHashSetReferencedItem::ListHashSetReferencedItem):
(TestWebKitAPI::ListHashSetReferencedItem::~ListHashSetReferencedItem):
(TestWebKitAPI::FakeElementAnimationRareData::FakeElementAnimationRareData):
(TestWebKitAPI::FakeElementAnimationRareData::~FakeElementAnimationRareData):
(TestWebKitAPI::FakeElementAnimationRareData::collection):
(TestWebKitAPI::FakeElementAnimationRareData::setCollection):
(TestWebKitAPI::TEST):
LayoutTests:
Add a test that checks that setting a CSSAnimation on an element, waiting a frame, and removing it will not leak that CSSAnimation.
* webanimations/leak-css-animation-expected.txt: Added.
* webanimations/leak-css-animation.html: Added.
* webanimations/resources/css-animation-leak-iframe.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (258825 => 258826)
--- trunk/LayoutTests/ChangeLog 2020-03-23 04:57:06 UTC (rev 258825)
+++ trunk/LayoutTests/ChangeLog 2020-03-23 08:17:26 UTC (rev 258826)
@@ -1,5 +1,19 @@
2020-03-22 Antoine Quint <grao...@apple.com>
+ DocumentTimeline / CSSTransition objects are leaking on CNN.com
+ https://bugs.webkit.org/show_bug.cgi?id=208069
+ <rdar://problem/59680143>
+
+ Reviewed by Simon Fraser, Geoffrey Garen and Darin Adler.
+
+ Add a test that checks that setting a CSSAnimation on an element, waiting a frame, and removing it will not leak that CSSAnimation.
+
+ * webanimations/leak-css-animation-expected.txt: Added.
+ * webanimations/leak-css-animation.html: Added.
+ * webanimations/resources/css-animation-leak-iframe.html: Added.
+
+2020-03-22 Antoine Quint <grao...@apple.com>
+
[ 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>
Added: trunk/LayoutTests/webanimations/leak-css-animation-expected.txt (0 => 258826)
--- trunk/LayoutTests/webanimations/leak-css-animation-expected.txt (rev 0)
+++ trunk/LayoutTests/webanimations/leak-css-animation-expected.txt 2020-03-23 08:17:26 UTC (rev 258826)
@@ -0,0 +1,10 @@
+This test asserts that a CSSAnimation doesn't leak after it was removed declaratively and the document was replaced.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS The CSS animation was destroyed.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/webanimations/leak-css-animation.html (0 => 258826)
--- trunk/LayoutTests/webanimations/leak-css-animation.html (rev 0)
+++ trunk/LayoutTests/webanimations/leak-css-animation.html 2020-03-23 08:17:26 UTC (rev 258826)
@@ -0,0 +1,74 @@
+<!DOCTYPE html>
+<html>
+<body _onload_="runTest()">
+<script src=""
+<script>
+description("This test asserts that a CSSAnimation doesn't leak after it was removed declaratively and the document was replaced.");
+
+if (window.internals)
+ jsTestIsAsync = true;
+
+function runTest() {
+ if (!window.internals)
+ return;
+
+ var frame = document.body.appendChild(document.createElement("iframe"));
+
+ frame.addEventListener("load", async () => {
+ if (frame.src ="" 'about:blank')
+ return;
+
+ const animationId = "leak-css-animation";
+
+ await (element => {
+ return new Promise(resolve => {
+ const animation = element.getAnimations()[0];
+ if (!animation) {
+ testFailed("The expected CSS animation was not created.");
+ finishJSTest();
+ }
+
+ animation.id = animationId;
+ if (!internals.animationWithIdExists(animationId)) {
+ testFailed("The expected CSS animation with the provided ID was not initially found.");
+ finishJSTest();
+ }
+
+ requestAnimationFrame(() => {
+ element.style.animation = "none";
+ resolve();
+ });
+ });
+ })(frame.contentDocument.querySelector("div"));
+
+ requestAnimationFrame(() => {
+ frame.remove();
+ frame = null;
+
+ gc();
+ let timeout = 0;
+ const handle = setInterval(() => {
+ if (!internals.animationWithIdExists(animationId)) {
+ clearInterval(handle);
+ testPassed("The CSS animation was destroyed.");
+ finishJSTest();
+ return;
+ }
+ timeout++;
+ if (timeout == 500) {
+ clearInterval(handle);
+ testFailed("The CSS animation was leaked.");
+ finishJSTest();
+ return;
+ }
+ gc();
+ }, 10);
+ });
+ });
+
+ frame.src = '';
+}
+
+</script>
+</body>
+</html>
Added: trunk/LayoutTests/webanimations/resources/css-animation-leak-iframe.html (0 => 258826)
--- trunk/LayoutTests/webanimations/resources/css-animation-leak-iframe.html (rev 0)
+++ trunk/LayoutTests/webanimations/resources/css-animation-leak-iframe.html 2020-03-23 08:17:26 UTC (rev 258826)
@@ -0,0 +1,10 @@
+<style>
+div {
+ animation: animation 1000s;
+}
+
+@keyframes animation {
+ to { margin-left: 100px }
+}
+</style>
+<div></div>
Modified: trunk/Source/WTF/ChangeLog (258825 => 258826)
--- trunk/Source/WTF/ChangeLog 2020-03-23 04:57:06 UTC (rev 258825)
+++ trunk/Source/WTF/ChangeLog 2020-03-23 08:17:26 UTC (rev 258826)
@@ -1,3 +1,19 @@
+2020-03-22 Antoine Quint <grao...@apple.com>
+
+ DocumentTimeline / CSSTransition objects are leaking on CNN.com
+ https://bugs.webkit.org/show_bug.cgi?id=208069
+ <rdar://problem/59680143>
+
+ Reviewed by Simon Fraser, Geoffrey Garen and Darin Adler.
+
+ If a CSSAnimation is set on an element using the `animation-name` CSS property, and later removed, it will leak due to the ListHashSet<RefPtr<CSSAnimation>>
+ (aka CSSAnimationCollection) member on ElementAnimationRareData being replaced to the new list, but the old list not being cleared from its members.
+
+ We fix the ListHashSet assignment operator to use swap ensuring previously held items are cleared.
+
+ * wtf/ListHashSet.h:
+ (WTF::=):
+
2020-03-20 Per Arne Vollan <pvol...@apple.com>
[Cocoa] Deny access to database mapping service
Modified: trunk/Source/WTF/wtf/ListHashSet.h (258825 => 258826)
--- trunk/Source/WTF/wtf/ListHashSet.h 2020-03-23 04:57:06 UTC (rev 258825)
+++ trunk/Source/WTF/wtf/ListHashSet.h 2020-03-23 08:17:26 UTC (rev 258826)
@@ -351,9 +351,8 @@
template<typename T, typename U>
inline ListHashSet<T, U>& ListHashSet<T, U>::operator=(ListHashSet&& other)
{
- m_impl = WTFMove(other.m_impl);
- m_head = std::exchange(other.m_head, nullptr);
- m_tail = std::exchange(other.m_tail, nullptr);
+ ListHashSet tmp(WTFMove(other));
+ swap(tmp);
return *this;
}
Modified: trunk/Source/WebCore/ChangeLog (258825 => 258826)
--- trunk/Source/WebCore/ChangeLog 2020-03-23 04:57:06 UTC (rev 258825)
+++ trunk/Source/WebCore/ChangeLog 2020-03-23 08:17:26 UTC (rev 258826)
@@ -1,5 +1,28 @@
2020-03-22 Antoine Quint <grao...@apple.com>
+ DocumentTimeline / CSSTransition objects are leaking on CNN.com
+ https://bugs.webkit.org/show_bug.cgi?id=208069
+ <rdar://problem/59680143>
+
+ Reviewed by Simon Fraser, Geoffrey Garen and Darin Adler.
+
+ Test: webanimations/leak-css-animation.html
+
+ We add a test feature that lets use query the availability of a given WebAnimation by its "id" property in the WebAnimation::instances list.
+ We also fix some build issues that appeared with a change in UnifiedSources order.
+
+ * animation/ElementAnimationRareData.cpp:
+ (WebCore::ElementAnimationRareData::setAnimationsCreatedByMarkup):
+ * animation/ElementAnimationRareData.h:
+ (WebCore::ElementAnimationRareData::setAnimationsCreatedByMarkup): Deleted.
+ * animation/WebAnimation.h:
+ * testing/Internals.cpp:
+ (WebCore::Internals::animationWithIdExists const):
+ * testing/Internals.h:
+ * testing/Internals.idl:
+
+2020-03-22 Antoine Quint <grao...@apple.com>
+
[ 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>
Modified: trunk/Source/WebCore/animation/ElementAnimationRareData.cpp (258825 => 258826)
--- trunk/Source/WebCore/animation/ElementAnimationRareData.cpp 2020-03-23 04:57:06 UTC (rev 258825)
+++ trunk/Source/WebCore/animation/ElementAnimationRareData.cpp 2020-03-23 08:17:26 UTC (rev 258826)
@@ -26,6 +26,7 @@
#include "config.h"
#include "ElementAnimationRareData.h"
+#include "CSSAnimation.h"
#include "KeyframeEffectStack.h"
namespace WebCore {
@@ -45,4 +46,9 @@
return *m_keyframeEffectStack.get();
}
+void ElementAnimationRareData::setAnimationsCreatedByMarkup(CSSAnimationCollection&& animations)
+{
+ m_animationsCreatedByMarkup = WTFMove(animations);
+}
+
} // namespace WebCore
Modified: trunk/Source/WebCore/animation/ElementAnimationRareData.h (258825 => 258826)
--- trunk/Source/WebCore/animation/ElementAnimationRareData.h 2020-03-23 04:57:06 UTC (rev 258825)
+++ trunk/Source/WebCore/animation/ElementAnimationRareData.h 2020-03-23 08:17:26 UTC (rev 258826)
@@ -48,7 +48,7 @@
AnimationCollection& cssAnimations() { return m_cssAnimations; }
AnimationCollection& transitions() { return m_transitions; }
CSSAnimationCollection& animationsCreatedByMarkup() { return m_animationsCreatedByMarkup; }
- void setAnimationsCreatedByMarkup(CSSAnimationCollection&& animations) { m_animationsCreatedByMarkup = WTFMove(animations); }
+ void setAnimationsCreatedByMarkup(CSSAnimationCollection&&);
PropertyToTransitionMap& completedTransitionByProperty() { return m_completedTransitionByProperty; }
PropertyToTransitionMap& runningTransitionsByProperty() { return m_runningTransitionsByProperty; }
Modified: trunk/Source/WebCore/animation/WebAnimation.h (258825 => 258826)
--- trunk/Source/WebCore/animation/WebAnimation.h 2020-03-23 04:57:06 UTC (rev 258825)
+++ trunk/Source/WebCore/animation/WebAnimation.h 2020-03-23 08:17:26 UTC (rev 258826)
@@ -57,7 +57,7 @@
static Ref<WebAnimation> create(Document&, AnimationEffect*, AnimationTimeline*);
~WebAnimation();
- static HashSet<WebAnimation*>& instances();
+ WEBCORE_EXPORT static HashSet<WebAnimation*>& instances();
virtual bool isDeclarativeAnimation() const { return false; }
virtual bool isCSSAnimation() const { return false; }
Modified: trunk/Source/WebCore/testing/Internals.cpp (258825 => 258826)
--- trunk/Source/WebCore/testing/Internals.cpp 2020-03-23 04:57:06 UTC (rev 258825)
+++ trunk/Source/WebCore/testing/Internals.cpp 2020-03-23 08:17:26 UTC (rev 258826)
@@ -193,6 +193,7 @@
#include "UserMediaController.h"
#include "ViewportArguments.h"
#include "VoidCallback.h"
+#include "WebAnimation.h"
#include "WebCoreJSClientData.h"
#include "WindowProxy.h"
#include "WorkerThread.h"
@@ -1035,6 +1036,15 @@
return contextDocument()->page()->lastSpatialNavigationCandidateCount();
}
+bool Internals::animationWithIdExists(const String& id) const
+{
+ for (auto* animation : WebAnimation::instances()) {
+ if (animation && animation->id() == id)
+ return true;
+ }
+ return false;
+}
+
unsigned Internals::numberOfActiveAnimations() const
{
if (RuntimeEnabledFeatures::sharedFeatures().webAnimationsCSSIntegrationEnabled())
Modified: trunk/Source/WebCore/testing/Internals.h (258825 => 258826)
--- trunk/Source/WebCore/testing/Internals.h 2020-03-23 04:57:06 UTC (rev 258825)
+++ trunk/Source/WebCore/testing/Internals.h 2020-03-23 08:17:26 UTC (rev 258826)
@@ -210,6 +210,7 @@
ExceptionOr<unsigned> lastSpatialNavigationCandidateCount() const;
// CSS Animation testing.
+ bool animationWithIdExists(const String&) const;
unsigned numberOfActiveAnimations() const;
ExceptionOr<bool> animationsAreSuspended() const;
ExceptionOr<void> suspendAnimations() const;
Modified: trunk/Source/WebCore/testing/Internals.idl (258825 => 258826)
--- trunk/Source/WebCore/testing/Internals.idl 2020-03-23 04:57:06 UTC (rev 258825)
+++ trunk/Source/WebCore/testing/Internals.idl 2020-03-23 08:17:26 UTC (rev 258826)
@@ -242,6 +242,7 @@
readonly attribute unsigned long inflightBeaconsCount;
// CSS Animation testing.
+ boolean animationWithIdExists(DOMString id);
unsigned long numberOfActiveAnimations();
[MayThrowException] void suspendAnimations();
[MayThrowException] void resumeAnimations();
Modified: trunk/Tools/ChangeLog (258825 => 258826)
--- trunk/Tools/ChangeLog 2020-03-23 04:57:06 UTC (rev 258825)
+++ trunk/Tools/ChangeLog 2020-03-23 08:17:26 UTC (rev 258826)
@@ -1,3 +1,23 @@
+2020-03-22 Antoine Quint <grao...@apple.com>
+
+ DocumentTimeline / CSSTransition objects are leaking on CNN.com
+ https://bugs.webkit.org/show_bug.cgi?id=208069
+ <rdar://problem/59680143>
+
+ Reviewed by Simon Fraser, Geoffrey Garen and Darin Adler.
+
+ Add a test that checks that a ListHashSet containing RefPtr<> types correctly calls the destructor for those items when the assignment operator is used.
+
+ * TestWebKitAPI/Tests/WTF/ListHashSet.cpp:
+ (TestWebKitAPI::ListHashSetReferencedItem::create):
+ (TestWebKitAPI::ListHashSetReferencedItem::ListHashSetReferencedItem):
+ (TestWebKitAPI::ListHashSetReferencedItem::~ListHashSetReferencedItem):
+ (TestWebKitAPI::FakeElementAnimationRareData::FakeElementAnimationRareData):
+ (TestWebKitAPI::FakeElementAnimationRareData::~FakeElementAnimationRareData):
+ (TestWebKitAPI::FakeElementAnimationRareData::collection):
+ (TestWebKitAPI::FakeElementAnimationRareData::setCollection):
+ (TestWebKitAPI::TEST):
+
2020-03-21 Michael Catanzaro <mcatanz...@gnome.org>
[GTK] Use ${PYTHON_EXECUTABLE} to run generate-gtkdoc
Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/ListHashSet.cpp (258825 => 258826)
--- trunk/Tools/TestWebKitAPI/Tests/WTF/ListHashSet.cpp 2020-03-23 04:57:06 UTC (rev 258825)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/ListHashSet.cpp 2020-03-23 08:17:26 UTC (rev 258826)
@@ -28,6 +28,8 @@
#include "Counters.h"
#include "MoveOnly.h"
#include <wtf/ListHashSet.h>
+#include <wtf/NeverDestroyed.h>
+#include <wtf/RefCounted.h>
namespace TestWebKitAPI {
@@ -476,4 +478,63 @@
EXPECT_EQ(1u, ConstructorDestructorCounter::destructionCount);
}
+class ListHashSetReferencedItem : public RefCounted<ListHashSetReferencedItem> {
+public:
+ static Ref<ListHashSetReferencedItem> create()
+ {
+ auto result = adoptRef(*new ListHashSetReferencedItem());
+ return result;
+ }
+
+ explicit ListHashSetReferencedItem()
+ {
+ instances().add(this);
+ }
+
+ ~ListHashSetReferencedItem()
+ {
+ ASSERT(instances().contains(this));
+ instances().remove(this);
+ }
+
+ static HashSet<ListHashSetReferencedItem*>& instances()
+ {
+ static NeverDestroyed<HashSet<ListHashSetReferencedItem*>> instances;
+ return instances;
+ }
+};
+
+using Collection = ListHashSet<RefPtr<ListHashSetReferencedItem>>;
+
+class FakeElementAnimationRareData {
+ WTF_MAKE_NONCOPYABLE(FakeElementAnimationRareData);
+ WTF_MAKE_FAST_ALLOCATED;
+public:
+ explicit FakeElementAnimationRareData() { };
+ ~FakeElementAnimationRareData() { };
+
+ Collection& collection() { return m_collection; }
+ void setCollection(Collection&& collection) { m_collection = WTFMove(collection); }
+
+private:
+ Collection m_collection;
+};
+
+TEST(WTF_ListHashSet, ClearsItemUponAssignment)
+{
+ std::unique_ptr<FakeElementAnimationRareData> data = ""
+
+ EXPECT_EQ(0u, ListHashSetReferencedItem::instances().size());
+
+ Collection firstCollection({ ListHashSetReferencedItem::create() });
+ data->setCollection(WTFMove(firstCollection));
+
+ EXPECT_EQ(1u, ListHashSetReferencedItem::instances().size());
+
+ Collection secondCollection;
+ data->setCollection(WTFMove(secondCollection));
+
+ EXPECT_EQ(0u, ListHashSetReferencedItem::instances().size());
+}
+
} // namespace TestWebKitAPI
