[webkit-reviews] review denied: [Bug 170355] fast/mediastream/MediaStream-page-muted.html times out and asserts : [Attachment 312300] Proposed patch.

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has denied Eric Carlson
<eric.carlson at apple.com>'s request for review:
Bug 170355: fast/mediastream/MediaStream-page-muted.html times out and asserts
https://bugs.webkit.org/show_bug.cgi?id=170355

Attachment 312300: Proposed patch.

https://bugs.webkit.org/attachment.cgi?id=312300&action=review




--- Comment #22 from Geoffrey Garen <ggaren at apple.com> ---
Comment on attachment 312300
  --> https://bugs.webkit.org/attachment.cgi?id=312300
Proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=312300&action=review

>>> Source/WebCore/Modules/mediastream/MediaStream.cpp:526
>>> +	 return hasEventListeners(eventNames().activeEvent) ||
hasEventListeners(eventNames().inactiveEvent);
>> 
>> Can you explain why having a listener counts as having activity? This seems
odd.
>> 
>> Normally, it is about something being active / playing or having pending
events.
> 
> I suspect this would cause leaks if there is any event listener registered.

I agree. I think this needs to check m_isActive or m_isProducingData --
whichever one indicates whether the stream will ever produce new data in the
future.