Attachment 369139 Details for Bug 197533 – Patch for landing

[patch] Patch for landing

bug-197533-20190506110152.patch (text/plain), 10.00 KB, created by youenn fablet on 2019-05-06 11:01:53 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2019-05-06 11:01:53 PDT

Size: 10.00 KB

patch

obsolete

>Subversion Revision: 244802
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index a61cd4ddd7a41a52958cd0a63d428f96bb43c5d2..ac6cadadf1f60befca1ba6bd55d7f9178945bb72 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2019-05-06  Youenn Fablet  <youenn@apple.com>
>+
>+        WebAudio Node JS wrappers should not be collected if events can be fired
>+        https://bugs.webkit.org/show_bug.cgi?id=197533
>+
>+        Reviewed by Jer Noble.
>+
>+        Before the patch, some web audio nodes could fire event listeners, but were not protected from GC.
>+        Use CustomIsReachable to ensure theses nodes can be collected if:
>+        - their AudioContext is stopped (typically due to document being navigated away).
>+        - their AudioContext is closed.
>+        - nodes do not have event listeners.
>+
>+        Covered by WPT mediacapture-streams/MediaStreamTrack-MediaElement-disabled-audio-is-silence.https.html and
>+        WPT webaudio/the-audio-api/the-mediaelementaudiosourcenode-interface/mediaElementAudioSourceToScriptProcessorTest.html
>+        and web audio WebRTC tests.
>+        Specific newly added test: webaudio/webaudio-gc.html
>+
>+        * Modules/webaudio/AudioContext.h:
>+        (WebCore::AudioContext::isClosed const):
>+        * Modules/webaudio/AudioNode.idl:
>+        * Sources.txt:
>+        * WebCore.xcodeproj/project.pbxproj:
>+        * bindings/js/JSAudioNodeCustom.cpp: Added.
>+        (WebCore::JSAudioNodeOwner::isReachableFromOpaqueRoots):
>+
> 2019-05-01  Youenn Fablet  <youenn@apple.com>
> 
>         Cache.add and Cache.addAll should compute a correct response body size
>diff --git a/Source/WebCore/Modules/webaudio/AudioContext.h b/Source/WebCore/Modules/webaudio/AudioContext.h
>index 3705c4953eac66052d6bd310fbe4fa8038ae1bc5..b8a24a6ce03755087cb8ce7b4d0644cae21fd147 100644
>--- a/Source/WebCore/Modules/webaudio/AudioContext.h
>+++ b/Source/WebCore/Modules/webaudio/AudioContext.h
>@@ -130,6 +130,7 @@ public:
> 
>     enum class State { Suspended, Running, Interrupted, Closed };
>     State state() const;
>+    bool isClosed() const { return m_state == State::Closed; }
> 
>     bool wouldTaintOrigin(const URL&) const;
> 
>diff --git a/Source/WebCore/Modules/webaudio/AudioNode.idl b/Source/WebCore/Modules/webaudio/AudioNode.idl
>index ac83a8bc9345f46af264a380a03c123ce66e5423..6357ed47499eb84838b566e068b7c63533848fbe 100644
>--- a/Source/WebCore/Modules/webaudio/AudioNode.idl
>+++ b/Source/WebCore/Modules/webaudio/AudioNode.idl
>@@ -24,7 +24,7 @@
> 
> [
>     Conditional=WEB_AUDIO,
>-    GenerateIsReachable=Impl,
>+    CustomIsReachable,
> ] interface AudioNode : EventTarget {
>     readonly attribute AudioContext context;
>     readonly attribute unsigned long numberOfInputs;
>diff --git a/Source/WebCore/Sources.txt b/Source/WebCore/Sources.txt
>index e09eb2eeb03b4fdc274abb95833dad7066ca218c..ba6097e9b747080ec81095fc0c0f8288b0616831 100644
>--- a/Source/WebCore/Sources.txt
>+++ b/Source/WebCore/Sources.txt
>@@ -431,6 +431,7 @@ animation/DocumentTimeline.cpp
> animation/KeyframeEffect.cpp
> animation/WebAnimation.cpp
> 
>+bindings/js/JSAudioNodeCustom.cpp
> bindings/js/CachedModuleScriptLoader.cpp
> bindings/js/CachedScriptFetcher.cpp
> bindings/js/CallTracer.cpp
>diff --git a/Source/WebCore/WebCore.xcodeproj/project.pbxproj b/Source/WebCore/WebCore.xcodeproj/project.pbxproj
>index 7bfdca2fbf1b9f6350bdac29ddd8c9c918275eec..6c29d8cb75ea0fc05838cca41bad5693ac4fc881 100644
>--- a/Source/WebCore/WebCore.xcodeproj/project.pbxproj
>+++ b/Source/WebCore/WebCore.xcodeproj/project.pbxproj
>@@ -7160,6 +7160,7 @@
> 		3FFFF9A7159D9A550020BBD5 /* WebKitCSSViewportRule.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WebKitCSSViewportRule.h; sourceTree = "<group>"; };
> 		3FFFF9AB159D9B060020BBD5 /* ViewportStyleResolver.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ViewportStyleResolver.cpp; sourceTree = "<group>"; };
> 		3FFFF9AC159D9B060020BBD5 /* ViewportStyleResolver.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ViewportStyleResolver.h; sourceTree = "<group>"; };
>+		410626A72280A22A006D1B59 /* JSAudioNodeCustom.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSAudioNodeCustom.cpp; sourceTree = "<group>"; };
> 		4107908A1FC3E4F20061B27A /* ClientOrigin.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ClientOrigin.h; sourceTree = "<group>"; };
> 		410B7E711045FAB000D8224F /* JSMessageEventCustom.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSMessageEventCustom.cpp; sourceTree = "<group>"; };
> 		41103AA71E39790A00769F03 /* RealtimeOutgoingAudioSource.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = RealtimeOutgoingAudioSource.cpp; sourceTree = "<group>"; };
>@@ -20575,6 +20576,7 @@
> 				71EFCEDE202B39C700D7C411 /* JSAnimationEffectCustom.cpp */,
> 				71025ED51F99F147004A250C /* JSAnimationTimelineCustom.cpp */,
> 				BC2ED6BB0C6BD2F000920BFF /* JSAttrCustom.cpp */,
>+				410626A72280A22A006D1B59 /* JSAudioNodeCustom.cpp */,
> 				BE6DF70E171CA2DA00DD52B8 /* JSAudioTrackCustom.cpp */,
> 				BE6DF710171CA2DA00DD52B8 /* JSAudioTrackListCustom.cpp */,
> 				576082562011BE0200116678 /* JSAuthenticatorResponseCustom.cpp */,
>diff --git a/Source/WebCore/bindings/js/JSAudioNodeCustom.cpp b/Source/WebCore/bindings/js/JSAudioNodeCustom.cpp
>new file mode 100644
>index 0000000000000000000000000000000000000000..f4ea920a049b9c5d96ce8f76b1cecdc388ece61e
>--- /dev/null
>+++ b/Source/WebCore/bindings/js/JSAudioNodeCustom.cpp
>@@ -0,0 +1,60 @@
>+/*
>+ * Copyright (C) 2019 Apple Inc. All rights reserved.
>+ *
>+ * Redistribution and use in source and binary forms, with or without
>+ * modification, are permitted provided that the following conditions
>+ * are met:
>+ * 1. Redistributions of source code must retain the above copyright
>+ *    notice, this list of conditions and the following disclaimer.
>+ * 2. Redistributions in binary form must reproduce the above copyright
>+ *    notice, this list of conditions and the following disclaimer in the
>+ *    documentation and/or other materials provided with the distribution.
>+ *
>+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
>+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
>+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
>+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
>+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
>+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
>+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>+ */
>+
>+#include "config.h"
>+
>+#if ENABLE(WEB_AUDIO)
>+
>+#include "AudioContext.h"
>+#include "JSAudioNode.h"
>+
>+namespace WebCore {
>+using namespace JSC;
>+
>+bool JSAudioNodeOwner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown> handle, void*, SlotVisitor& visitor, const char** reason)
>+{
>+    auto& node = jsCast<JSAudioNode*>(handle.slot()->asCell())->wrapped();
>+
>+    if (node.isFiringEventListeners()) {
>+        if (UNLIKELY(reason))
>+            *reason = "AudioNode is firing event listeners";
>+        return true;
>+    }
>+
>+    if (!node.context().hasPendingActivity() || node.context().isClosed())
>+        return false;
>+
>+    if (node.hasEventListeners()) {
>+        if (UNLIKELY(reason))
>+            *reason = "AudioNode has event listeners";
>+        return true;
>+    }
>+
>+    return visitor.containsOpaqueRoot(&node);
>+}
>+
>+} // namespace WebCore
>+
>+#endif
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 8052416196bc1190de2bbcbbe7700b950cb11f10..466cd37920471772eb6b98fcbce3e8713c9a05e5 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2019-05-06  Youenn Fablet  <youenn@apple.com>
>+
>+        WebAudio Node JS wrappers should not be collected if events can be fired
>+        https://bugs.webkit.org/show_bug.cgi?id=197533
>+
>+        Reviewed by Jer Noble.
>+
>+        * webaudio/webaudio-gc-expected.txt: Added.
>+        * webaudio/webaudio-gc.html: Added.
>+
> 2019-05-03  Youenn Fablet  <youenn@apple.com>
> 
>         [iOS] set the default maximum camera count to 1 for enumerateDevices
>diff --git a/LayoutTests/webaudio/webaudio-gc-expected.txt b/LayoutTests/webaudio/webaudio-gc-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..e4ad617155cff9f52beb738e9c550692cce46a9f
>--- /dev/null
>+++ b/LayoutTests/webaudio/webaudio-gc-expected.txt
>@@ -0,0 +1,3 @@
>+
>+PASS ScriptProcessorNode and gc 
>+
>diff --git a/LayoutTests/webaudio/webaudio-gc.html b/LayoutTests/webaudio/webaudio-gc.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..2b0b4683e8dc16d5f65278d052f0fa2d1e5d8a12
>--- /dev/null
>+++ b/LayoutTests/webaudio/webaudio-gc.html
>@@ -0,0 +1,34 @@
>+<!doctype html>
>+<html>
>+<head>
>+<title>Web audio events while gcing</title>
>+</head>
>+<body>
>+<script src=../resources/testharness.js></script>
>+<script src=../resources/testharnessreport.js></script>
>+<script src=../resources/gc.js></script>
>+<script>
>+
>+promise_test(async (t) => {
>+    const stream = await navigator.mediaDevices.getUserMedia({audio: true});
>+    let resolve;
>+    const promise = new Promise(r => resolve = r);
>+    function init() {
>+        let ctx = new webkitAudioContext();
>+        let source = ctx.createMediaStreamSource(stream);
>+        let detector = ctx.createScriptProcessor(1024);
>+        let count = 100;
>+        detector.onaudioprocess = t.step_func(function (e) {
>+            if (!--count)
>+                resolve();
>+        });
>+        source.connect(detector);
>+        detector.connect(ctx.destination);
>+    }
>+    init();
>+    gc();
>+    return promise;
>+}, "ScriptProcessorNode and gc");
>+</script>
>+</body>
>+</html>

Flags:

ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 197533: 368835 | 368890 | 368907 | 369124 | 369139 | 369152 | 369160