WebAudio Node JS wrappers should not be collected if events can be fired
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 6 May 2019 21:14:12 +0000 (21:14 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 6 May 2019 21:14:12 +0000 (21:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=197533

Reviewed by Jer Noble.

Source/WebCore:

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):

LayoutTests:

* webaudio/webaudio-gc-expected.txt: Added.
* webaudio/webaudio-gc.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@244977 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/webaudio/webaudio-gc-expected.txt [new file with mode: 0644]
LayoutTests/webaudio/webaudio-gc.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/webaudio/AudioContext.h
Source/WebCore/Modules/webaudio/AudioNode.idl
Source/WebCore/Sources.txt
Source/WebCore/WebCore.xcodeproj/project.pbxproj
Source/WebCore/bindings/js/JSAudioNodeCustom.cpp [new file with mode: 0644]

index d0c163a..4474d63 100644 (file)
@@ -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-06  Ryan Haddad  <ryanhaddad@apple.com>
 
         REGRESSION: Layout test imported/w3c/web-platform-tests/resource-timing/resource-timing-level1.sub.html is frequently failing on EWS
diff --git a/LayoutTests/webaudio/webaudio-gc-expected.txt b/LayoutTests/webaudio/webaudio-gc-expected.txt
new file mode 100644 (file)
index 0000000..2ec1395
--- /dev/null
@@ -0,0 +1,3 @@
+
+PASS Web audio events while gcing 
+
diff --git a/LayoutTests/webaudio/webaudio-gc.html b/LayoutTests/webaudio/webaudio-gc.html
new file mode 100644 (file)
index 0000000..09f47f1
--- /dev/null
@@ -0,0 +1,38 @@
+<!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>
+let count = 100;
+
+function checkEvent() {
+    if (!--count)
+        done();
+}
+
+function init() {
+    let ctx = new webkitAudioContext();
+
+    let source = ctx.createOscillator();
+    source.frequency.value = 300;
+    source.type = 0;
+    source.connect(ctx.destination);
+    source.start(0);
+
+    let detector = ctx.createScriptProcessor(1024);
+    detector.onaudioprocess = checkEvent;
+    detector.connect(ctx.destination);
+}
+
+init();
+
+gc();
+
+</script>
+</body>
+</html>
index b965910..6f9318a 100644 (file)
@@ -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-06  Daniel Bates  <dabates@apple.com>
 
         Google Docs & Yahoo! Japan: Can’t compose characters with Chinese or Japanese keyboard
index eb34720..5d790a4 100644 (file)
@@ -133,6 +133,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;
 
index ac83a8b..6357ed4 100644 (file)
@@ -24,7 +24,7 @@
 
 [
     Conditional=WEB_AUDIO,
-    GenerateIsReachable=Impl,
+    CustomIsReachable,
 ] interface AudioNode : EventTarget {
     readonly attribute AudioContext context;
     readonly attribute unsigned long numberOfInputs;
index e09eb2e..ba6097e 100644 (file)
@@ -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
index 37d615c..6d9f96e 100644 (file)
                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>"; };
                                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 (file)
index 0000000..f4ea920
--- /dev/null
@@ -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