Disable MediaSession API for WebView

Disable MediaSession API for WebView

Currently there is no UI for media metadata and controls on
WebView. Therefore we should disable the API on WebView. It is
reasonable to let the host app show the UI itself on WebView.

The API intent to ship is still pending approval so it's not
enabled on any platform by default yet. After getting approved,
the next step is to just enable the API on Android and WebView
will be excluded because of this CL.

Intent to ship (pending approval):
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/xIpJ_303phw

BUG=678979
Review-Url: https://codereview.chromium.org/2613133003
Cr-Commit-Position: refs/heads/master@{#442640}
Committed: https://chromium.googlesource.com/chromium/src/+/30fb31340be3ffd85db42b176f997527fa92538f

[Zhiqiang Zhang (Slow), 2017-01-06 20:14:19]

zqzhang@chromium.org changed reviewers:
+ torne@chromium.org

[Torne, 2017-01-09 11:47:06]

https://codereview.chromium.org/2613133003/diff/1/android_webview/lib/main/aw...
File android_webview/lib/main/aw_main_delegate.cc (right):

https://codereview.chromium.org/2613133003/diff/1/android_webview/lib/main/aw...
android_webview/lib/main/aw_main_delegate.cc:117:
cl->AppendSwitchASCII(switches::kDisableBlinkFeatures, "MediaSession");
I'm not too familiar with how command line manipulation works; is it safe to
call AppendSwitch for kDisableBlinkFeatures multiple times with different
values? Do they all take effect?

[Zhiqiang Zhang (Slow), 2017-01-09 14:29:21]

PTAL

https://codereview.chromium.org/2613133003/diff/1/android_webview/lib/main/aw...
File android_webview/lib/main/aw_main_delegate.cc (right):

https://codereview.chromium.org/2613133003/diff/1/android_webview/lib/main/aw...
android_webview/lib/main/aw_main_delegate.cc:117:
cl->AppendSwitchASCII(switches::kDisableBlinkFeatures, "MediaSession");
On 2017/01/09 11:47:06, Torne wrote:
> I'm not too familiar with how command line manipulation works; is it safe to
> call AppendSwitch for kDisableBlinkFeatures multiple times with different
> values? Do they all take effect?

You are right, they will conflict.
Now I just follow how RemotePlayback and Presentation does.

[Torne, 2017-01-09 14:37:07]

lgtm

[Zhiqiang Zhang (Slow), 2017-01-09 18:22:00]

zqzhang@chromium.org changed reviewers:
+ jochen@chromium.org

[Zhiqiang Zhang (Slow), 2017-01-09 18:22:01]

+jochen: content/* and third_party/WebKit/*

[jochen (gone - plz use gerrit), 2017-01-10 09:54:51]

can you send an email to blink-dev@ about disabling this API?

can you also explain why we can't just add the UI instead?

https://codereview.chromium.org/2613133003/diff/20001/android_webview/lib/mai...
File android_webview/lib/main/aw_main_delegate.cc (right):

https://codereview.chromium.org/2613133003/diff/20001/android_webview/lib/mai...
android_webview/lib/main/aw_main_delegate.cc:119: 
nit. only one empty line

[jochen (gone - plz use gerrit), 2017-01-10 09:55:09]

jochen@chromium.org changed reviewers:
+ mlamouri@chromium.org

[jochen (gone - plz use gerrit), 2017-01-10 09:55:10]

lgtm with actions taken

[Torne, 2017-01-10 10:50:36]

On 2017/01/10 09:54:51, jochen wrote:
> can you send an email to blink-dev@ about disabling this API?
> 
> can you also explain why we can't just add the UI instead?

WebView's UI is expected to be controlled by the application, so we generally
don't add any new UI without first adding callbacks for the embedding app to
replace the UI with their own. This can only be done in a new Android OS
release, because the WebView API is not updatable.

>
https://codereview.chromium.org/2613133003/diff/20001/android_webview/lib/mai...
> File android_webview/lib/main/aw_main_delegate.cc (right):
> 
>
https://codereview.chromium.org/2613133003/diff/20001/android_webview/lib/mai...
> android_webview/lib/main/aw_main_delegate.cc:119: 
> nit. only one empty line

[mlamouri (slow - plz ping), 2017-01-10 10:56:04]

lgtm

[Zhiqiang Zhang (Slow), 2017-01-10 11:05:43]

Description was changed from

==========
Disable MediaSession API for WebView

Currently there is no UI for media metadata and controls on WebView.
Therefore we should disable the API on WebView.

BUG=678979
==========

to

==========
Disable MediaSession API for WebView

Currently there is no UI for media metadata and controls on
WebView. Therefore we should disable the API on WebView. It is
reasonable to let the host app show the UI itself on WebView.

The API intent to ship is still pending approval so it's not
enabled on any platform by default yet. After getting approved,
the next step is to just enable the API on Android and WebView
will be excluded because of this CL.

Intent to ship (pending approval):
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/xIpJ_303phw

BUG=678979
==========

[Zhiqiang Zhang (Slow), 2017-01-10 11:06:15]

On 2017/01/10 09:54:51, jochen wrote:
> can you send an email to blink-dev@ about disabling this API?

We have already sent an intent to ship for the API, where we pointed we plan to
ship the API on Android only (without WebView).
The intent to ship is pending approval. After the approval, we can just turn on
the API for Android and WebView will be excluded because of this CL.
I think there's no need to send another email since the discussions will happen
in the intent to ship.

> can you also explain why we can't just add the UI instead?

Currently, the UI code is in chrome/, so it's not available to WebView.
We discussed with the WebView team and agreed not to show the UI.
It is reasonable to let the host app show the UI itself if it wants to do so.

I've updated the CL description to include the answers to your questions.

https://codereview.chromium.org/2613133003/diff/20001/android_webview/lib/mai...
File android_webview/lib/main/aw_main_delegate.cc (right):

https://codereview.chromium.org/2613133003/diff/20001/android_webview/lib/mai...
android_webview/lib/main/aw_main_delegate.cc:119: 
On 2017/01/10 09:54:50, jochen wrote:
> nit. only one empty line

Done.

[Zhiqiang Zhang (Slow), 2017-01-10 11:07:22]

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

[commit-bot: I haz the power, 2017-01-10 11:07:39]

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, 2017-01-10 12:34:55]

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

[commit-bot: I haz the power, 2017-01-10 12:34:56]

Dry run: This issue passed the CQ dry run.

[jochen (gone - plz use gerrit), 2017-01-10 14:17:44]

ok, still lgtm

[Zhiqiang Zhang (Slow), 2017-01-10 17:20:56]

The CQ bit was checked by zqzhang@chromium.org

[Zhiqiang Zhang (Slow), 2017-01-10 17:20:57]

The patchset sent to the CQ was uploaded after l-g-t-m from torne@chromium.org,
mlamouri@chromium.org, jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/2613133003/#ps60001
(title: "rebased")

[commit-bot: I haz the power, 2017-01-10 17:21:15]

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, 2017-01-10 18:52:28]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1484068856628500,
"parent_rev": "1f59c91a97dad5a9061d4b755544ade9f3af58a0", "commit_rev":
"30fb31340be3ffd85db42b176f997527fa92538f"}

[commit-bot: I haz the power, 2017-01-10 18:52:56]

Description was changed from

==========
Disable MediaSession API for WebView

Currently there is no UI for media metadata and controls on
WebView. Therefore we should disable the API on WebView. It is
reasonable to let the host app show the UI itself on WebView.

The API intent to ship is still pending approval so it's not
enabled on any platform by default yet. After getting approved,
the next step is to just enable the API on Android and WebView
will be excluded because of this CL.

Intent to ship (pending approval):
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/xIpJ_303phw

BUG=678979
==========

to

==========
Disable MediaSession API for WebView

Currently there is no UI for media metadata and controls on
WebView. Therefore we should disable the API on WebView. It is
reasonable to let the host app show the UI itself on WebView.

The API intent to ship is still pending approval so it's not
enabled on any platform by default yet. After getting approved,
the next step is to just enable the API on Android and WebView
will be excluded because of this CL.

Intent to ship (pending approval):
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/xIpJ_303phw

BUG=678979

Review-Url: https://codereview.chromium.org/2613133003
Cr-Commit-Position: refs/heads/master@{#442640}
Committed:
https://chromium.googlesource.com/chromium/src/+/30fb31340be3ffd85db42b176f99...
==========

[commit-bot: I haz the power, 2017-01-10 18:52:57]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/30fb31340be3ffd85db42b176f99...