Pause EME player whose external surface is stolen by another player

content/browser/media/android/browser_media_player_manager.cc

 // Copyright 2013 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 "content/browser/media/android/browser_media_player_manager.h"
 
 #include "base/android/scoped_java_ref.h"
 #include "base/command_line.h"
 #include "content/browser/android/content_view_core_impl.h"
 #include "content/browser/media/android/browser_demuxer_android.h"
@@ skipping 330 matching lines @@
                                                            jobject surface) {
   MediaPlayerAndroid* player = GetPlayer(player_id);
   if (player) {
     player->SetVideoSurface(
         gfx::ScopedJavaSurface::AcquireExternalSurface(surface));
   }
 }
 
 void BrowserMediaPlayerManager::DetachExternalVideoSurface(int player_id) {
   MediaPlayerAndroid* player = GetPlayer(player_id);
-  if (player)
+  if (player) {
+    // If a player looses its external surface because
+    // it enters fullscreen, the video should not be paused.
+    if (player_id != fullscreen_player_id_) {
+      player->Release();
+      Send(new MediaPlayerMsg_DidMediaPlayerPause(RoutingID(), player_id));
+    }
     player->SetVideoSurface(gfx::ScopedJavaSurface());
+  }
 }
 
 void BrowserMediaPlayerManager::OnFrameInfoUpdated() {
   if (external_video_surface_container_)
     external_video_surface_container_->OnFrameInfoUpdated();
 }
 
 void BrowserMediaPlayerManager::OnNotifyExternalSurface(
     int player_id, bool is_request, const gfx::RectF& rect) {
   if (!web_contents_)
@@ skipping 24 matching lines @@
                    base::Unretained(this)),
         base::Bind(&BrowserMediaPlayerManager::DetachExternalVideoSurface,
                    base::Unretained(this)));
   }
 }
 #endif  // defined(VIDEO_HOLE)
 
 void BrowserMediaPlayerManager::OnEnterFullscreen(int player_id) {
   DCHECK_EQ(fullscreen_player_id_, -1);
 #if defined(VIDEO_HOLE)
+  fullscreen_player_id_ = player_id;

[qinmin, 2014/11/21 00:23:50]

As I mentioned earlier, using fullscreen_player_id to do the reentrance trick is
really bad. If another developer look at this code, he will see that
fullscreen_player_id is set to -1 on line 410, and will remove this line. And
that will break your logic.

Since there can be at most one player_id associated with
external_video_surface_container, can't we just do sth like
external_video_surface_container_->ReleaseCurrentExternalVideoSurfacePlayer()?

+  // Avoid Z-fight: when any player goes
+  // fullscreen release any external surface.
   if (external_video_surface_container_)
-    external_video_surface_container_->ReleaseExternalVideoSurface(player_id);
+    external_video_surface_container_->ReleaseExternalVideoSurface(
+        ExternalVideoSurfaceContainer::kAnyPlayer);
+  fullscreen_player_id_ = -1;
 #endif  // defined(VIDEO_HOLE)
   if (video_view_.get()) {
     fullscreen_player_id_ = player_id;
     video_view_->OpenVideo();
     return;
   } else if (!ContentVideoView::GetInstance()) {
     // In Android WebView, two ContentViewCores could both try to enter
     // fullscreen video, we just ignore the second one.
     video_view_.reset(new ContentVideoView(this));
     base::android::ScopedJavaLocalRef<jobject> j_content_video_view =
@@ skipping 181 matching lines @@
     external_video_surface_container_->ReleaseExternalVideoSurface(player_id);
 #endif  // defined(VIDEO_HOLE)
 }
 
 void BrowserMediaPlayerManager::ReleasePlayer(MediaPlayerAndroid* player) {
   player->Release();
   ReleaseMediaResources(player->player_id());
 }
 
 }  // namespace content