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 f5dd958ba5c10fd7a5cce1640997d7ee6dcfd3a0..52e2cbac949c1c41941783cc8d2e140798ca2b1c 100644
--- a/content/renderer/media/android/webmediaplayer_android.cc
+++ b/content/renderer/media/android/webmediaplayer_android.cc
@@ -160,6 +160,7 @@ WebMediaPlayerAndroid::WebMediaPlayerAndroid(
 }
 
 WebMediaPlayerAndroid::~WebMediaPlayerAndroid() {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   SetVideoFrameProviderClient(NULL);
   client_->setWebLayer(NULL);
 
@@ -188,6 +189,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::load(LoadType load_type,
 }
 
 void WebMediaPlayerAndroid::DidLoadMediaInfo(MediaInfoLoader::Status status) {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   DCHECK(!media_source_delegate_);
   if (status == MediaInfoLoader::kFailed) {
     info_loader_.reset();
@@ -269,6 +272,7 @@ void WebMediaPlayerAndroid::DidLoadMediaInfo(MediaInfoLoader::Status status) {
 }
 
 void WebMediaPlayerAndroid::play() {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
 #if defined(VIDEO_HOLE)
   if (hasVideo() && needs_external_surface_ &&
       !player_manager_->IsInFullscreen(frame_)) {
@@ -292,6 +296,7 @@ void WebMediaPlayerAndroid::play() {
 }
 
 void WebMediaPlayerAndroid::pause() {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   Pause(true);
 }
 
@@ -348,10 +353,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();
@@ -373,6 +380,7 @@ bool WebMediaPlayerAndroid::hasVideo() const {
 }
 
 bool WebMediaPlayerAndroid::hasAudio() const {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   if (!url_.has_path())
     return false;
   std::string mime;
@@ -396,6 +404,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();
@@ -407,6 +416,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();
@@ -418,6 +428,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()) {
@@ -466,6 +477,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)
@@ -496,6 +508,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());
@@ -544,6 +557,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;
@@ -562,16 +576,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
@@ -605,6 +609,7 @@ bool WebMediaPlayerAndroid::copyVideoTextureToPlatformTexture(
 }
 
 bool WebMediaPlayerAndroid::hasSingleSecurityOrigin() const {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   if (info_loader_)
     return info_loader_->HasSingleOrigin();
   // The info loader may have failed.
@@ -614,6 +619,7 @@ bool WebMediaPlayerAndroid::hasSingleSecurityOrigin() const {
 }
 
 bool WebMediaPlayerAndroid::didPassCORSAccessCheck() const {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   if (info_loader_)
     return info_loader_->DidPassCORSAccessCheck();
   return false;
@@ -653,6 +659,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"))
@@ -756,6 +763,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,13 @@ void WebMediaPlayerAndroid::OnVideoSizeChanged(int width, int height) {
   natural_size_.height = height;
   ReallocateVideoFrame();
 
+  // For hidden video element (with style "display:none"), ensure the texture
+  // size is set.
+  if (!is_remote_) {
+    stream_texture_factory_->SetStreamTextureSize(
+        stream_id_, gfx::Size(natural_size_.width, natural_size_.height));
+  }
+
   // Lazily allocate compositing layer.
   if (!video_weblayer_) {
     video_weblayer_.reset(new WebLayerImpl(cc::VideoLayer::Create(this)));
@@ -954,7 +969,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;
@@ -1076,6 +1090,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)
@@ -1120,12 +1135,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_);

[qinmin, 2014/08/08 22:02:58]

I don't think this is correct, the stream_texture_proxy is created on main
thread, but lives on the compositor thread

[dshwang, 2014/08/09 06:01:26]

As above comments, I bind the proxy to the compositor thread because only
compositor thread calls this method with client argument.

[qinmin, 2014/08/10 18:33:51]

i see, i missed the non-null check on client.

On 2014/08/09 06:01:26, dshwang wrote:

+      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;
 }
@@ -1137,16 +1162,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;
 }
 
@@ -1155,6 +1170,7 @@ void WebMediaPlayerAndroid::PutCurrentFrame(
 }
 
 void WebMediaPlayerAndroid::TryCreateStreamTextureProxyIfNeeded() {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   // Already created.
   if (stream_texture_proxy_)
     return;
@@ -1174,6 +1190,7 @@ void WebMediaPlayerAndroid::TryCreateStreamTextureProxyIfNeeded() {
 }
 
 void WebMediaPlayerAndroid::EstablishSurfaceTexturePeer() {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   if (!stream_texture_proxy_)
     return;
 
@@ -1183,6 +1200,7 @@ void WebMediaPlayerAndroid::EstablishSurfaceTexturePeer() {
 }
 
 void WebMediaPlayerAndroid::DoCreateStreamTexture() {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
   DCHECK(!stream_id_);
   DCHECK(!texture_id_);
   stream_id_ = stream_texture_factory_->CreateStreamTexture(