Require a user gesture to use navigator.share.

Require a user gesture to use navigator.share.

BUG=635387
Committed: https://crrev.com/69be009de71cb4bcf0e32ec5c4e8a86876458c1a
Cr-Commit-Position: refs/heads/master@{#410556}

[Sam McNally, 2016-08-08 05:02:40]

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

[commit-bot: I haz the power, 2016-08-08 05:02:54]

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-08 05:04:07]

Patchset #1 (id:1) has been deleted

[Sam McNally, 2016-08-08 05:04:14]

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

[commit-bot: I haz the power, 2016-08-08 05:04:33]

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-08 06:32:00]

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

[commit-bot: I haz the power, 2016-08-08 06:32:01]

Dry run: This issue passed the CQ dry run.

[Sam McNally, 2016-08-08 06:33:11]

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

[Matt Giuca, 2016-08-08 08:00:06]

https://codereview.chromium.org/2219383002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/webshare/share-without-user-gesture.html
(right):

https://codereview.chromium.org/2219383002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webshare/share-without-user-gesture.html:9:
mock.pushShareResult('the title', 'the message', 'error from browser');
I think this test should be based on the success test, not the error test. That
is, it should be a case that *would* succeed, were it not for the SecurityError.

https://codereview.chromium.org/2219383002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webshare/NavigatorShare.cpp (right):

https://codereview.chromium.org/2219383002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webshare/NavigatorShare.cpp:86: return
ScriptPromise::rejectWithDOMException(scriptState,
DOMException::create(SecurityError, "Must be handling a user gesture to perform
a share request."));
nit: Maybe split this into a temp variable, for line length.

[Sam McNally, 2016-08-08 08:14:50]

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

[commit-bot: I haz the power, 2016-08-08 08:15:05]

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-08 09:08:44]

https://codereview.chromium.org/2219383002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/webshare/share-without-user-gesture.html
(right):

https://codereview.chromium.org/2219383002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webshare/share-without-user-gesture.html:9:
mock.pushShareResult('the title', 'the message', 'error from browser');
On 2016/08/08 08:00:06, Matt Giuca wrote:
> I think this test should be based on the success test, not the error test.
That
> is, it should be a case that *would* succeed, were it not for the
SecurityError.

Removed this line entirely so the mock will cause a test failure if the call
succeeds.

https://codereview.chromium.org/2219383002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/webshare/NavigatorShare.cpp (right):

https://codereview.chromium.org/2219383002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/webshare/NavigatorShare.cpp:86: return
ScriptPromise::rejectWithDOMException(scriptState,
DOMException::create(SecurityError, "Must be handling a user gesture to perform
a share request."));
On 2016/08/08 08:00:06, Matt Giuca wrote:
> nit: Maybe split this into a temp variable, for line length.

Done.

[commit-bot: I haz the power, 2016-08-08 09:18:40]

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

[commit-bot: I haz the power, 2016-08-08 09:18:41]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Matt Giuca, 2016-08-09 00:09:26]

lgtm

https://codereview.chromium.org/2219383002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/webshare/share-without-user-gesture.html
(right):

https://codereview.chromium.org/2219383002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webshare/share-without-user-gesture.html:9:
mock.pushShareResult('the title', 'the message', 'error from browser');
On 2016/08/08 09:08:44, Sam McNally wrote:
> On 2016/08/08 08:00:06, Matt Giuca wrote:
> > I think this test should be based on the success test, not the error test.
> That
> > is, it should be a case that *would* succeed, were it not for the
> SecurityError.
> 
> Removed this line entirely so the mock will cause a test failure if the call
> succeeds.

Right, my reading of the code is that it will fail with "Unexpected call to mojo
share method", right? If so, that's great.

[Sam McNally, 2016-08-09 00:53:49]

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

[commit-bot: I haz the power, 2016-08-09 00:54:21]

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-09 01:02:37]

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

[commit-bot: I haz the power, 2016-08-09 01:02:38]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)

[Sam McNally, 2016-08-09 01:24:35]

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

[Sam McNally, 2016-08-09 01:24:36]

+haraken

https://codereview.chromium.org/2219383002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/webshare/share-without-user-gesture.html
(right):

https://codereview.chromium.org/2219383002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webshare/share-without-user-gesture.html:9:
mock.pushShareResult('the title', 'the message', 'error from browser');
On 2016/08/09 00:09:26, Matt Giuca wrote:
> On 2016/08/08 09:08:44, Sam McNally wrote:
> > On 2016/08/08 08:00:06, Matt Giuca wrote:
> > > I think this test should be based on the success test, not the error test.
> > That
> > > is, it should be a case that *would* succeed, were it not for the
> > SecurityError.
> > 
> > Removed this line entirely so the mock will cause a test failure if the call
> > succeeds.
> 
> Right, my reading of the code is that it will fail with "Unexpected call to
mojo
> share method", right? If so, that's great.

Yes.

[haraken, 2016-08-09 02:38:55]

LGTM

[Sam McNally, 2016-08-09 02:41:59]

The CQ bit was checked by sammc@chromium.org

[Sam McNally, 2016-08-09 02:41:59]

The patchset sent to the CQ was uploaded after l-g-t-m from mgiuca@chromium.org
Link to the patchset: https://codereview.chromium.org/2219383002/#ps60001
(title: " ")

[commit-bot: I haz the power, 2016-08-09 02:42:27]

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-09 03:05:04]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-08-09 03:06:50]

Description was changed from

==========
Require a user gesture to use navigator.share.

BUG=635387
==========

to

==========
Require a user gesture to use navigator.share.

BUG=635387

Committed: https://crrev.com/69be009de71cb4bcf0e32ec5c4e8a86876458c1a
Cr-Commit-Position: refs/heads/master@{#410556}
==========

[commit-bot: I haz the power, 2016-08-09 03:06:51]

Patchset 3 (id:??) landed as
https://crrev.com/69be009de71cb4bcf0e32ec5c4e8a86876458c1a
Cr-Commit-Position: refs/heads/master@{#410556}