Attachment 368670 Details for Bug 197457 – Patch

[patch] Patch

bug-197457-20190501230750.patch (text/plain), 7.16 KB, created by cathiechen on 2019-05-01 08:07:54 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: cathiechen

Created: 2019-05-01 08:07:54 PDT

Size: 7.16 KB

patch

obsolete

>Subversion Revision: 244820
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index a00aff979a2dedbf6d037ff5ef9ceb0b6786d912..17a6fbef8a6f218b423f3698bb094b303da19ed9 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,23 @@
>+2019-05-01  Cathie Chen  <cathiechen@igalia.com>
>+
>+        JS wrapper of target in an ResizeObserverEntry should not get collected
>+        https://bugs.webkit.org/show_bug.cgi?id=197457
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The JS wrapper of target in ResizeObserverEntry could be collected if no other reference to
>+        this target. In order to keep the JS wrapper live, using GCReachableRef in ResizeObserverEntry
>+        to make sure the target is reachable for GC.
>+
>+        Test: resize-observer/resize-observer-entry-keeps-js-wrapper-of-target-alive.html
>+
>+        * page/ResizeObserver.cpp:
>+        (WebCore::ResizeObserver::deliverObservations):
>+        * page/ResizeObserverEntry.h:
>+        (WebCore::ResizeObserverEntry::create):
>+        (WebCore::ResizeObserverEntry::target const):
>+        (WebCore::ResizeObserverEntry::ResizeObserverEntry):
>+
> 2019-04-30  John Wilander  <wilander@apple.com>
> 
>         Add logging of Ad Click Attribution errors and events to a dedicated channel
>diff --git a/Source/WebCore/page/ResizeObserver.cpp b/Source/WebCore/page/ResizeObserver.cpp
>index 9ff2b6ebc455462e436166f9d35367ea7fde8d7a..1c17c31eb13d0aa043d654df8d6ba0e590129860 100644
>--- a/Source/WebCore/page/ResizeObserver.cpp
>+++ b/Source/WebCore/page/ResizeObserver.cpp
>@@ -118,7 +118,7 @@ void ResizeObserver::deliverObservations()
>     Vector<Ref<ResizeObserverEntry>> entries;
>     for (const auto& observation : m_activeObservations) {
>         ASSERT(observation->target());
>-        entries.append(ResizeObserverEntry::create(observation->target(), observation->computeContentRect()));
>+        entries.append(ResizeObserverEntry::create(*observation->target(), observation->computeContentRect()));
>     }
>     m_activeObservations.clear();
>     m_callback->handleEvent(entries, *this);
>diff --git a/Source/WebCore/page/ResizeObserverEntry.h b/Source/WebCore/page/ResizeObserverEntry.h
>index 9c39fb8f548de64e7d68736cc65e3d9b800f9065..81a144e140c2fc5b779e08a560d78e8ff1b08428 100644
>--- a/Source/WebCore/page/ResizeObserverEntry.h
>+++ b/Source/WebCore/page/ResizeObserverEntry.h
>@@ -30,6 +30,7 @@
> #include "DOMRectReadOnly.h"
> #include "Element.h"
> #include "FloatRect.h"
>+#include "GCReachableRef.h"
> #include <wtf/RefCounted.h>
> 
> namespace WebCore {
>@@ -39,22 +40,22 @@ class Element;
> class ResizeObserverEntry : public RefCounted<ResizeObserverEntry> {
>     WTF_MAKE_FAST_ALLOCATED;
> public:
>-    static Ref<ResizeObserverEntry> create(Element* target, const FloatRect& contentRect)
>+    static Ref<ResizeObserverEntry> create(Element& target, const FloatRect& contentRect)
>     {
>         return adoptRef(*new ResizeObserverEntry(target, contentRect));
>     }
> 
>-    Element* target() const { return m_target.get(); }
>+    Element* target() const { return m_target.ptr(); }
>     DOMRectReadOnly* contentRect() const { return m_contentRect.get(); }
> 
> private:
>-    ResizeObserverEntry(Element* target, const FloatRect& contentRect)
>+    ResizeObserverEntry(Element& target, const FloatRect& contentRect)
>         : m_target(target)
>         , m_contentRect(DOMRectReadOnly::create(contentRect.x(), contentRect.y(), contentRect.width(), contentRect.height()))
>     {
>     }
> 
>-    RefPtr<Element> m_target;
>+    GCReachableRef<Element> m_target;
>     RefPtr<DOMRectReadOnly> m_contentRect;
> };
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index d3a8cc12bc326887905db86d451b6960f4d98b50..eb092852a01617138fb764c01fa5db1b45cb6c53 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2019-05-01  Cathie Chen  <cathiechen@igalia.com>
>+
>+        JS wrapper of target in an ResizeObserverEntry should not get collected
>+        https://bugs.webkit.org/show_bug.cgi?id=197457
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * resize-observer/resize-observer-entry-keeps-js-wrapper-of-target-alive-expected.txt: Added.
>+        * resize-observer/resize-observer-entry-keeps-js-wrapper-of-target-alive.html: Added.
>+
> 2019-04-30  Myles C. Maxfield  <mmaxfield@apple.com>
> 
>         font-weight: 1000 is not parsed successfully
>diff --git a/LayoutTests/resize-observer/resize-observer-entry-keeps-js-wrapper-of-target-alive-expected.txt b/LayoutTests/resize-observer/resize-observer-entry-keeps-js-wrapper-of-target-alive-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..96314c9eeb32e4af60eab749442b292131c0d200
>--- /dev/null
>+++ b/LayoutTests/resize-observer/resize-observer-entry-keeps-js-wrapper-of-target-alive-expected.txt
>@@ -0,0 +1,12 @@
>+CONSOLE MESSAGE: ResizeObserver loop completed with undelivered notifications.
>+CONSOLE MESSAGE: ResizeObserver loop completed with undelivered notifications.
>+CONSOLE MESSAGE: ResizeObserver loop completed with undelivered notifications.
>+CONSOLE MESSAGE: ResizeObserver loop completed with undelivered notifications.
>+This tests that JS wrappers of targets in an ResizeObserverEntry do not get collected.
>+
>+PASS
>+PASS
>+PASS
>+PASS
>+PASS
>+
>diff --git a/LayoutTests/resize-observer/resize-observer-entry-keeps-js-wrapper-of-target-alive.html b/LayoutTests/resize-observer/resize-observer-entry-keeps-js-wrapper-of-target-alive.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..736cf6f83744d9c4622ef334ec2816ae74a2b0aa
>--- /dev/null
>+++ b/LayoutTests/resize-observer/resize-observer-entry-keeps-js-wrapper-of-target-alive.html
>@@ -0,0 +1,60 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<p>This tests that JS wrappers of targets in an ResizeObserverEntry do not get collected.</p>
>+<pre id="log"></pre>
>+<script src="../resources/gc.js"></script>
>+<script>
>+
>+if (window.testRunner)
>+    testRunner.dumpAsText();
>+
>+const targetCount = 5;
>+const iterationCount = 5;
>+
>+async function observe() {
>+    for (let i = 0; i < targetCount; ++i) {
>+        let target = document.createElement('div');
>+        target.myState = 'live';
>+        document.body.appendChild(target);
>+    }
>+
>+    return new Promise((resolve) => {
>+        const observer = new ResizeObserver(entries => {
>+            resolve(entries);
>+            observer.disconnect();
>+        });
>+        document.querySelectorAll('div').forEach(target => { observer.observe(target); });
>+    });
>+}
>+
>+function check(entries) {
>+    let deadCount = 0;
>+    for (const entry of entries) {
>+        if (entry.target.myState != 'live')
>+            deadCount++;
>+    }
>+    document.getElementById('log').textContent += (deadCount ? `FAIL - ${deadCount} targets lost JS wrappers` : 'PASS') + '\n';
>+}
>+
>+async function runAll() {
>+    if (window.testRunner)
>+        testRunner.waitUntilDone();
>+
>+    for (let j = 0; j < iterationCount; ++j) {
>+        const entries = await observe();
>+        document.querySelectorAll('div').forEach(target => { target.remove(); });
>+        await Promise.resolve();
>+        gc();
>+        check(entries);
>+    }
>+
>+    if (window.testRunner)
>+        testRunner.notifyDone();
>+}
>+
>+runAll();
>+
>+</script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 197457: 368670 | 369394 | 369426 | 369680 | 369681 | 369683 | 369738 | 369746 | 369752 | 369755 | 369756 | 369764 | 369833 | 369837 | 369840 | 369844 | 369845 | 369846 | 369847 | 369848 | 369854 | 369861 | 369862 | 369866 | 369870 | 369876 | 369877 | 369921 | 369922 | 369923 | 369925 | 369926 | 369927 | 369935 | 369942 | 370111 | 370681 | 371183 | 371186 | 371241