Remove video overlay support from WebView

Remove video overlay support from WebView

Video overlay for embedded videos (aka hole punching)
was diabled in https://codereview.chromium.org/2026423002/.
This CL removes it completely.

BUG=616583
Committed: https://crrev.com/78b35e8a8b01f2db768a30033dee03323f71f316
Cr-Commit-Position: refs/heads/master@{#417025}

[Tima Vaisburd, 2016-09-02 22:14:32]

The CQ bit was checked by timav@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-02 22:15:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Tima Vaisburd, 2016-09-02 22:18:44]

timav@chromium.org changed reviewers:
+ boliu@chromium.org

[Tima Vaisburd, 2016-09-02 22:18:45]

PTAL.

https://codereview.chromium.org/2299883007/diff/1/android_webview/java/src/or...
File android_webview/java/src/org/chromium/android_webview/AwSettings.java
(right):

https://codereview.chromium.org/2299883007/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/AwSettings.java:1702:
public void setVideoOverlayForEmbeddedVideoEnabled(final boolean enabled) {
Is it possible to remove this too together with the corresponding stuff in glue/
?

[boliu, 2016-09-02 23:18:56]

remove all mention of components/external_video_surface from android_webview
https://cs.chromium.org/search/?q=components/external_video_surface+file:src/...

https://codereview.chromium.org/2299883007/diff/1/android_webview/java/src/or...
File android_webview/java/src/org/chromium/android_webview/AwSettings.java
(right):

https://codereview.chromium.org/2299883007/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/AwSettings.java:1702:
public void setVideoOverlayForEmbeddedVideoEnabled(final boolean enabled) {
On 2016/09/02 22:18:45, Tima Vaisburd wrote:
> Is it possible to remove this too together with the corresponding stuff in
glue/
> ?

You can remove this one by making glue no-op, but you can't remove the glue one.

https://codereview.chromium.org/2299883007/diff/1/android_webview/javatests/s...
File
android_webview/javatests/src/org/chromium/android_webview/test/MultipleVideosTest.java
(right):

https://codereview.chromium.org/2299883007/diff/1/android_webview/javatests/s...
android_webview/javatests/src/org/chromium/android_webview/test/MultipleVideosTest.java:16:
public class MultipleVideosTest extends AwTestBase {
delete this whole file, there was only one test in it

[commit-bot: I haz the power, 2016-09-02 23:32:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-02 23:32:39]

Dry run: This issue passed the CQ dry run.

[Tima Vaisburd, 2016-09-03 00:10:52]

The CQ bit was checked by timav@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-03 00:11:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-03 00:19:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-03 00:19:40]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Tima Vaisburd, 2016-09-03 01:23:17]

Patchset #2 (id:20001) has been deleted

[Tima Vaisburd, 2016-09-03 01:32:37]

Patchset #2 (id:40001) has been deleted

[Tima Vaisburd, 2016-09-03 02:02:58]

> remove all mention of components/external_video_surface from android_webview

Done.

https://codereview.chromium.org/2299883007/diff/1/android_webview/java/src/or...
File android_webview/java/src/org/chromium/android_webview/AwSettings.java
(right):

https://codereview.chromium.org/2299883007/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/AwSettings.java:1702:
public void setVideoOverlayForEmbeddedVideoEnabled(final boolean enabled) {
On 2016/09/02 23:18:56, boliu_OOO wrote:
> On 2016/09/02 22:18:45, Tima Vaisburd wrote:
> > Is it possible to remove this too together with the corresponding stuff in
> glue/
> > ?
> 
> You can remove this one by making glue no-op, but you can't remove the glue
one.

Not done, I think I need to make the glue no-op on clank/ first.

https://codereview.chromium.org/2299883007/diff/1/android_webview/javatests/s...
File
android_webview/javatests/src/org/chromium/android_webview/test/MultipleVideosTest.java
(right):

https://codereview.chromium.org/2299883007/diff/1/android_webview/javatests/s...
android_webview/javatests/src/org/chromium/android_webview/test/MultipleVideosTest.java:16:
public class MultipleVideosTest extends AwTestBase {
On 2016/09/02 23:18:56, boliu_OOO wrote:
> delete this whole file, there was only one test in it

Oops. Done.

[Tima Vaisburd, 2016-09-03 02:03:37]

The CQ bit was checked by timav@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-03 02:03:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[boliu, 2016-09-03 02:26:30]

On Sep 2, 2016 19:02, <timav@chromium.org> wrote:
>
> > remove all mention of components/external_video_surface from
android_webview
>
> Done.
>
>
>
>
https://codereview.chromium.org/2299883007/diff/1/android_webview/java/src/or...
> File
> android_webview/java/src/org/chromium/android_webview/AwSettings.java
> (right):
>
>
https://codereview.chromium.org/2299883007/diff/1/android_webview/java/src/or...
>
android_webview/java/src/org/chromium/android_webview/AwSettings.java:1702:
> public void setVideoOverlayForEmbeddedVideoEnabled(final boolean
> enabled) {
> On 2016/09/02 23:18:56, boliu_OOO wrote:
> > On 2016/09/02 22:18:45, Tima Vaisburd wrote:
> > > Is it possible to remove this too together with the corresponding
> stuff in
> > glue/
> > > ?
> >
> > You can remove this one by making glue no-op, but you can't remove the
> glue one.
>
> Not done, I think I need to make the glue no-op on clank/ first.

No you do not. We land glue changes upstream then merge downstream. But if
you make breaking changes like this, you should coordinate with current
Android sheriff before landing.

>
>
>
https://codereview.chromium.org/2299883007/diff/1/android_webview/javatests/s...
> File
>
android_webview/javatests/src/org/chromium/android_webview/test/MultipleVideosTest.java
> (right):
>
>
https://codereview.chromium.org/2299883007/diff/1/android_webview/javatests/s...
>
android_webview/javatests/src/org/chromium/android_webview/test/MultipleVideosTest.java:16:
> public class MultipleVideosTest extends AwTestBase {
> On 2016/09/02 23:18:56, boliu_OOO wrote:
> > delete this whole file, there was only one test in it
>
> Oops. Done.
>
> https://codereview.chromium.org/2299883007/

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Tima Vaisburd, 2016-09-03 02:39:45]

On 2016/09/03 02:26:30, boliu_OOO wrote:
> No you do not. We land glue changes upstream then merge downstream. But if
> you make breaking changes like this, you should coordinate with current
> Android sheriff before landing.

That's the usual way, but in this particular case if we land downstream first we
can avoid the breakage altogether, I think.
I do not have an access yet so I cannot create the corresponding downstream CL,
this one might unblock Jinsuk if everything
else is correct, of course.

[boliu, 2016-09-03 03:14:50]

FYI I'm about to delete the downstream glue.

On Sep 2, 2016 19:39, <timav@chromium.org> wrote:

> On 2016/09/03 02:26:30, boliu_OOO wrote:
> > No you do not. We land glue changes upstream then merge downstream. But
> if
> > you make breaking changes like this, you should coordinate with current
> > Android sheriff before landing.
>
> That's the usual way, but in this particular case if we land downstream
> first we
> can avoid the breakage altogether, I think.
> I do not have an access yet so I cannot create the corresponding
> downstream CL,
> this one might unblock Jinsuk if everything
> else is correct, of course.
>
> https://codereview.chromium.org/2299883007/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2016-09-03 03:23:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-03 03:23:10]

Dry run: This issue passed the CQ dry run.

[Tima Vaisburd, 2016-09-06 15:44:46]

The CQ bit was checked by timav@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-06 15:45:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Tima Vaisburd, 2016-09-06 15:48:57]

https://codereview.chromium.org/2299883007/diff/1/android_webview/java/src/or...
File android_webview/java/src/org/chromium/android_webview/AwSettings.java
(right):

https://codereview.chromium.org/2299883007/diff/1/android_webview/java/src/or...
android_webview/java/src/org/chromium/android_webview/AwSettings.java:1702:
public void setVideoOverlayForEmbeddedVideoEnabled(final boolean enabled) {
On 2016/09/03 02:02:58, Tima Vaisburd wrote:
> On 2016/09/02 23:18:56, boliu_OOO wrote:
> > On 2016/09/02 22:18:45, Tima Vaisburd wrote:
> > > Is it possible to remove this too together with the corresponding stuff in
> > glue/
> > > ?
> > 
> > You can remove this one by making glue no-op, but you can't remove the glue
> one.
> 
> Not done, I think I need to make the glue no-op on clank/ first.

Done in PS4

[commit-bot: I haz the power, 2016-09-06 16:53:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-06 16:53:11]

Dry run: This issue passed the CQ dry run.

[Tima Vaisburd, 2016-09-06 21:20:10]

Description was changed from

==========
Remove video overlay support WebView

Video overlay for embedded videos (aka hole punching)
was diabled in https://codereview.chromium.org/2026423002/.
This CL removes it completely.

BUG=616583
==========

to

==========
Remove video overlay support from WebView

Video overlay for embedded videos (aka hole punching)
was diabled in https://codereview.chromium.org/2026423002/.
This CL removes it completely.

BUG=616583
==========

[boliu, 2016-09-07 16:52:32]

https://codereview.chromium.org/2299883007/diff/100001/android_webview/test/B...
File android_webview/test/BUILD.gn (right):

https://codereview.chromium.org/2299883007/diff/100001/android_webview/test/B...
android_webview/test/BUILD.gn:109: "//components/external_video_surface:java",
remove this?

[boliu, 2016-09-07 16:56:02]

On 2016/09/07 16:52:32, boliu wrote:
>
https://codereview.chromium.org/2299883007/diff/100001/android_webview/test/B...
> File android_webview/test/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2299883007/diff/100001/android_webview/test/B...
> android_webview/test/BUILD.gn:109: "//components/external_video_surface:java",
> remove this?

lgtm after that

[Tima Vaisburd, 2016-09-07 18:05:34]

https://codereview.chromium.org/2299883007/diff/100001/android_webview/test/B...
File android_webview/test/BUILD.gn (right):

https://codereview.chromium.org/2299883007/diff/100001/android_webview/test/B...
android_webview/test/BUILD.gn:109: "//components/external_video_surface:java",
On 2016/09/07 16:52:32, boliu wrote:
> remove this?

Done.

[Tima Vaisburd, 2016-09-07 18:05:56]

The CQ bit was checked by timav@chromium.org

[Tima Vaisburd, 2016-09-07 18:05:56]

The patchset sent to the CQ was uploaded after l-g-t-m from boliu@chromium.org
Link to the patchset: https://codereview.chromium.org/2299883007/#ps120001
(title: "Removed another external_video_surface reference")

[commit-bot: I haz the power, 2016-09-07 18:06:25]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-07 19:24:32]

Description was changed from

==========
Remove video overlay support from WebView

Video overlay for embedded videos (aka hole punching)
was diabled in https://codereview.chromium.org/2026423002/.
This CL removes it completely.

BUG=616583
==========

to

==========
Remove video overlay support from WebView

Video overlay for embedded videos (aka hole punching)
was diabled in https://codereview.chromium.org/2026423002/.
This CL removes it completely.

BUG=616583
==========

[commit-bot: I haz the power, 2016-09-07 19:24:33]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2016-09-07 19:26:48]

Description was changed from

==========
Remove video overlay support from WebView

Video overlay for embedded videos (aka hole punching)
was diabled in https://codereview.chromium.org/2026423002/.
This CL removes it completely.

BUG=616583
==========

to

==========
Remove video overlay support from WebView

Video overlay for embedded videos (aka hole punching)
was diabled in https://codereview.chromium.org/2026423002/.
This CL removes it completely.

BUG=616583

Committed: https://crrev.com/78b35e8a8b01f2db768a30033dee03323f71f316
Cr-Commit-Position: refs/heads/master@{#417025}
==========

[commit-bot: I haz the power, 2016-09-07 19:26:49]

Patchset 5 (id:??) landed as
https://crrev.com/78b35e8a8b01f2db768a30033dee03323f71f316
Cr-Commit-Position: refs/heads/master@{#417025}