media: Fix not-thread-safe access to variables in android media code.

content/renderer/media/android/webmediaplayer_android.cc

Index: content/renderer/media/android/webmediaplayer_android.cc
diff --git a/content/renderer/media/android/webmediaplayer_android.cc b/content/renderer/media/android/webmediaplayer_android.cc
index 08a67450095360d70a557bf00d880160040498c6..418b4111a84b37c30af58d317aaf7e8d0ae12be0 100644
--- a/content/renderer/media/android/webmediaplayer_android.cc
+++ b/content/renderer/media/android/webmediaplayer_android.cc
@@ -163,6 +163,7 @@ WebMediaPlayerAndroid::WebMediaPlayerAndroid(
 }
 
 WebMediaPlayerAndroid::~WebMediaPlayerAndroid() {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   SetVideoFrameProviderClient(NULL);
   client_->setWebLayer(NULL);
 
@@ -191,6 +192,7 @@ WebMediaPlayerAndroid::~WebMediaPlayerAndroid() {
 void WebMediaPlayerAndroid::load(LoadType load_type,
                                  const blink::WebURL& url,
                                  CORSMode cors_mode) {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   ReportMediaSchemeUma(GURL(url));
 
   switch (load_type) {
@@ -256,6 +258,7 @@ void WebMediaPlayerAndroid::DidLoadMediaInfo(
     const GURL& redirected_url,
     const GURL& first_party_for_cookies,
     bool allow_stored_credentials) {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   DCHECK(!media_source_delegate_);
   if (status == MediaInfoLoader::kFailed) {
     info_loader_.reset();
@@ -270,6 +273,7 @@ void WebMediaPlayerAndroid::DidLoadMediaInfo(
 }
 
 void WebMediaPlayerAndroid::play() {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
 #if defined(VIDEO_HOLE)
   if (hasVideo() && needs_external_surface_ &&
       !player_manager_->IsInFullscreen(frame_)) {
@@ -293,6 +297,7 @@ void WebMediaPlayerAndroid::play() {
 }
 
 void WebMediaPlayerAndroid::pause() {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   Pause(true);
 }
 
@@ -349,10 +354,12 @@ void WebMediaPlayerAndroid::setRate(double rate) {
 }
 
 void WebMediaPlayerAndroid::setVolume(double volume) {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   player_manager_->SetVolume(player_id_, volume);
 }
 
 bool WebMediaPlayerAndroid::hasVideo() const {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   // If we have obtained video size information before, use it.
   if (has_size_info_)
     return !natural_size_.isEmpty();
@@ -374,6 +381,7 @@ bool WebMediaPlayerAndroid::hasVideo() const {
 }
 
 bool WebMediaPlayerAndroid::hasAudio() const {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   if (!url_.has_path())
     return false;
   std::string mime;
@@ -397,6 +405,7 @@ bool WebMediaPlayerAndroid::seeking() const {
 }
 
 double WebMediaPlayerAndroid::duration() const {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   // HTML5 spec requires duration to be NaN if readyState is HAVE_NOTHING
   if (ready_state_ == WebMediaPlayer::ReadyStateHaveNothing)
     return std::numeric_limits<double>::quiet_NaN();
@@ -408,6 +417,7 @@ double WebMediaPlayerAndroid::duration() const {
 }
 
 double WebMediaPlayerAndroid::timelineOffset() const {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   base::Time timeline_offset;
   if (media_source_delegate_)
     timeline_offset = media_source_delegate_->GetTimelineOffset();
@@ -419,6 +429,7 @@ double WebMediaPlayerAndroid::timelineOffset() const {
 }
 
 double WebMediaPlayerAndroid::currentTime() const {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   // If the player is processing a seek, return the seek time.
   // Blink may still query us if updatePlaybackState() occurs while seeking.
   if (seeking()) {
@@ -467,6 +478,7 @@ bool WebMediaPlayerAndroid::EnsureTextureBackedSkBitmap(GrContext* gr,
                                                         const WebSize& size,
                                                         GrSurfaceOrigin origin,
                                                         GrPixelConfig config) {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   if (!bitmap.getTexture() || bitmap.width() != size.width
       || bitmap.height() != size.height) {
     if (!gr)
@@ -497,6 +509,7 @@ bool WebMediaPlayerAndroid::EnsureTextureBackedSkBitmap(GrContext* gr,
 void WebMediaPlayerAndroid::paint(blink::WebCanvas* canvas,
                                   const blink::WebRect& rect,
                                   unsigned char alpha) {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   scoped_ptr<blink::WebGraphicsContext3DProvider> provider =
     scoped_ptr<blink::WebGraphicsContext3DProvider>(blink::Platform::current(
       )->createSharedOffscreenGraphicsContext3DProvider());
@@ -545,6 +558,7 @@ bool WebMediaPlayerAndroid::copyVideoTextureToPlatformTexture(
     unsigned int type,
     bool premultiply_alpha,
     bool flip_y) {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   // Don't allow clients to copy an encrypted video frame.
   if (needs_external_surface_)
     return false;
@@ -563,16 +577,6 @@ bool WebMediaPlayerAndroid::copyVideoTextureToPlatformTexture(
           mailbox_holder->texture_target == GL_TEXTURE_EXTERNAL_OES) ||
          (is_remote_ && mailbox_holder->texture_target == GL_TEXTURE_2D));
 
-  // For hidden video element (with style "display:none"), ensure the texture
-  // size is set.
-  if (!is_remote_ &&
-      (cached_stream_texture_size_.width != natural_size_.width ||
-       cached_stream_texture_size_.height != natural_size_.height)) {
-    stream_texture_factory_->SetStreamTextureSize(
-        stream_id_, gfx::Size(natural_size_.width, natural_size_.height));
-    cached_stream_texture_size_ = natural_size_;
-  }
-
   web_graphics_context->waitSyncPoint(mailbox_holder->sync_point);
 
   // Ensure the target of texture is set before copyTextureCHROMIUM, otherwise
@@ -606,6 +610,7 @@ bool WebMediaPlayerAndroid::copyVideoTextureToPlatformTexture(
 }
 
 bool WebMediaPlayerAndroid::hasSingleSecurityOrigin() const {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   if (player_type_ != MEDIA_PLAYER_TYPE_URL)
     return true;
 
@@ -626,6 +631,7 @@ bool WebMediaPlayerAndroid::hasSingleSecurityOrigin() const {
 }
 
 bool WebMediaPlayerAndroid::didPassCORSAccessCheck() const {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   if (info_loader_)
     return info_loader_->DidPassCORSAccessCheck();
   return false;
@@ -665,6 +671,7 @@ unsigned WebMediaPlayerAndroid::videoDecodedByteCount() const {
 
 void WebMediaPlayerAndroid::OnMediaMetadataChanged(
     const base::TimeDelta& duration, int width, int height, bool success) {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   bool need_to_signal_duration_changed = false;
 
   if (url_.SchemeIs("file"))
@@ -768,6 +775,7 @@ void WebMediaPlayerAndroid::OnMediaError(int error_type) {
 }
 
 void WebMediaPlayerAndroid::OnVideoSizeChanged(int width, int height) {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   has_size_info_ = true;
   if (natural_size_.width == width && natural_size_.height == height)
     return;
@@ -787,6 +795,9 @@ void WebMediaPlayerAndroid::OnVideoSizeChanged(int width, int height) {
     SetNeedsEstablishPeer(true);
   }
 #endif  // defined(VIDEO_HOLE)
+  natural_size_.width = width;
+  natural_size_.height = height;
+
   // When play() gets called, |natural_size_| may still be empty and
   // EstablishSurfaceTexturePeer() will not get called. As a result, the video
   // may play without a surface texture. When we finally get the valid video
@@ -795,10 +806,16 @@ void WebMediaPlayerAndroid::OnVideoSizeChanged(int width, int height) {
   if (!paused() && needs_establish_peer_)
     EstablishSurfaceTexturePeer();
 
-  natural_size_.width = width;
-  natural_size_.height = height;
   ReallocateVideoFrame();
 
+  // For hidden video element (with style "display:none"), ensure the texture
+  // size is set.
+  if (!is_remote_ && cached_stream_texture_size_ != natural_size_) {

[dshwang, 2014/08/11 08:43:17]

qinmin@, could you review this part again?
Previous patch has issue; if video size is change in remote mode,
WebMediaPlayerAndroid never report the size to stream texture although the mode
is changed to not-remote mode.

So I change to set the size in EstablishSurfaceTexturePeer() if needed.
WDYT?

+    stream_texture_factory_->SetStreamTextureSize(
+        stream_id_, gfx::Size(natural_size_.width, natural_size_.height));
+    cached_stream_texture_size_ = natural_size_;
+  }
+
   // Lazily allocate compositing layer.
   if (!video_weblayer_) {
     video_weblayer_.reset(new WebLayerImpl(
@@ -972,7 +989,6 @@ void WebMediaPlayerAndroid::Pause(bool is_media_related_action) {
 
 void WebMediaPlayerAndroid::DrawRemotePlaybackText(
     const std::string& remote_playback_message) {
-
   DCHECK(main_thread_checker_.CalledOnValidThread());
   if (!video_weblayer_)
     return;
@@ -1094,6 +1110,7 @@ void WebMediaPlayerAndroid::DrawRemotePlaybackText(
 }
 
 void WebMediaPlayerAndroid::ReallocateVideoFrame() {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   if (needs_external_surface_) {
     // VideoFrame::CreateHoleFrame is only defined under VIDEO_HOLE.
 #if defined(VIDEO_HOLE)
@@ -1138,12 +1155,22 @@ void WebMediaPlayerAndroid::SetVideoFrameProviderClient(
   video_frame_provider_client_ = client;
 
   // Set the callback target when a frame is produced.
-  if (stream_texture_proxy_)
+  if (stream_texture_proxy_) {
     stream_texture_proxy_->SetClient(client);
+    // If client exists, the compositor thread calls it. At that time,
+    // stream_id_, needs_external_surface_, is_remote_ can be accessed because
+    // the main thread is blocked.
+    if (client && !stream_texture_proxy_initialized_ && stream_id_ &&
+        !needs_external_surface_ && !is_remote_) {
+      stream_texture_proxy_->BindToCurrentThread(stream_id_);
+      stream_texture_proxy_initialized_ = true;
+    }
+  }
 }
 
 void WebMediaPlayerAndroid::SetCurrentFrameInternal(
     scoped_refptr<media::VideoFrame>& video_frame) {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   base::AutoLock auto_lock(current_frame_lock_);
   current_frame_ = video_frame;
 }
@@ -1155,16 +1182,6 @@ scoped_refptr<media::VideoFrame> WebMediaPlayerAndroid::GetCurrentFrame() {
     video_frame = current_frame_;
   }
 
-  if (!stream_texture_proxy_initialized_ && stream_texture_proxy_ &&
-      stream_id_ && !needs_external_surface_ && !is_remote_) {
-    gfx::Size natural_size = video_frame->natural_size();
-    // TODO(sievers): These variables are accessed on the wrong thread here.
-    stream_texture_proxy_->BindToCurrentThread(stream_id_);
-    stream_texture_factory_->SetStreamTextureSize(stream_id_, natural_size);
-    stream_texture_proxy_initialized_ = true;
-    cached_stream_texture_size_ = natural_size;
-  }
-
   return video_frame;
 }
 
@@ -1173,6 +1190,7 @@ void WebMediaPlayerAndroid::PutCurrentFrame(
 }
 
 void WebMediaPlayerAndroid::TryCreateStreamTextureProxyIfNeeded() {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   // Already created.
   if (stream_texture_proxy_)
     return;
@@ -1192,15 +1210,25 @@ void WebMediaPlayerAndroid::TryCreateStreamTextureProxyIfNeeded() {
 }
 
 void WebMediaPlayerAndroid::EstablishSurfaceTexturePeer() {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   if (!stream_texture_proxy_)
     return;
 
   if (stream_texture_factory_.get() && stream_id_)
     stream_texture_factory_->EstablishPeer(stream_id_, player_id_);
+
+  // Set the differed size because the size was changed in remote mode.
+  if (!is_remote_ && cached_stream_texture_size_ != natural_size_) {
+    stream_texture_factory_->SetStreamTextureSize(
+        stream_id_, gfx::Size(natural_size_.width, natural_size_.height));
+    cached_stream_texture_size_ = natural_size_;
+  }
+
   needs_establish_peer_ = false;
 }
 
 void WebMediaPlayerAndroid::DoCreateStreamTexture() {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   DCHECK(!stream_id_);
   DCHECK(!texture_id_);
   stream_id_ = stream_texture_factory_->CreateStreamTexture(