Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1922)

Unified Diff: content/renderer/media/android/webmediaplayer_android.cc

Issue 448063003: media: Fix not-thread-safe access to variables in android media code. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: fix errata on comment Created 6 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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..2c27ffdf9a82dbfcfc8058d08a304511c879dba8 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_) {
+ 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 deferred 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(
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698