Refactoring ContentVideoViewContextDelegate.java

content/browser/android/media_player_manager_android.cc

Index: content/browser/android/media_player_manager_android.cc
diff --git a/content/browser/android/media_player_manager_android.cc b/content/browser/android/media_player_manager_android.cc
index ef650286e98e604102be1ff23d8b357a45741ef2..8e4dc1ca499f0d11d0c7ecdd75fbfe858c8d6bc1 100644
--- a/content/browser/android/media_player_manager_android.cc
+++ b/content/browser/android/media_player_manager_android.cc
@@ -5,6 +5,7 @@
 #include "content/browser/android/media_player_manager_android.h"
 
 #include "base/bind.h"
+#include "content/browser/android/content_view_core_impl.h"
 #include "content/browser/android/media_resource_getter_impl.h"
 #include "content/browser/web_contents/web_contents_view_android.h"
 #include "content/common/media/media_player_messages.h"
@@ -12,6 +13,7 @@
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/storage_partition.h"
+#include "content/public/browser/web_contents.h"
 
 using media::MediaPlayerBridge;
 
@@ -24,7 +26,6 @@ namespace content {
 MediaPlayerManagerAndroid::MediaPlayerManagerAndroid(
     RenderViewHost* render_view_host)
     : RenderViewHostObserver(render_view_host),
-      ALLOW_THIS_IN_INITIALIZER_LIST(video_view_(this)),
       fullscreen_player_id_(-1),
       web_contents_(WebContents::FromRenderViewHost(render_view_host)) {
 }
@@ -87,6 +88,7 @@ void MediaPlayerManagerAndroid::ExitFullscreen(bool release_media_player) {
     player->Release();
   else
     player->SetVideoSurface(NULL);
+  video_view_.reset();
 }
 
 void MediaPlayerManagerAndroid::SetVideoSurface(jobject surface) {
@@ -155,9 +157,17 @@ void MediaPlayerManagerAndroid::OnPause(int player_id) {
 
 void MediaPlayerManagerAndroid::OnEnterFullscreen(int player_id) {
   DCHECK_EQ(fullscreen_player_id_, -1);
-
-  fullscreen_player_id_ = player_id;
-  video_view_.CreateContentVideoView();
+  if (video_view_.get())
+    video_view_->OpenVideo();
+  else {
+    fullscreen_player_id_ = player_id;
+    WebContents* web_contents =
+        WebContents::FromRenderViewHost(render_view_host());
+    ContentViewCoreImpl* content_view_core_impl =
+        ContentViewCoreImpl::FromWebContents(web_contents);
+    video_view_.reset(new ContentVideoView(content_view_core_impl->GetContext(),

[joth, 2013/05/15 22:38:57]

I'm not sure what stops two competing ContentViewCores from both getting here
when trying to enter fullscreen, and the second will then hit the singleton
DCHECK inside the ContentVideoView constructor?

Would it be better to have a ContentVideoView::Create() method, and inside there
we can detect there's already a fullscreen view and tear it down? (THis would
avoid the need for the code you have in AwContentClient to exit fullscreen)

[michaelbai, 2013/05/22 18:08:39]

hmm, that's no a common use case, it needs user's action to toggle to fullscreen
video, I just made the second one fail.


On 2013/05/15 22:38:57, joth wrote:

+        content_view_core_impl->GetContentVideoViewClient(), this));
+  }
 }
 
 void MediaPlayerManagerAndroid::OnExitFullscreen(int player_id) {
@@ -165,8 +175,9 @@ void MediaPlayerManagerAndroid::OnExitFullscreen(int player_id) {
     MediaPlayerBridge* player = GetPlayer(player_id);
     if (player)
       player->SetVideoSurface(NULL);
-    video_view_.DestroyContentVideoView();
     fullscreen_player_id_ = -1;
+    video_view_->OnExitFullscreen();
+    video_view_.reset();
   }
 }
 
@@ -194,7 +205,7 @@ void MediaPlayerManagerAndroid::OnDestroyPlayer(int player_id) {
 void MediaPlayerManagerAndroid::DestroyAllMediaPlayers() {
   players_.clear();
   if (fullscreen_player_id_ != -1) {
-    video_view_.DestroyContentVideoView();
+    video_view_.reset();
     fullscreen_player_id_ = -1;
   }
 }
@@ -240,13 +251,13 @@ void MediaPlayerManagerAndroid::OnMediaMetadataChanged(
   Send(new MediaPlayerMsg_MediaMetadataChanged(
       routing_id(), player_id, duration, width, height, success));
   if (fullscreen_player_id_ != -1)
-    video_view_.UpdateMediaMetadata();
+    video_view_->UpdateMediaMetadata();
 }
 
 void MediaPlayerManagerAndroid::OnPlaybackComplete(int player_id) {
   Send(new MediaPlayerMsg_MediaPlaybackCompleted(routing_id(), player_id));
   if (fullscreen_player_id_ != -1)
-    video_view_.OnPlaybackComplete();
+    video_view_->OnPlaybackComplete();
 }
 
 void MediaPlayerManagerAndroid::OnMediaInterrupted(int player_id) {
@@ -260,7 +271,7 @@ void MediaPlayerManagerAndroid::OnBufferingUpdate(
   Send(new MediaPlayerMsg_MediaBufferingUpdate(
       routing_id(), player_id, percentage));
   if (fullscreen_player_id_ != -1)
-    video_view_.OnBufferingUpdate(percentage);
+    video_view_->OnBufferingUpdate(percentage);
 }
 
 void MediaPlayerManagerAndroid::OnSeekComplete(int player_id,
@@ -272,7 +283,7 @@ void MediaPlayerManagerAndroid::OnSeekComplete(int player_id,
 void MediaPlayerManagerAndroid::OnError(int player_id, int error) {
   Send(new MediaPlayerMsg_MediaError(routing_id(), player_id, error));
   if (fullscreen_player_id_ != -1)
-    video_view_.OnMediaPlayerError(error);
+    video_view_->OnMediaPlayerError(error);
 }
 
 void MediaPlayerManagerAndroid::OnVideoSizeChanged(
@@ -280,7 +291,7 @@ void MediaPlayerManagerAndroid::OnVideoSizeChanged(
   Send(new MediaPlayerMsg_MediaVideoSizeChanged(routing_id(), player_id,
       width, height));
   if (fullscreen_player_id_ != -1)
-    video_view_.OnVideoSizeChanged(width, height);
+    video_view_->OnVideoSizeChanged(width, height);
 }
 
 void MediaPlayerManagerAndroid::OnTimeUpdate(int player_id,