Fix a bug that hasVideo() always returns true even after we have the size info

webkit/media/android/webmediaplayer_android.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "webkit/media/android/webmediaplayer_android.h"
 
 #include "base/file_path.h"
 #include "base/logging.h"
 #include "media/base/android/media_player_bridge.h"
 #include "net/base/mime_util.h"
@@ skipping 23 matching lines @@
       buffered_(1u),
       main_loop_(MessageLoop::current()),
       pending_seek_(0),
       seeking_(false),
       did_loading_progress_(false),
       manager_(manager),
       network_state_(WebMediaPlayer::NetworkStateEmpty),
       ready_state_(WebMediaPlayer::ReadyStateHaveNothing),
       is_playing_(false),
       needs_establish_peer_(true),
+      has_size_info_(false),
       stream_texture_factory_(factory) {
   main_loop_->AddDestructionObserver(this);
   if (manager_)
     player_id_ = manager_->RegisterMediaPlayer(this);
 
   if (stream_texture_factory_.get()) {
     stream_texture_proxy_.reset(stream_texture_factory_->CreateProxy());
     stream_id_ = stream_texture_factory_->CreateStreamTexture(&texture_id_);
     ReallocateVideoFrame();
   }
@@ skipping 69 matching lines @@
   // TODO(qinmin): Remove this from WebKit::WebMediaPlayer as it is never used.
 }
 
 bool WebMediaPlayerAndroid::totalBytesKnown() {
   NOTIMPLEMENTED();
   return false;
 }
 
 bool WebMediaPlayerAndroid::hasVideo() const {
   // If we have obtained video size information before, use it.
-  if (!natural_size_.isEmpty())
-    return true;
+  if (has_size_info_)
+    return !natural_size_.isEmpty();
 
   // TODO(qinmin): need a better method to determine whether the current media
   // content contains video. Android does not provide any function to do
   // this.
   // We don't know whether the current media content has video unless
   // the player is prepared. If the player is not prepared, we fall back
   // to the mime-type. There may be no mime-type on a redirect URL.
   // In that case, we conservatively assume it contains video so that
   // enterfullscreen call will not fail.
   if (!url_.has_path())
@@ skipping 171 matching lines @@
     case MediaPlayerBridge::MEDIA_ERROR_NOT_VALID_FOR_PROGRESSIVE_PLAYBACK:
       UpdateNetworkState(WebMediaPlayer::NetworkStateFormatError);
       break;
     case MediaPlayerBridge::MEDIA_ERROR_INVALID_CODE:
       break;
   }
   client_->repaint();
 }
 
 void WebMediaPlayerAndroid::OnVideoSizeChanged(int width, int height) {
+  has_size_info_ = true;

[Miguel Garcia, 2012/11/29 20:28:02]

shouldn't you check that width or height is > 0?

[qinmin, 2012/11/29 20:39:33]

No, we don't. This is called by the android media player, so if the size is (0,
0), then that means there is no video frames. And subsequent calls to hasVideo()
will return false. 
On the otherhand, if the size is > 0, than subsequence calls to hasVideo() will
return true.
If this is never called, we will use the previous MIME check scheme.

On 2012/11/29 20:28:02, Miguel Garcia wrote:

[Miguel Garcia, 2012/11/29 20:43:36]

OK, makes sense


On 2012/11/29 20:39:33, qinmin wrote:

   if (natural_size_.width == width && natural_size_.height == height)
     return;
 
   natural_size_.width = width;
   natural_size_.height = height;
   ReallocateVideoFrame();
 }
 
 void WebMediaPlayerAndroid::UpdateNetworkState(
     WebMediaPlayer::NetworkState state) {
@@ skipping 75 matching lines @@
 
 void WebMediaPlayerAndroid::SetNeedsEstablishPeer(bool needs_establish_peer) {
   needs_establish_peer_ = needs_establish_peer;
 }
 
 void WebMediaPlayerAndroid::UpdatePlayingState(bool is_playing) {
   is_playing_ = is_playing;
 }
 
 }  // namespace webkit_media