Attachment 348255 Details for Bug 189021 – Patch

[patch] Patch

bug-189021-20180827195045.patch (text/plain), 9.52 KB, created by youenn fablet on 2018-08-27 19:50:46 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2018-08-27 19:50:46 PDT

Size: 9.52 KB

patch

obsolete

>Subversion Revision: 235368
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index e2f333ef06bd01c460c81c24a50c704afad4b955..c70d31cf2cab034f38b78c662b5062223c49ae22 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2018-08-27  Youenn Fablet  <youenn@apple.com>
>+
>+        MediaDevices should be collectable as soon as its document is stopped
>+        https://bugs.webkit.org/show_bug.cgi?id=189021
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Introduce ActiveDOMObject::isContextStopped to check whether the context is stopped.
>+        Use this check in MediaDevices::hasPendingActivity so that it returns false as soon as active dom objects are stopped.
>+
>+        Add a way to query whether a context is stopped and implements this for Document and WorkerGlobalScope.
>+        Use this in ActiveDOMObject::isContextStopped.
>+
>+        Test: http/tests/media/collect-media-devices.https.html
>+
>+        * Modules/mediastream/MediaDevices.cpp:
>+        (WebCore::MediaDevices::hasPendingActivity const):
>+        * dom/ActiveDOMObject.cpp:
>+        (WebCore::ActiveDOMObject::hasPendingActivity const):
>+        (WebCore::ActiveDOMObject::isContextStopped const):
>+        * dom/ActiveDOMObject.h:
>+        * dom/Document.h:
>+        * dom/ScriptExecutionContext.h:
>+        * workers/WorkerGlobalScope.cpp:
>+        (WebCore::WorkerGlobalScope::prepareForTermination):
>+        * workers/WorkerGlobalScope.h:
>+
> 2018-08-27  Youenn Fablet  <youenn@apple.com>
> 
>         WebKitMediaSession should be GC collectable when its document is being stopped
>diff --git a/Source/WebCore/Modules/mediastream/MediaDevices.cpp b/Source/WebCore/Modules/mediastream/MediaDevices.cpp
>index 5c0f538e62dbf155b21ce9615109934dc04e006d..d9b0bb775ae877c9b2415aff0850953ef9d3d146 100644
>--- a/Source/WebCore/Modules/mediastream/MediaDevices.cpp
>+++ b/Source/WebCore/Modules/mediastream/MediaDevices.cpp
>@@ -162,7 +162,7 @@ void MediaDevices::scheduledEventTimerFired()
> 
> bool MediaDevices::hasPendingActivity() const
> {
>-    return scriptExecutionContext() && hasEventListeners(m_eventNames.devicechangeEvent);
>+    return !isContextStopped() && hasEventListeners(m_eventNames.devicechangeEvent);
> }
> 
> const char* MediaDevices::activeDOMObjectName() const
>diff --git a/Source/WebCore/dom/ActiveDOMObject.cpp b/Source/WebCore/dom/ActiveDOMObject.cpp
>index b81a884c40ffa26c9d55726b040f900c38e5e885..56c329b78f524e535821189f09140be3953e1ffb 100644
>--- a/Source/WebCore/dom/ActiveDOMObject.cpp
>+++ b/Source/WebCore/dom/ActiveDOMObject.cpp
>@@ -106,4 +106,9 @@ void ActiveDOMObject::stop()
> {
> }
> 
>+bool ActiveDOMObject::isContextStopped() const
>+{
>+    return !scriptExecutionContext() || scriptExecutionContext()->isStopped();
>+}
>+
> } // namespace WebCore
>diff --git a/Source/WebCore/dom/ActiveDOMObject.h b/Source/WebCore/dom/ActiveDOMObject.h
>index ce1ec5726f6d9f67aa152d67c32a6c7f58864f9c..360ad21489adbd5fecbb817680df4b9e8f789e74 100644
>--- a/Source/WebCore/dom/ActiveDOMObject.h
>+++ b/Source/WebCore/dom/ActiveDOMObject.h
>@@ -110,6 +110,8 @@ public:
>         return adoptRef(*new PendingActivity<T>(thisObject));
>     }
> 
>+    bool isContextStopped() const;
>+
> protected:
>     explicit ActiveDOMObject(ScriptExecutionContext*);
>     virtual ~ActiveDOMObject();
>diff --git a/Source/WebCore/dom/Document.h b/Source/WebCore/dom/Document.h
>index 0a1d903417d4ca9dc0e30bbffac41d188f31bdbc..d60fe36b0403fbf305f99b1b71772238d15449be 100644
>--- a/Source/WebCore/dom/Document.h
>+++ b/Source/WebCore/dom/Document.h
>@@ -1597,6 +1597,8 @@ private:
> 
>     bool shouldEnforceHTTP09Sandbox() const;
> 
>+    bool isStopped() final { return m_hasPreparedForDestruction; }
>+
>     void platformSuspendOrStopActiveDOMObjects();
> 
>     bool domainIsRegisterable(const String&) const;
>diff --git a/Source/WebCore/dom/ScriptExecutionContext.h b/Source/WebCore/dom/ScriptExecutionContext.h
>index 3bdfcc80dbe6e6adad5877d1fee301127376d563..89a56d6fe607dcdae0bbd67763d6b1641c64b149 100644
>--- a/Source/WebCore/dom/ScriptExecutionContext.h
>+++ b/Source/WebCore/dom/ScriptExecutionContext.h
>@@ -130,6 +130,8 @@ public:
>     // Active objects are not garbage collected even if inaccessible, e.g. because their activity may result in callbacks being invoked.
>     WEBCORE_EXPORT bool canSuspendActiveDOMObjectsForDocumentSuspension(Vector<ActiveDOMObject*>* unsuspendableObjects = nullptr);
> 
>+    virtual bool isStopped() = 0;
>+
>     // Active objects can be asked to suspend even if canSuspendActiveDOMObjectsForDocumentSuspension() returns 'false' -
>     // step-by-step JS debugging is one example.
>     virtual void suspendActiveDOMObjects(ReasonForSuspension);
>diff --git a/Source/WebCore/workers/WorkerGlobalScope.cpp b/Source/WebCore/workers/WorkerGlobalScope.cpp
>index 2a2631386beca42cb874084794cbab0f2ea688ee..a6945edea93d82387baf0f7278c3f4c9a7f4f3b5 100644
>--- a/Source/WebCore/workers/WorkerGlobalScope.cpp
>+++ b/Source/WebCore/workers/WorkerGlobalScope.cpp
>@@ -107,6 +107,9 @@ String WorkerGlobalScope::origin() const
> 
> void WorkerGlobalScope::prepareForTermination()
> {
>+    ASSERT(!m_isPreparingForTermination);
>+    m_isPreparingForTermination = true;
>+
> #if ENABLE(INDEXED_DATABASE)
>     stopIndexedDatabase();
> #endif
>diff --git a/Source/WebCore/workers/WorkerGlobalScope.h b/Source/WebCore/workers/WorkerGlobalScope.h
>index 5b9a0be369c299c0fb7541078fc7656d6de79ec1..ecae0f400d0747702cbdfc63802378890bf77837 100644
>--- a/Source/WebCore/workers/WorkerGlobalScope.h
>+++ b/Source/WebCore/workers/WorkerGlobalScope.h
>@@ -156,6 +156,7 @@ private:
>     WorkerEventQueue& eventQueue() const final;
>     String resourceRequestIdentifier() const final { return m_identifier; }
>     SocketProvider* socketProvider() final;
>+    bool isStopped() final { return m_isPreparingForTermination; }
> 
>     bool shouldBypassMainWorldContentSecurityPolicy() const final { return m_shouldBypassMainWorldContentSecurityPolicy; }
>     bool isJSExecutionForbidden() const final;
>@@ -190,6 +191,7 @@ private:
>     std::unique_ptr<MicrotaskQueue> m_microtaskQueue;
> 
>     bool m_closing { false };
>+    bool m_isPreparingForTermination { false };
>     bool m_isOnline;
>     bool m_shouldBypassMainWorldContentSecurityPolicy;
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 0a53b0e12d6be27b005f1a24664bef6c5de154f2..dc18f6a10fa32d930dec39b46a54db37550d975a 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-08-27  Youenn Fablet  <youenn@apple.com>
>+
>+        MediaDevices should be collectable as soon as its document is stopped
>+        https://bugs.webkit.org/show_bug.cgi?id=189021
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * http/tests/media/media-stream/collect-media-devices.https-expected.txt: Added.
>+        * http/tests/media/media-stream/collect-media-devices.https.html: Added.
>+        * http/tests/media/media-stream/resources/mymediadevicesframe.htm: Added.
>+
> 2018-08-27  Youenn Fablet  <youenn@apple.com>
> 
>         WebKitMediaSession should be GC collectable when its document is being stopped
>diff --git a/LayoutTests/http/tests/media/media-stream/collect-media-devices.https-expected.txt b/LayoutTests/http/tests/media/media-stream/collect-media-devices.https-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..89e6149f477b552ac79a1e64dc61867aab37c061
>--- /dev/null
>+++ b/LayoutTests/http/tests/media/media-stream/collect-media-devices.https-expected.txt
>@@ -0,0 +1,4 @@
>+
>+
>+PASS Ensuring frame document gets collected after being stopped while using MediaDevices 
>+
>diff --git a/LayoutTests/http/tests/media/media-stream/collect-media-devices.https.html b/LayoutTests/http/tests/media/media-stream/collect-media-devices.https.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..9a3adc828fd4c778383f5bf699b9f2085722f0e4
>--- /dev/null
>+++ b/LayoutTests/http/tests/media/media-stream/collect-media-devices.https.html
>@@ -0,0 +1,45 @@
>+<!DOCTYPE html>
>+<meta charset="utf-8">
>+<script src="/resources/testharness.js"></script>
>+<script src="/resources/testharnessreport.js"></script>
>+<script>
>+function waitFor(duration)
>+{
>+    return new Promise((resolve) => setTimeout(resolve, duration));
>+}
>+
>+var resolveCallback, rejectCallback;
>+var promise = new Promise((resolve, reject) => {
>+    resolveCallback = resolve;
>+    rejectCallback = reject;
>+});
>+
>+async function done()
>+{
>+    try {
>+        const frameIdentifier = internals.documentIdentifier(iframe.contentDocument);
>+        iframe.src = "non-existent-frame";
>+        let counter = 0;
>+        while (++counter < 10) {
>+            if (!internals.isDocumentAlive(frameIdentifier)) {
>+                resolveCallback();
>+                return;
>+            }
>+            if (window.GCController)
>+                GCController.collect();
>+
>+            await waitFor(50);
>+        }
>+    } finally {
>+        rejectCallback("Test failed");
>+    }
>+}
>+
>+promise_test((test) => {
>+    if (!window.internals)
>+        rejectCallback("Test require internals API");
>+    return promise;
>+}, "Ensuring frame document gets collected after being stopped while using MediaDevices");
>+
>+</script>
>+<iframe src="resources/mymediadevicesframe.htm" id="iframe"></iframe>
>diff --git a/LayoutTests/http/tests/media/media-stream/resources/mymediadevicesframe.htm b/LayoutTests/http/tests/media/media-stream/resources/mymediadevicesframe.htm
>new file mode 100644
>index 0000000000000000000000000000000000000000..61362277816246bad5b33ef7cd7cd7b465da2e62
>--- /dev/null
>+++ b/LayoutTests/http/tests/media/media-stream/resources/mymediadevicesframe.htm
>@@ -0,0 +1,5 @@
>+<!DOCTYPE html>
>+<script>
>+navigator.mediaDevices.ondevicechange = () => {};
>+parent.done();
>+</script>

Actions: View | Formatted Diff | Diff

Attachments on bug 189021: 348223 | 348247 | 348251 | 348255 | 348256 | 348302 | 348328