Eliminate MediaPlayer abstraction(seek, duration, poster APIs)

Source/core/html/HTMLMediaElement.cpp

Index: Source/core/html/HTMLMediaElement.cpp
diff --git a/Source/core/html/HTMLMediaElement.cpp b/Source/core/html/HTMLMediaElement.cpp
index 6d5c10d55dfe3ce3ff77473f5b76730b946d5a8e..343b50b79ab0fbfaaece47a83161b630dec5107a 100644
--- a/Source/core/html/HTMLMediaElement.cpp
+++ b/Source/core/html/HTMLMediaElement.cpp
@@ -1979,7 +1979,7 @@ void HTMLMediaElement::seek(double time, ExceptionState& exceptionState)
     scheduleEvent(EventTypeNames::seeking);
 
     // 9 - Set the current playback position to the given new playback position
-    m_player->seek(time);
+    webMediaPlayer()->seek(time);

[philipj_slow, 2014/07/13 22:12:49]

I missed in this in the previous review, but the m_player check in this function
doesn't guarantee that webMediaPlayer() is not null, or at least not obviously
so. What makes it safe?

[Srirama, 2014/07/14 07:50:54]

The check at the top if (m_readyState == HAVE_NOTHING) return; ensures that
webmediaplayer is not null(this is how we arrived at the bug 382721 and solved
before landing these patches).
On 2014/07/13 22:12:49, philipj wrote:

[philipj_slow, 2014/07/14 10:54:57]

OK. So since m_readyState >= HAVE_METADATA now implies that webMediaPlayer() is
not null, the check at the top is confusing. The !m_player part of the
expression never matters and can be removed.

[Srirama, 2014/07/14 12:18:21]

Done.

 
     // 10-14 are handled, if necessary, when the engine signals a readystate change or otherwise
     // satisfies seek completion and signals a time change.
@@ -2019,7 +2019,10 @@ bool HTMLMediaElement::seeking() const
 
 void HTMLMediaElement::refreshCachedTime() const
 {
-    m_cachedTime = m_player->currentTime();
+    if (!webMediaPlayer())
+        return;
+
+    m_cachedTime = webMediaPlayer()->currentTime();
     m_cachedTimeWallClockUpdateTime = WTF::currentTime();
 }
 
@@ -2043,7 +2046,7 @@ double HTMLMediaElement::currentTime() const
     static const double minCachedDeltaForWarning = 0.01;
 #endif
 
-    if (!m_player)
+    if (!m_player || !webMediaPlayer())

[philipj_slow, 2014/07/13 22:12:48]

!m_player implies !webMediaPlayer(), so just the latter check here is enough.
m_player is made unused in this function anyway.

[Srirama, 2014/07/14 07:50:54]

Done.

         return 0;
 
     if (m_seeking) {
@@ -2053,7 +2056,7 @@ double HTMLMediaElement::currentTime() const
 
     if (m_cachedTime != MediaPlayer::invalidTime() && m_paused) {
 #if LOG_CACHED_TIME_WARNINGS
-        double delta = m_cachedTime - m_player->currentTime();
+        double delta = m_cachedTime - webMediaPlayer()->currentTime();
         if (delta > minCachedDeltaForWarning)
             WTF_LOG(Media, "HTMLMediaElement::currentTime - WARNING, cached time is %f seconds off of media time when paused", delta);
 #endif
@@ -2092,7 +2095,7 @@ double HTMLMediaElement::duration() const
     if (m_mediaSource)
         return m_mediaSource->duration();
 
-    return m_player->duration();
+    return webMediaPlayer()->duration();

[philipj_slow, 2014/07/13 22:12:48]

Change the !m_player check at the top to match?

[Srirama, 2014/07/14 07:50:54]

Removed !m_player check but !webmediaplayer() check is not requried as the other
check (m_readyState < HAVE_METADATA) makes sure that webmediaplayer is not null.
On 2014/07/13 22:12:48, philipj wrote:

[philipj_slow, 2014/07/14 10:54:57]

The check at the top is equivalent to the one in seek, so maybe simplify it in
the same way? Both of these can be done in a separate CL if you think they'd
pollute this one, though.

[philipj_slow, 2014/07/14 11:06:56]

My bad, you already fixed this one.

 }
 
 bool HTMLMediaElement::paused() const
@@ -3026,7 +3029,7 @@ void HTMLMediaElement::mediaPlayerTimeChanged()
     invalidateCachedTime();
 
     // 4.8.10.9 steps 12-14. Needed if no ReadyState change is associated with the seek.
-    if (m_seeking && m_readyState >= HAVE_CURRENT_DATA && !m_player->seeking())
+    if (m_seeking && m_readyState >= HAVE_CURRENT_DATA && !webMediaPlayer()->seeking())
         finishSeek();
 
     // Always call scheduleTimeupdateEvent when the media engine reports a time discontinuity,