[webkit-reviews] review granted: [Bug 109857] Factor MediaSource methods out of MediaPlayer & MediaPlayerPrivate and into a new MediaSourcePrivate interface. : [Attachment 189572] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Aaron Colwell
<acolwell at chromium.org>'s request for review:
Bug 109857: Factor MediaSource methods out of MediaPlayer & MediaPlayerPrivate
and into a new MediaSourcePrivate interface.
https://bugs.webkit.org/show_bug.cgi?id=109857

Attachment 189572: Patch
https://bugs.webkit.org/attachment.cgi?id=189572&action=review

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


I don't feel super confident in this review, but this patch seems plausible to
me.  It looks like the apple-win bubble is red, which might indicate an issue
that we should resolve before landing this patch.

> Source/WebKit/chromium/src/WebMediaPlayerClientImpl.cpp:54
> +// FIXME(acolwell): Remove this class once the Chromium code implements its
own

(acolwell) <--- We usually skip the name annotations in WebKit.

> Source/WebKit/chromium/src/WebMediaPlayerClientImpl.cpp:309
> +	   mediaSource->open(new
WebMediaSourceClientImpl(m_webMediaPlayer.get()));

It's ok that mediaSource will get deleted immediately after this line?