[webkit-reviews] review denied: [Bug 66230] Also release media resources when media is completely loaded. : [Attachment 104161] Proposed patch 2.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Aug 17 07:05:13 PDT 2011
Eric Carlson <eric.carlson at apple.com> has denied Hao Zheng
<zhenghao at chromium.org>'s request for review:
Bug 66230: Also release media resources when media is completely loaded.
https://bugs.webkit.org/show_bug.cgi?id=66230
Attachment 104161: Proposed patch 2.
https://bugs.webkit.org/attachment.cgi?id=104161&action=review
------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=104161&action=review
> Source/WebCore/html/HTMLMediaElement.cpp:2654
> +void HTMLMediaElement::documentWillBecomeInactive()
> +{
> +#if !ENABLE(PLUGIN_PROXY_FOR_VIDEO)
> + LOG(Media, "HTMLMediaElement::documentWillBecomeInactive");
> + if (m_player)
> + m_player->willBecomeInactive();
> +#endif
> +}
> +
> +void HTMLMediaElement::documentDidBecomeActive()
> +{
> +#if !ENABLE(PLUGIN_PROXY_FOR_VIDEO)
> + LOG(Media, "HTMLMediaElement::documentDidBecomeActive");
> + if (m_player)
> + m_player->didBecomeActive();
> +#endif
> +}
> +
These new methods are unnecessary. documentWillBecomeInactive is called by
Document::detach, *after* it calls stopActiveDOMObjects(). HTMLMediaElement is
an ActiveDOMObject.
> Source/WebKit/chromium/src/WebMediaPlayerClientImpl.cpp:243
> +void WebMediaPlayerClientImpl::willBecomeInactive()
> +{
> + if (m_webMediaPlayer.get())
> + m_webMediaPlayer->willBecomeInactive();
> +}
> +
> +void WebMediaPlayerClientImpl::didBecomeActive()
> +{
> + if (m_webMediaPlayer.get())
> + m_webMediaPlayer->didBecomeActive();
> +}
> +
I assume that the Chromium media player sets network and ready states correctly
when it unloads cached media data? Where are the tests to show that the
behavior is correct.?
More information about the webkit-reviews
mailing list