[webkit-reviews] review granted: [Bug 102356] leak: AudioContext objects are leaking. : [Attachment 174846] with guard #if ENABLE(WEB_AUDIO)

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Ilya Tikhonovsky
<loislo at chromium.org>'s request for review:
Bug 102356: leak: AudioContext objects are leaking.
https://bugs.webkit.org/show_bug.cgi?id=102356

Attachment 174846: with guard #if ENABLE(WEB_AUDIO)
https://bugs.webkit.org/attachment.cgi?id=174846&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=174846&action=review


> Source/WebCore/Modules/webaudio/AudioContext.cpp:317
>      m_document = 0; // document is going away

There's a bug on file about m_document not being needed in this class because
ActiveDOMObject already provides one.

> Source/WebCore/Modules/webaudio/AudioContext.cpp:319
> +    // Usually ScriptExecutionController calls stop twice.

What is a ScriptExecutionController?  Grepping in WebCore doesn't find anything
by that name.

> Source/WebCore/Modules/webaudio/AudioContext.h:260
> +    // ScriptExecutionContext calls stop twice.
> +    // We'd like to schedule only one stop action for them.

Is that a bug with ScriptExecutionContext?  Should we file a but and
investigate?

> LayoutTests/inspector-protocol/nmi-webaudio-leak-test.html:3
> +<script type="text/javascript"
src="../http/tests/inspector-protocol/resources/protocol-test.js"></script>

type="text/javascript"	<--- this isn't needed