Skip to content

Commit

Permalink
Remove video overlay support from WebView
Browse files Browse the repository at this point in the history
Video overlay for embedded videos (aka hole punching)
was diabled in https://codereview.chromium.org/2026423002/.
This CL removes it completely.

BUG=616583

Review-Url: https://codereview.chromium.org/2299883007
Cr-Commit-Position: refs/heads/master@{#417025}
  • Loading branch information
timav authored and Commit bot committed Sep 7, 2016
1 parent 6dcf5d5 commit 78b35e8
Show file tree
Hide file tree
Showing 21 changed files with 6 additions and 545 deletions.
1 change: 0 additions & 1 deletion android_webview/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -713,7 +713,6 @@ android_library("android_webview_java") {
":resources",
":strings_grd",
"//base:base_java",
"//components/external_video_surface:java",
"//components/navigation_interception/android:navigation_interception_java",
"//components/web_contents_delegate_android:web_contents_delegate_android_java",
"//components/web_restrictions:web_restrictions_java",
Expand Down
8 changes: 0 additions & 8 deletions android_webview/browser/aw_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -521,12 +521,4 @@ AwContentBrowserClient::CreateThrottlesForNavigation(
return throttles;
}

#if defined(VIDEO_HOLE)
content::ExternalVideoSurfaceContainer*
AwContentBrowserClient::OverrideCreateExternalVideoSurfaceContainer(
content::WebContents* web_contents) {
return native_factory_->CreateExternalVideoSurfaceContainer(web_contents);
}
#endif

} // namespace android_webview
5 changes: 0 additions & 5 deletions android_webview/browser/aw_content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,6 @@ class AwContentBrowserClient : public content::ContentBrowserClient {
content::WebPreferences* web_prefs) override;
ScopedVector<content::NavigationThrottle> CreateThrottlesForNavigation(
content::NavigationHandle* navigation_handle) override;
#if defined(VIDEO_HOLE)
content::ExternalVideoSurfaceContainer*
OverrideCreateExternalVideoSurfaceContainer(
content::WebContents* web_contents) override;
#endif

private:
// Android WebView currently has a single global (non-off-the-record) browser
Expand Down
5 changes: 0 additions & 5 deletions android_webview/browser/jni_dependency_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include "base/memory/ref_counted.h"

namespace content {
class ExternalVideoSurfaceContainer;
class WebContents;
class WebContentsViewDelegate;
} // namespace content
Expand All @@ -33,10 +32,6 @@ class JniDependencyFactory {
virtual AwWebPreferencesPopulater* CreateWebPreferencesPopulater() = 0;
virtual AwMessagePortService* CreateAwMessagePortService() = 0;
virtual AwLocaleManager* CreateAwLocaleManager() = 0;
#if defined(VIDEO_HOLE)
virtual content::ExternalVideoSurfaceContainer*
CreateExternalVideoSurfaceContainer(content::WebContents* contents) = 0;
#endif
};

} // namespace android_webview
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -553,11 +553,12 @@ public int getDisabledActionModeMenuItems() {

@Override
public void setVideoOverlayForEmbeddedEncryptedVideoEnabled(boolean flag) {
mAwSettings.setVideoOverlayForEmbeddedVideoEnabled(flag);
// No-op, see http://crbug.com/616583
}

@Override
public boolean getVideoOverlayForEmbeddedEncryptedVideoEnabled() {
return mAwSettings.getVideoOverlayForEmbeddedVideoEnabled();
// Always false, see http://crbug.com/616583
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ public class AwSettings {
private boolean mEnableSupportedHardwareAcceleratedFeatures = false;
private int mMixedContentMode = WebSettings.MIXED_CONTENT_NEVER_ALLOW;

private boolean mForceVideoOverlayForTests = false;
private boolean mOffscreenPreRaster = false;
private int mDisabledMenuItems = MENU_ITEM_NONE;

Expand Down Expand Up @@ -1696,46 +1695,6 @@ public void setDisabledActionModeMenuItems(int menuItems) {
}
}

/**
* Sets whether to use the video overlay for the embedded video.
* @param flag whether to enable the video overlay for the embedded video.
*/
public void setVideoOverlayForEmbeddedVideoEnabled(final boolean enabled) {
// No-op, see http://crbug.com/616583
}

/**
* Gets whether to use the video overlay for the embedded video.
* @return true if the WebView enables the video overlay for the embedded video.
*/
public boolean getVideoOverlayForEmbeddedVideoEnabled() {
// Always false, see http://crbug.com/616583
return false;
}

@CalledByNative
private boolean getVideoOverlayForEmbeddedVideoEnabledLocked() {
// Always false, see http://crbug.com/616583
return false;
}

@VisibleForTesting
public void setForceVideoOverlayForTests(final boolean enabled) {
synchronized (mAwSettingsLock) {
if (mForceVideoOverlayForTests != enabled) {
mForceVideoOverlayForTests = enabled;
mEventHandler.runOnUiThreadBlockingAndLocked(new Runnable() {
@Override
public void run() {
if (mNativeAwSettings != 0) {
nativeUpdateRendererPreferencesLocked(mNativeAwSettings);
}
}
});
}
}
}

@VisibleForTesting
public void updateAcceptLanguages() {
synchronized (mAwSettingsLock) {
Expand All @@ -1750,12 +1709,6 @@ public void run() {
}
}

@CalledByNative
private boolean getForceVideoOverlayForTests() {
assert Thread.holdsLock(mAwSettingsLock);
return mForceVideoOverlayForTests;
}

@CalledByNative
private boolean supportsDoubleTapZoomLocked() {
assert Thread.holdsLock(mAwSettingsLock);
Expand Down
1 change: 0 additions & 1 deletion android_webview/javatests/DEPS
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
include_rules = [
"+components/external_video_surface/android/java",
"+components/policy/android/java",
"+components/policy/android/javatests",
"+content/public/android/java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,101 +218,6 @@ public void doTestOnShowCustomViewAndPlayWithHtmlControl(String videoTestUrl) th
DOMUtils.waitForMediaPlay(getWebContentsOnUiThread(), VIDEO_ID);
}

/*
@MediumTest
@Feature({"AndroidWebView"})
*/
@DisabledTest(message = "crbug.com/597495")
public void testHolePunchingSurfaceNotCreatedForClearVideo() throws Throwable {
loadTestPage(VIDEO_TEST_URL);
assertFalse(DOMUtils.isFullscreen(getWebContentsOnUiThread()));

// Play and verify that a surface view for hole punching is not created.
// Note that VIDEO_TEST_URL contains clear video.
tapPlayButton();
DOMUtils.waitForMediaPlay(getWebContentsOnUiThread(), VIDEO_ID);
// Wait to ensure that the surface view is not added asynchronously.
VideoSurfaceViewUtils.waitAndAssertContainsZeroVideoHoleSurfaceViews(this,
mTestContainerView);
}

/*
@MediumTest
@Feature({"AndroidWebView"})
*/
@DisabledTest(message = "crbug.com/597495")
public void testOnShowCustomViewTransfersHolePunchingSurfaceForVideoInsideDiv()
throws Throwable {
getInstrumentation().runOnMainSync(new Runnable() {
@Override
public void run() {
mTestContainerView.getAwContents().getSettings().setForceVideoOverlayForTests(true);
}
});

loadTestPage(VIDEO_INSIDE_DIV_TEST_URL);
assertFalse(DOMUtils.isFullscreen(getWebContentsOnUiThread()));

// Play and verify that there is a surface view for hole punching.
tapPlayButton();
DOMUtils.waitForMediaPlay(getWebContentsOnUiThread(), VIDEO_ID);
VideoSurfaceViewUtils.pollAndAssertContainsOneVideoHoleSurfaceView(this,
mTestContainerView);

// Enter fullscreen and verify that the hole punching surface is transferred. Note
// that VIDEO_INSIDE_DIV_TEST_URL goes fullscreen on a <div> element, so in fullscreen
// the video will still be embedded in the page and the hole punching surface required.
// We need to transfer the external surface so that scrolling is synchronized with the
// new container view.
DOMUtils.clickNode(this, mContentViewCore, CUSTOM_FULLSCREEN_CONTROL_ID);
mContentsClient.waitForCustomViewShown();
View customView = mContentsClient.getCustomView();
VideoSurfaceViewUtils.assertContainsZeroVideoHoleSurfaceViews(this, mTestContainerView);
// Wait to ensure that the surface view stays there after being transfered and not
// removed asynchronously.
VideoSurfaceViewUtils.waitAndAssertContainsOneVideoHoleSurfaceView(this, customView);
}

/*
@MediumTest
@Feature({"AndroidWebView"})
*/
@DisabledTest(message = "crbug.com/597495")
public void testOnShowCustomViewRemovesHolePunchingSurfaceForVideo() throws Throwable {
getInstrumentation().runOnMainSync(new Runnable() {
@Override
public void run() {
mTestContainerView.getAwContents().getSettings().setForceVideoOverlayForTests(true);
}
});

loadTestPage(VIDEO_TEST_URL);
assertFalse(DOMUtils.isFullscreen(getWebContentsOnUiThread()));

// Play and verify that there is a surface view for hole punching.
tapPlayButton();
DOMUtils.waitForMediaPlay(getWebContentsOnUiThread(), VIDEO_ID);
VideoSurfaceViewUtils.pollAndAssertContainsOneVideoHoleSurfaceView(this,
mTestContainerView);

// Enter fullscreen and verify that the surface view is removed. Note that
// VIDEO_TEST_URL goes fullscreen on the <video> element, so in fullscreen
// the video will not be embedded in the page and the external surface
// not longer required.
DOMUtils.clickNode(this, mContentViewCore, CUSTOM_FULLSCREEN_CONTROL_ID);
mContentsClient.waitForCustomViewShown();
View customView = mContentsClient.getCustomView();
VideoSurfaceViewUtils.assertContainsZeroVideoHoleSurfaceViews(this, mTestContainerView);
// We need to wait because the surface view is first transfered, and then removed
// asynchronously.
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
@Feature({"AndroidWebView"})
public void testFullscreenNotSupported_video() throws Throwable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2730,15 +2730,15 @@ public void testSetInitialScale() throws Throwable {
@LargeTest
@Feature({"AndroidWebView", "Preferences"})
public void testMediaPlaybackWithoutUserGesture() throws Throwable {
assertTrue(VideoTestUtil.runVideoTest(this, false, false, WAIT_TIMEOUT_MS));
assertTrue(VideoTestUtil.runVideoTest(this, false, WAIT_TIMEOUT_MS));
}

@DisableHardwareAccelerationForTest
@SmallTest
@Feature({"AndroidWebView", "Preferences"})
public void testMediaPlaybackWithUserGesture() throws Throwable {
// Wait for 5 second to see if video played.
assertFalse(VideoTestUtil.runVideoTest(this, true, false, scaleTimeout(5000)));
assertFalse(VideoTestUtil.runVideoTest(this, true, scaleTimeout(5000)));
}

@SmallTest
Expand Down

This file was deleted.

Loading

0 comments on commit 78b35e8

Please sign in to comment.