Commit 61288e6c authored by hugo.holgersson's avatar hugo.holgersson Committed by Commit bot

Tear down ContentVideoView from content-layer

Background: ContentVideoView displays fullscreen video in a
VideoViewSurface. Currently, each Android content embedder
(WebView, ContentShell, and ChromeShell) is responsible for
releasing this surface when exiting fullscreen.

Problem: Releasing ContentVideoView from each embedder leads to
code duplication. Worse, this was never done for the WebView:
when it exits fullscreen video playback, AwContents's
exitFullScreen removes the VideoViewSurface which triggers
ContentVideoView's surfaceDestroyed(). surfaceDestroyed()
releases the MediaPlayerAndroid. Releasing the MediaPlayerAndroid
during playback gives a frozen video (see bug).

Solution: For all content embedders on Android, tear down
ContentVideoView, from content-layer (before the embedder
specific "exit fullscreen"-work).

BUG=449152
TEST=For unencrypted video, manually verify that video playback
continues inline when exiting fullscreen (tested in
com.android.browser, ContentShell and ChromeShell). Added
automatic test for VIDEO_HOLE-playback in WebView.

Review URL: https://codereview.chromium.org/845193005

Cr-Commit-Position: refs/heads/master@{#313531}
parent 0b9b6ca3
...@@ -267,6 +267,11 @@ public class AwContentsClientFullScreenTest extends AwTestBase { ...@@ -267,6 +267,11 @@ public class AwContentsClientFullScreenTest extends AwTestBase {
// We need to wait because the surface view is first transfered, and then removed // We need to wait because the surface view is first transfered, and then removed
// asynchronously. // asynchronously.
VideoSurfaceViewUtils.waitAndAssertContainsZeroVideoHoleSurfaceViews(this, customView); VideoSurfaceViewUtils.waitAndAssertContainsZeroVideoHoleSurfaceViews(this, customView);
// Exit fullscreen and verify that the video hole surface is re-created.
DOMUtils.exitFullscreen(mContentViewCore.getWebContents());
VideoSurfaceViewUtils.pollAndAssertContainsOneVideoHoleSurfaceView(this,
mTestContainerView);
} }
@MediumTest @MediumTest
...@@ -478,6 +483,7 @@ public class AwContentsClientFullScreenTest extends AwTestBase { ...@@ -478,6 +483,7 @@ public class AwContentsClientFullScreenTest extends AwTestBase {
} }
} }
})); }));
// TODO: Test that inline video is actually displayed.
} }
private void assertContainsContentVideoView() throws Exception { private void assertContainsContentVideoView() throws Exception {
......
...@@ -10,7 +10,6 @@ import android.view.Window; ...@@ -10,7 +10,6 @@ import android.view.Window;
import org.chromium.chrome.browser.Tab; import org.chromium.chrome.browser.Tab;
import org.chromium.chrome.browser.fullscreen.FullscreenHtmlApiHandler.FullscreenHtmlApiDelegate; import org.chromium.chrome.browser.fullscreen.FullscreenHtmlApiHandler.FullscreenHtmlApiDelegate;
import org.chromium.chrome.browser.tabmodel.TabModelSelector; import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.content.browser.ContentVideoView;
/** /**
* Manages the basic fullscreen functionality required by a Tab. * Manages the basic fullscreen functionality required by a Tab.
...@@ -142,10 +141,6 @@ public abstract class FullscreenManager { ...@@ -142,10 +141,6 @@ public abstract class FullscreenManager {
mHtmlApiHandler.setPersistentFullscreenMode(enabled); mHtmlApiHandler.setPersistentFullscreenMode(enabled);
Tab tab = mModelSelector.getCurrentTab(); Tab tab = mModelSelector.getCurrentTab();
if (!enabled && mOverlayVideoMode) {
ContentVideoView videoView = ContentVideoView.getContentVideoView();
if (videoView != null) videoView.exitFullscreen(false);
}
if (tab != null) { if (tab != null) {
tab.updateFullscreenEnabledState(); tab.updateFullscreenEnabledState();
} }
......
...@@ -12,7 +12,6 @@ import org.chromium.chrome.browser.UrlUtilities; ...@@ -12,7 +12,6 @@ import org.chromium.chrome.browser.UrlUtilities;
import org.chromium.chrome.browser.contextmenu.ChromeContextMenuPopulator; import org.chromium.chrome.browser.contextmenu.ChromeContextMenuPopulator;
import org.chromium.chrome.browser.contextmenu.ContextMenuPopulator; import org.chromium.chrome.browser.contextmenu.ContextMenuPopulator;
import org.chromium.chrome.browser.tabmodel.TabModel.TabLaunchType; import org.chromium.chrome.browser.tabmodel.TabModel.TabLaunchType;
import org.chromium.content.browser.ContentVideoView;
import org.chromium.content.browser.ContentViewClient; import org.chromium.content.browser.ContentViewClient;
import org.chromium.content_public.browser.LoadUrlParams; import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.content_public.browser.NavigationController; import org.chromium.content_public.browser.NavigationController;
...@@ -127,12 +126,6 @@ public class ChromeShellTab extends Tab { ...@@ -127,12 +126,6 @@ public class ChromeShellTab extends Tab {
public void toggleFullscreenModeForTab(boolean enterFullscreen) { public void toggleFullscreenModeForTab(boolean enterFullscreen) {
mIsFullscreen = enterFullscreen; mIsFullscreen = enterFullscreen;
super.toggleFullscreenModeForTab(enterFullscreen); super.toggleFullscreenModeForTab(enterFullscreen);
if (!mIsFullscreen) {
ContentVideoView videoView = ContentVideoView.getContentVideoView();
if (videoView != null) videoView.exitFullscreen(false);
}
} }
@Override @Override
......
...@@ -21,7 +21,6 @@ ...@@ -21,7 +21,6 @@
#include "content/public/browser/render_view_host.h" #include "content/public/browser/render_view_host.h"
#include "content/public/browser/storage_partition.h" #include "content/public/browser/storage_partition.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "content/public/browser/web_contents_delegate.h"
#include "content/public/common/content_client.h" #include "content/public/common/content_client.h"
#include "content/public/common/content_switches.h" #include "content/public/common/content_switches.h"
#include "media/base/android/media_player_bridge.h" #include "media/base/android/media_player_bridge.h"
...@@ -137,8 +136,6 @@ BrowserMediaPlayerManager::~BrowserMediaPlayerManager() { ...@@ -137,8 +136,6 @@ BrowserMediaPlayerManager::~BrowserMediaPlayerManager() {
} }
void BrowserMediaPlayerManager::ExitFullscreen(bool release_media_player) { void BrowserMediaPlayerManager::ExitFullscreen(bool release_media_player) {
if (WebContentsDelegate* delegate = web_contents_->GetDelegate())
delegate->ExitFullscreenModeForTab(web_contents_);
if (RenderWidgetHostViewAndroid* view_android = if (RenderWidgetHostViewAndroid* view_android =
static_cast<RenderWidgetHostViewAndroid*>( static_cast<RenderWidgetHostViewAndroid*>(
web_contents_->GetRenderWidgetHostView())) { web_contents_->GetRenderWidgetHostView())) {
......
...@@ -108,6 +108,7 @@ ...@@ -108,6 +108,7 @@
#endif #endif
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
#include "content/browser/android/content_video_view.h"
#include "content/browser/android/date_time_chooser_android.h" #include "content/browser/android/date_time_chooser_android.h"
#include "content/browser/media/android/browser_media_player_manager.h" #include "content/browser/media/android/browser_media_player_manager.h"
#include "content/browser/web_contents/web_contents_android.h" #include "content/browser/web_contents/web_contents_android.h"
...@@ -1485,6 +1486,12 @@ void WebContentsImpl::ExitFullscreenMode() { ...@@ -1485,6 +1486,12 @@ void WebContentsImpl::ExitFullscreenMode() {
if (widget_view) if (widget_view)
RenderWidgetHostImpl::From(widget_view->GetRenderWidgetHost())->Shutdown(); RenderWidgetHostImpl::From(widget_view->GetRenderWidgetHost())->Shutdown();
#if defined(OS_ANDROID)
ContentVideoView* video_view = ContentVideoView::GetInstance();
if (video_view != NULL)
video_view->OnExitFullscreen();
#endif
if (delegate_) if (delegate_)
delegate_->ExitFullscreenModeForTab(this); delegate_->ExitFullscreenModeForTab(this);
......
...@@ -378,8 +378,8 @@ public class ContentVideoView extends FrameLayout ...@@ -378,8 +378,8 @@ public class ContentVideoView extends FrameLayout
} }
public void exitFullscreen(boolean relaseMediaPlayer) { public void exitFullscreen(boolean relaseMediaPlayer) {
destroyContentVideoView(false);
if (mNativeContentVideoView != 0) { if (mNativeContentVideoView != 0) {
destroyContentVideoView(false);
if (mUmaRecorded && !mPossibleAccidentalChange) { if (mUmaRecorded && !mPossibleAccidentalChange) {
long currentTime = System.currentTimeMillis(); long currentTime = System.currentTimeMillis();
long timeBeforeOrientationChange = mOrientationChangedTime - mPlaybackStartTime; long timeBeforeOrientationChange = mOrientationChangedTime - mPlaybackStartTime;
......
...@@ -22,7 +22,6 @@ import android.widget.TextView.OnEditorActionListener; ...@@ -22,7 +22,6 @@ import android.widget.TextView.OnEditorActionListener;
import org.chromium.base.CalledByNative; import org.chromium.base.CalledByNative;
import org.chromium.base.JNINamespace; import org.chromium.base.JNINamespace;
import org.chromium.content.browser.ContentVideoView;
import org.chromium.content.browser.ContentView; import org.chromium.content.browser.ContentView;
import org.chromium.content.browser.ContentViewClient; import org.chromium.content.browser.ContentViewClient;
import org.chromium.content.browser.ContentViewCore; import org.chromium.content.browser.ContentViewCore;
...@@ -260,11 +259,6 @@ public class Shell extends LinearLayout { ...@@ -260,11 +259,6 @@ public class Shell extends LinearLayout {
mIsFullscreen = enterFullscreen; mIsFullscreen = enterFullscreen;
LinearLayout toolBar = (LinearLayout) findViewById(R.id.toolbar); LinearLayout toolBar = (LinearLayout) findViewById(R.id.toolbar);
toolBar.setVisibility(enterFullscreen ? GONE : VISIBLE); toolBar.setVisibility(enterFullscreen ? GONE : VISIBLE);
if (!mIsFullscreen) {
ContentVideoView videoView = ContentVideoView.getContentVideoView();
if (videoView != null) videoView.exitFullscreen(false);
}
} }
@CalledByNative @CalledByNative
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment