Disable WebShare on platforms other than android.

Disable WebShare on platforms other than android.

This adds origin_trial_os_whitelist to RuntimeEnabledFeatures. If set,
an origin trial will only be available on the whitelisted OSes.

BUG=635741
Committed: https://crrev.com/4d59fb592c7df5a06274bbffbe5a565f01307c11
Cr-Commit-Position: refs/heads/master@{#422981}

[Sam McNally, 2016-08-16 08:33:01]

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

[commit-bot: I haz the power, 2016-08-16 08:33:17]

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-08-16 10:34:18]

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

[commit-bot: I haz the power, 2016-08-16 10:34:19]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Sam McNally, 2016-08-16 23:47:58]

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

[commit-bot: I haz the power, 2016-08-16 23:48:28]

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-08-17 01:12:51]

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

[commit-bot: I haz the power, 2016-08-17 01:12:54]

Dry run: This issue passed the CQ dry run.

[Sam McNally, 2016-08-17 02:41:01]

Patchset #1 (id:1) has been deleted

[Sam McNally, 2016-08-17 05:10:16]

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

[commit-bot: I haz the power, 2016-08-17 05:10:26]

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

[Sam McNally, 2016-08-17 07:13:45]

Patchset #1 (id:20001) has been deleted

[commit-bot: I haz the power, 2016-08-17 08:12:29]

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

[commit-bot: I haz the power, 2016-08-17 08:12:30]

Dry run: This issue passed the CQ dry run.

[Sam McNally, 2016-08-17 09:50:23]

sammc@chromium.org changed reviewers:
+ mgiuca@chromium.org

[Matt Giuca, 2016-08-22 20:38:44]

https://codereview.chromium.org/2249043002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/platform/android/webexposed/global-interface-listing-expected.txt
(right):

https://codereview.chromium.org/2249043002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/platform/android/webexposed/global-interface-listing-expected.txt:1:
CONSOLE WARNING: line 94: 'webkitOfflineAudioContext' is deprecated. Please use
'OfflineAudioContext' instead.
You added an entire new file for Android that's a copy of the other one? That
seems ... bad.

Where is this linked up?

[Sam McNally, 2016-08-23 16:54:52]

https://codereview.chromium.org/2249043002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/platform/android/webexposed/global-interface-listing-expected.txt
(right):

https://codereview.chromium.org/2249043002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/platform/android/webexposed/global-interface-listing-expected.txt:1:
CONSOLE WARNING: line 94: 'webkitOfflineAudioContext' is deprecated. Please use
'OfflineAudioContext' instead.
On 2016/08/22 20:38:44, Matt Giuca wrote:
> You added an entire new file for Android that's a copy of the other one? That
> seems ... bad.
> 
> Where is this linked up?

This seems to be how platform-specific expectations are done. If you can find a
better alternative, I'd be happy to try it.

The test runner chooses the right expectation file.

[Matt Giuca, 2016-08-23 17:43:39]

https://codereview.chromium.org/2249043002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/platform/android/webexposed/global-interface-listing-expected.txt
(right):

https://codereview.chromium.org/2249043002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/platform/android/webexposed/global-interface-listing-expected.txt:1:
CONSOLE WARNING: line 94: 'webkitOfflineAudioContext' is deprecated. Please use
'OfflineAudioContext' instead.
On 2016/08/23 16:54:52, Sam McNally wrote:
> On 2016/08/22 20:38:44, Matt Giuca wrote:
> > You added an entire new file for Android that's a copy of the other one?
That
> > seems ... bad.
> > 
> > Where is this linked up?
> 
> This seems to be how platform-specific expectations are done. If you can find
a
> better alternative, I'd be happy to try it.
> 
> The test runner chooses the right expectation file.

Is this the first ever Android-specific expectation? Just seems bizarre and
scary to copy an 8000-line file for just this purpose.

There aren't any other global-interface-listing-expected.txt files in any other
LayoutTests/platform directories. Is this like the first ever platform-specific
expectation? Perhaps we're doing it wrong in that case, and we should just have
the method exist but fail somehow.

[Matt Giuca, 2016-08-23 21:40:59]

mgiuca@chromium.org changed reviewers:
+ esprehn@chromium.org

[Matt Giuca, 2016-08-23 21:40:59]

+Elliott: For your opinion. We can't find any precedent for making an API
completely missing on some platforms but not others. The problem is the
global-interface-listing-expected.txt file, which seems to need a full copy for
each platform.

Three choices we can see:
1. Full copy of global-interface-listing-expected.txt for Android.
2. Blacklist navigator.share in the global interface listing text file (and just
have it covered by our own tests).
3. Make navigator.share present on all platforms but fail on everywhere but
Android. (This makes it impossible to feature-detect properly.)

Leaning towards #2, do you think that's the right call?

[esprehn, 2016-08-24 05:16:05]

On 2016/08/23 at 21:40:59, mgiuca wrote:
> +Elliott: For your opinion. We can't find any precedent for making an API
completely missing on some platforms but not others. The problem is the
global-interface-listing-expected.txt file, which seems to need a full copy for
each platform.
> 
> Three choices we can see:
> 1. Full copy of global-interface-listing-expected.txt for Android.
> 2. Blacklist navigator.share in the global interface listing text file (and
just have it covered by our own tests).
> 3. Make navigator.share present on all platforms but fail on everywhere but
Android. (This makes it impossible to feature-detect properly.)
> 
> Leaning towards #2, do you think that's the right call?

I might enable navigator.share always in Content Shell which is where this test
runs. The code should be resilient to having no impl for that platform and
reject the promises or something anyway. Then I'd make the production chrome
disable the feature except on android?

[esprehn, 2016-08-24 05:18:35]

esprehn@chromium.org changed reviewers:
+ dglazkov@chromium.org, jochen@chromium.org

[esprehn, 2016-08-24 05:18:35]

jochen@, dglazkov@ Got other ideas? :)

[dglazkov, 2016-08-24 16:13:25]

On 2016/08/24 at 05:18:35, esprehn wrote:
> jochen@, dglazkov@ Got other ideas? :)

This (shipping an API that's only available on one OS) does seem to have a
smell. How can we avoid it? IOW, can we formulate sharing being disabled in
terms of an API output? Can the promise reject immediately, for instance?

[Matt Giuca, 2016-08-24 17:27:36]

On 2016/08/24 16:13:25, dglazkov wrote:
> On 2016/08/24 at 05:18:35, esprehn wrote:
> > jochen@, dglazkov@ Got other ideas? :)
> 
> This (shipping an API that's only available on one OS) does seem to have a
> smell. How can we avoid it? IOW, can we formulate sharing being disabled in
> terms of an API output? Can the promise reject immediately, for instance?

It's possible that some platforms (e.g., Desktop Linux) may never support this
API, due to simply having no share infrastructure. We want sites to be able to
feature detect so they can disable or hide their Sharing UI in this case.

Previously, the proposal was that navigator.share always exists but you can use
a separate method navigator.canShare to detect whether it's supported. Then we
decided (in our discussions with Mozilla editor Marcos Cáceres) to remove this
method an just use the presence or absence of navigator.share as a signal.

https://github.com/WICG/web-share/pull/8

If you want navigator.share to be present no matter what, I think we have to go
back to adding canShare(). Which we can do, but it requires agreement at the
spec level. We can't just reject the promise because then you can't feature
detect.

[Matt Giuca, 2016-08-24 17:35:19]

On 2016/08/24 17:27:36, Matt Giuca wrote:
> On 2016/08/24 16:13:25, dglazkov wrote:
> > On 2016/08/24 at 05:18:35, esprehn wrote:
> > > jochen@, dglazkov@ Got other ideas? :)
> > 
> > This (shipping an API that's only available on one OS) does seem to have a
> > smell. How can we avoid it? IOW, can we formulate sharing being disabled in
> > terms of an API output? Can the promise reject immediately, for instance?
> 
> It's possible that some platforms (e.g., Desktop Linux) may never support this
> API, due to simply having no share infrastructure. We want sites to be able to
> feature detect so they can disable or hide their Sharing UI in this case.
> 
> Previously, the proposal was that navigator.share always exists but you can
use
> a separate method navigator.canShare to detect whether it's supported. Then we
> decided (in our discussions with Mozilla editor Marcos Cáceres) to remove this
> method an just use the presence or absence of navigator.share as a signal.
> 
> https://github.com/WICG/web-share/pull/8
> 
> If you want navigator.share to be present no matter what, I think we have to
go
> back to adding canShare(). Which we can do, but it requires agreement at the
> spec level. We can't just reject the promise because then you can't feature
> detect.

I'll add that there's a precedent here: lots of APIs that kinda-sorta-only make
sense on mobile (Accelerometer, Vibration, Screen Orientation). All of these API
surfaces are visible on the desktop platforms as well, but just don't really do
anything meaningful.

[dglazkov, 2016-08-24 18:07:50]

On 2016/08/24 at 17:35:19, mgiuca wrote:
> On 2016/08/24 17:27:36, Matt Giuca wrote:
> > On 2016/08/24 16:13:25, dglazkov wrote:
> > > On 2016/08/24 at 05:18:35, esprehn wrote:
> > > > jochen@, dglazkov@ Got other ideas? :)
> > > 
> > > This (shipping an API that's only available on one OS) does seem to have a
> > > smell. How can we avoid it? IOW, can we formulate sharing being disabled
in
> > > terms of an API output? Can the promise reject immediately, for instance?
> > 
> > It's possible that some platforms (e.g., Desktop Linux) may never support
this
> > API, due to simply having no share infrastructure. We want sites to be able
to
> > feature detect so they can disable or hide their Sharing UI in this case.
> > 
> > Previously, the proposal was that navigator.share always exists but you can
use
> > a separate method navigator.canShare to detect whether it's supported. Then
we
> > decided (in our discussions with Mozilla editor Marcos Cáceres) to remove
this
> > method an just use the presence or absence of navigator.share as a signal.
> > 
> > https://github.com/WICG/web-share/pull/8
> > 
> > If you want navigator.share to be present no matter what, I think we have to
go
> > back to adding canShare(). Which we can do, but it requires agreement at the
> > spec level. We can't just reject the promise because then you can't feature
> > detect.
> 
> I'll add that there's a precedent here: lots of APIs that kinda-sorta-only
make sense on mobile (Accelerometer, Vibration, Screen Orientation). All of
these API surfaces are visible on the desktop platforms as well, but just don't
really do anything meaningful.

I would prefer that we follow that precedent. IOW, the API surface does not
change from OS to OS, but the way it responds does. If you'd like me to chime in
on the bug, I can.

[Matt Giuca, 2016-08-24 18:22:53]

> I would prefer that we follow that precedent. IOW, the API surface does not
> change from OS to OS, but the way it responds does. If you'd like me to chime
in
> on the bug, I can.

Seems reasonable. No need to chime in unless you want to. I'll just link to this
discussion from the bug.

[Matt Giuca, 2016-09-29 03:54:24]

On 2016/08/24 18:22:53, Matt Giuca (OOO til Oct 4) wrote:
> > I would prefer that we follow that precedent. IOW, the API surface does not
> > change from OS to OS, but the way it responds does. If you'd like me to
chime
> in
> > on the bug, I can.
> 
> Seems reasonable. No need to chime in unless you want to. I'll just link to
this
> discussion from the bug.

We had some good discussion on https://github.com/WICG/web-share/pull/8. Looks
like the new consensus is that canShare adds more complexity without actually
removing the need to test for undefinedness. So let's drop canShare (for now at
least) and have this be undefined on those non-Android platforms.

Therefore, can we go ahead with this CL?

https://codereview.chromium.org/2249043002#msg22 has some tips for how to get
around the testing issue (make it always enabled in content shell, and then
disable on other platforms, with the tests only running on content shell you can
assume it is always present).

[dglazkov, 2016-09-29 04:01:36]

lgtm

[Matt Giuca, 2016-09-29 04:02:28]

Description was changed from

==========
Disable WebShare on platforms other than android.

BUG=635741
==========

to

==========
Disable WebShare on platforms other than android.

BUG=635741
==========

[Matt Giuca, 2016-09-29 04:02:36]

On 2016/09/29 04:01:36, dglazkov wrote:
> lgtm

Unclosed.

[Sam McNally, 2016-09-29 07:39:00]

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

[commit-bot: I haz the power, 2016-09-29 07:39:26]

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

[Sam McNally, 2016-09-29 08:04:40]

Description was changed from

==========
Disable WebShare on platforms other than android.

BUG=635741
==========

to

==========
Disable WebShare on platforms other than android.

This adds origin_trial_os_whitelist to RuntimeEnabledFeatures. If set,
an origin trial will only be available on the whitelisted OSes.

BUG=635741
==========

[Sam McNally, 2016-09-29 08:07:12]

The previous approach in patch set 1 doesn't work with origin trials. It set the
default availability to false on other platforms, but origin trials enables a
feature if it's either available by default or the site is in the origin trial.

Instead, I've changed RuntimeEnabledFeatures and the generated origin trials
code to whitelist the webshare origin trial to Android. PTAL.

[commit-bot: I haz the power, 2016-09-29 08:55:53]

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

[commit-bot: I haz the power, 2016-09-29 08:55:54]

Dry run: This issue passed the CQ dry run.

[dglazkov, 2016-09-29 15:59:47]

https://codereview.chromium.org/2249043002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/build/scripts/make_runtime_features.py (right):

https://codereview.chromium.org/2249043002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/make_runtime_features.py:52:
'origin_trial_os_whitelist': [],
just origin_trial_os? Also, will you need to modify this again a feature exits
origin trials?

Could this be somehow orthogonal to origin trial flags?

[Sam McNally, 2016-10-04 01:53:18]

Patchset #3 (id:80001) has been deleted

[Sam McNally, 2016-10-04 01:54:03]

https://codereview.chromium.org/2249043002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/build/scripts/make_runtime_features.py (right):

https://codereview.chromium.org/2249043002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/make_runtime_features.py:52:
'origin_trial_os_whitelist': [],
On 2016/09/29 15:59:47, dglazkov wrote:
> just origin_trial_os? 
Done.

> Also, will you need to modify this again a feature exits
> origin trials?

When the origin trial ends, the previous approach for restricting platforms will
work again.
> 
> Could this be somehow orthogonal to origin trial flags?

Runtime features are either on or off and origin trials can turn an off into an
on. To merge this into the existing per-platform controls we'd need another
state for runtime features that isn't affected by origin trials.

[Sam McNally, 2016-10-04 01:54:17]

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

[commit-bot: I haz the power, 2016-10-04 01:55:00]

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-10-04 03:51:07]

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

[commit-bot: I haz the power, 2016-10-04 03:51:08]

Dry run: This issue passed the CQ dry run.

[dglazkov, 2016-10-04 17:03:55]

lgtm

[Sam McNally, 2016-10-04 22:55:27]

The CQ bit was checked by sammc@chromium.org

[commit-bot: I haz the power, 2016-10-04 22:55:49]

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-10-04 23:00:01]

Committed patchset #4 (id:120001)

[commit-bot: I haz the power, 2016-10-04 23:01:48]

Description was changed from

==========
Disable WebShare on platforms other than android.

This adds origin_trial_os_whitelist to RuntimeEnabledFeatures. If set,
an origin trial will only be available on the whitelisted OSes.

BUG=635741
==========

to

==========
Disable WebShare on platforms other than android.

This adds origin_trial_os_whitelist to RuntimeEnabledFeatures. If set,
an origin trial will only be available on the whitelisted OSes.

BUG=635741

Committed: https://crrev.com/4d59fb592c7df5a06274bbffbe5a565f01307c11
Cr-Commit-Position: refs/heads/master@{#422981}
==========

[commit-bot: I haz the power, 2016-10-04 23:01:49]

Patchset 4 (id:??) landed as
https://crrev.com/4d59fb592c7df5a06274bbffbe5a565f01307c11
Cr-Commit-Position: refs/heads/master@{#422981}