WebView implements ContentVideoViewEmbedder.enterFullscreenVideo here:
https://code.google.com/p/chromium/codesearch#chromium/src/android_webview/java/src/org/chromium/android_webview/AwContentViewClient.java&l=77

Is spitzer using a different path in chrome now?

Actually this should work then because we're reusing the ContentVideoView for spitzer.

I'm unclear about one thing though: the ExternalViewSurfaceContainer overrides SurfaceView to not punch a hole (https://code.google.com/p/chromium/codesearch#chromium/src/components/external_video_surface/android/java/src/org/chromium/components/external_video_surface/ExternalVideoSurfaceContainer.java&sq=package:chromium&type=cs&l=46)

Why does it do that? And then why does ContentVideoView work? Because it does the normal SurfaceView hole punching.

ExternalViewSurfaceContainer is only used for the VIDEO_HOLE, ie hole punching. With hole punching, compositor will punch a hole itself where the video is, so there is no need for SurfaceView to do it:

https://code.google.com/p/chromium/codesearch#chromium/src/cc/layers/video_layer_impl.cc&l=349

Hole punching is only for inline-video, not full screen video I believe. So maybe bug should be retitled spitzer doesn't work with hole punching?

Honestly it's still unclear to me if anyone actually uses hole punching in webview, since can only turn it on in a hidden api. And the people who pushed for it and implemented it have kind of went away.. Hole punching is used by chromecast as well though.

Oh I see. AFAIK, that means ContentVideoView doesn't necessarily need to do the SurfaceView hole punching either (but I also don't know of any downside to leaving it as-is).

It's true that Spitzer doesn't do the embedded hole punching thing yet. It would be awesome if no one was using it and we could get rid of it though.

> Oh I see. AFAIK, that means ContentVideoView doesn't necessarily need to do the SurfaceView hole punching either (but I also don't know of any downside to leaving it as-is).

Not sure. Might not be needed for chrome, since chrome controls the whole view tree. I'm sure if there are weird corner cases with webview sandwiched in a weird way between other views to need the surface view to do the hole punching. Can't think of any at the moment.

Definitely test both chrome and webview though, since webview rendering doesn't match chrome exactly either.

> It's true that Spitzer doesn't do the embedded hole punching thing yet. It would be awesome if no one was using it and we could get rid of it though.

Webview will get limited UMA "soon". Maybe wait until data from that to see..?

There's also chromecast though

> There's also chromecast though

i have an AI* to follow up on chrome cast about hole punching + spitzer.

* this one does not play Go.

After building webview and implementing onShowCustomView (thanks boliu) I discovered (remembered) that we actually disabled ContentVideoView fullscreen for webview.. We're using the copying strategy.

It's possible to do SurfaceView fullscreen without getting zero copy working by choosing the backing strategy during AVDA::Initialize when we know whether a surface id was passed, or not.

Frank, if you're going to get zero copy working for webview I might just wait for that.

Currently these tests fail when spitzer is enabled:

org.chromium.android_webview.test.AwContentsClientFullScreenTest#testExitFullscreenEndsIfAppInvokesCallbackFromOnHideCustomView
org.chromium.android_webview.test.AwContentsClientFullScreenTest#testExitFullscreenEndsIfAppInvokesCallbackFromOnHideCustomView with {--webview-sandboxed-renderer --ipc-sync-compositing}
org.chromium.android_webview.test.AwContentsClientFullScreenTest#testOnShowAndHideCustomViewWithBackKey_video
org.chromium.android_webview.test.AwContentsClientFullScreenTest#testOnShowAndHideCustomViewWithBackKey_video with {--webview-sandboxed-renderer --ipc-sync-compositing}
org.chromium.android_webview.test.AwContentsClientFullScreenTest#testOnShowAndHideCustomViewWithCallback_video
org.chromium.android_webview.test.AwContentsClientFullScreenTest#testOnShowAndHideCustomViewWithCallback_video with {--webview-sandboxed-renderer --ipc-sync-compositing}
org.chromium.android_webview.test.AwContentsClientFullScreenTest#testOnShowAndHideCustomViewWithJavascript_video
org.chromium.android_webview.test.AwContentsClientFullScreenTest#testOnShowAndHideCustomViewWithJavascript_video with {--webview-sandboxed-renderer --ipc-sync-compositing}
org.chromium.android_webview.test.AwContentsClientFullScreenTest#testOnShowCustomViewAndPlayWithHtmlControl_video
org.chromium.android_webview.test.AwContentsClientFullScreenTest#testOnShowCustomViewAndPlayWithHtmlControl_video with {--webview-sandboxed-renderer --ipc-sync-compositing}
org.chromium.android_webview.test.AwContentsClientFullScreenTest#testOnShowCustomViewRemovesHolePunchingSurfaceForVideo
org.chromium.android_webview.test.AwContentsClientFullScreenTest#testOnShowCustomViewRemovesHolePunchingSurfaceForVideo with {--webview-sandboxed-renderer --ipc-sync-compositing}
org.chromium.android_webview.test.AwContentsClientFullScreenTest#testOnShowCustomViewTransfersHolePunchingSurfaceForVideoInsideDiv
org.chromium.android_webview.test.AwContentsClientFullScreenTest#testOnShowCustomViewTransfersHolePunchingSurfaceForVideoInsideDiv with {--webview-sandboxed-renderer --ipc-sync-compositing}
org.chromium.android_webview.test.AwContentsClientFullScreenTest#testPowerSaveBlockerIsEnabledDuringFullscreenPlayback_video
org.chromium.android_webview.test.AwContentsClientFullScreenTest#testPowerSaveBlockerIsEnabledDuringFullscreenPlayback_video with {--webview-sandboxed-renderer --ipc-sync-compositing}
org.chromium.android_webview.test.AwContentsClientGetVideoLoadingProgressViewTest#testGetVideoLoadingProgressView
org.chromium.android_webview.test.AwContentsClientGetVideoLoadingProgressViewTest#testGetVideoLoadingProgressView with {--webview-sandboxed-renderer --ipc-sync-compositing}
org.chromium.android_webview.test.ExternalVideoSurfaceContainerTest#testEnableVideoOverlayForEmbeddedVideo
org.chromium.android_webview.test.ExternalVideoSurfaceContainerTest#testEnableVideoOverlayForEmbeddedVideo with {--webview-sandboxed-renderer --ipc-sync-compositing}

These too, but this is more of a behavioral change unrelated to fullscreen I think:
org.chromium.android_webview.test.MultipleVideosTest#testFirstVideoPausesWhenSecondVideoStarts
org.chromium.android_webview.test.MultipleVideosTest#testFirstVideoPausesWhenSecondVideoStarts with {--webview-sandboxed-renderer --ipc-sync-compositing}

webview zero copy CL is here: https://codereview.chromium.org/1680213002/

but i think it still fails on one of the bots, and it hasn't been at the top of my list to fix since the copying strategy is working.

i'll rebase and kick off the bots again, and see what the state of it is.

testFirstVideoPausesWhenSecondVideoStarts is testing VIDEO_HOLE hole punching, globally supports only one video globally..

> globally supports only one video globally..

Barr copy-editing.. Should have read:

VIDEO_HOLE supports only one video globally..

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb

commit 7defc181e284d5e24a00a9254d4dc7ac3f2c15cb
Author: dalecurtis <dalecurtis@chromium.org>
Date: Fri Apr 01 00:46:57 2016

Flip unified media pipeline to default-on w/ disabled holdback.

Will be merged back to M50 pending launch-approval.

Experiment config will be updated to target M50 stable only at a 95%
disabled rate and slowly ramped down to 0-1% over a couple weeks
to ensure there are no major regressions.

Since there are still some WebView issues to sort out, WebView
has been disabled in this patch set.

Notably on M51 this will also enable 100% of MSE/EME playbacks
onto the unified pipeline. Since MSE only makes up ~9% of
Android playbacks (EME < 1%) and have always been codec restricted,
this is an acceptable risk.

BUG= 597495 , 598840,  598963 
TEST=passes bots.

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

Cr-Commit-Position: refs/heads/master@{#384451}

[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/android_webview/lib/main/aw_main_delegate.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/chrome/app/generated_resources.grd
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/chrome/browser/about_flags.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/content/browser/media/encrypted_media_browsertest.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/content/browser/media/media_canplaytype_browsertest.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/media/base/key_systems_unittest.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/media/base/media.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/media/base/media_switches.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/media/base/media_switches.h
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/media/base/mime_util_internal.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/media/base/mime_util_unittest.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/media/base/run_all_unittests.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/media/filters/chunk_demuxer_unittest.cc
[modify] https://crrev.com/7defc181e284d5e24a00a9254d4dc7ac3f2c15cb/tools/metrics/histograms/histograms.xml

tima: Are you able to take this on now? After tobiasjs' change I was able to verify that spitzer fullscreen is at least partially working. So now I don't know why the tests are failing.

I'm also not yet sure if we need to do anything different for software decoded video. Previously all fullscreen video used ContentVideoView, but now we prefer to use libvpx if the device doesn't have a HW decoder, so there's a new fullscreen mode where there won't be a ContentVideoView. In that case there's no view to give to the embedder in onShowCustomView, so I'm not sure how it will work.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bb95c90f63793ada603c740e3a48a2c5fbc01d2e

commit bb95c90f63793ada603c740e3a48a2c5fbc01d2e
Author: timav <timav@chromium.org>
Date: Fri Jun 24 21:04:38 2016

Enable deferred rendering strategy for Android WebView for Spitzer

In Android Webview (single process) AVDA deferred strategy can work
if we require texture copying during composition with frame's
COPY_REQUIRED flag. This CL sets AVDA deferred strategy for all
cases and sets COPY_REQUIRED for WebView.

This is prerequisite for https://codereview.chromium.org/2057743002

BUG= 582170 ,  597495 

Review-Url: https://codereview.chromium.org/2052103002
Cr-Commit-Position: refs/heads/master@{#401967}

[modify] https://crrev.com/bb95c90f63793ada603c740e3a48a2c5fbc01d2e/media/filters/gpu_video_decoder.cc
[modify] https://crrev.com/bb95c90f63793ada603c740e3a48a2c5fbc01d2e/media/filters/gpu_video_decoder.h
[modify] https://crrev.com/bb95c90f63793ada603c740e3a48a2c5fbc01d2e/media/gpu/android_video_decode_accelerator.cc
[modify] https://crrev.com/bb95c90f63793ada603c740e3a48a2c5fbc01d2e/media/gpu/android_video_decode_accelerator.h
[modify] https://crrev.com/bb95c90f63793ada603c740e3a48a2c5fbc01d2e/media/video/video_decode_accelerator.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a08264d208ffa47ebadbdde57c8226f1ecee5114

commit a08264d208ffa47ebadbdde57c8226f1ecee5114
Author: timav <timav@chromium.org>
Date: Wed Jun 29 22:20:44 2016

Fix AW tests for Spitzer

In constast to the MediaPlayerBridge/MediaSourcePlayer
based pipeline Spitzer never creates an underlay SurfaceView
(ContentVideoView) when it works for WebView.

This ContentVideoView was required for some tests. this CL removes
this requirements for all full screen tests and disables
hole punching tests since we consider them obsolete.

This is a prerequisite for enabling Spitzer in Android Webview
(https://codereview.chromium.org/2057743002).

BUG= 597495 

Review-Url: https://codereview.chromium.org/2029053003
Cr-Commit-Position: refs/heads/master@{#402950}

[modify] https://crrev.com/a08264d208ffa47ebadbdde57c8226f1ecee5114/android_webview/javatests/src/org/chromium/android_webview/test/AwContentsClientFullScreenTest.java
[modify] https://crrev.com/a08264d208ffa47ebadbdde57c8226f1ecee5114/android_webview/javatests/src/org/chromium/android_webview/test/ExternalVideoSurfaceContainerTest.java
[modify] https://crrev.com/a08264d208ffa47ebadbdde57c8226f1ecee5114/android_webview/javatests/src/org/chromium/android_webview/test/MultipleVideosTest.java

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e0e5c818737bb30a4526245db6ef32d9c6a8ef5e

commit e0e5c818737bb30a4526245db6ef32d9c6a8ef5e
Author: timav <timav@chromium.org>
Date: Thu Jun 30 00:33:07 2016

Enable Spitzer for Android Webview

The CL removes the command line flag that disabled it.

BUG= 597495 

Review-Url: https://codereview.chromium.org/2057743002
Cr-Commit-Position: refs/heads/master@{#403031}

[modify] https://crrev.com/e0e5c818737bb30a4526245db6ef32d9c6a8ef5e/android_webview/lib/main/aw_main_delegate.cc

With c#20 Spitzer is enabled for WebView.

The title is wrong though: currently we do not rely on ContentVideoView for WebView full screen video. In WebView the full screen rendering uses the same mechanism as embedded video, with SurfaceTexture and copying.

ContentVideoView is still created for both Chrome and WebView due to https://codereview.chromium.org/2083123003, but for the different purpose.