[webkit-reviews] review granted: [Bug 190438] Implement error handler of MediaRecorder : [Attachment 352215] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Wendy <yuhan_wu at apple.com>'s
request for review:
Bug 190438: Implement error handler of MediaRecorder
https://bugs.webkit.org/show_bug.cgi?id=190438

Attachment 352215: Patch

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




--- Comment #37 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 352215
  --> https://bugs.webkit.org/attachment.cgi?id=352215
Patch

LGTM too :)

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

>> Source/WebCore/Modules/mediarecorder/MediaRecorder.cpp:59
>> +	m_isActive = false;
> 
> Should we remove ourselves as an observer here as well? It seems like it'd be
good to do it as early as possible.
> 
> Then I think we would not need the m_isActive flag as didAddOrRemoveTrack()
could not get called after we're no longer active.

We discussed this with Wendy yesterday and thought that this might not be
worth.
Contrary to observer being call for every frame, this observer will never be
called.
But we will in most cases try to remove the observer from the observer map
twice.