[NFC] Only connect to NFC impl if ExecutionContext supports it

[NFC] Only connect to NFC impl if ExecutionContext supports it

NFC has the spec property that it can only be accessed from the main
frame. Currently, NFC.cpp unconditionally connects to the NFC impl in
its constructor and then implements this spec by disallowing requests
made in the context of nested frames in the implementations of the IDL
invocations (e.g., push).

An upcoming refactor of NFC for servicification would be eased by the
simplifying property that the browser receives NFC requests only from
the main frame. This CL adds this property by having NFC.cpp connect
to the NFC implementation only if its ExecutionContext supports it.
This change will have no user-visible effect (all of the IDL invocations
will fail in the same way as they did before, as the CL does not change
the checks that they are making), but makes the servicification task
significantly easier.

BUG=680924
Review-Url: https://codereview.chromium.org/2875283002
Cr-Commit-Position: refs/heads/master@{#471707}
Committed: https://chromium.googlesource.com/chromium/src/+/b167a1b1183710ff042fd72c8ca53aae3bb81bd3

[blundell, 2017-05-12 10:27:00]

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

[commit-bot: I haz the power, 2017-05-12 10:27:22]

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-05-12 11:29:59]

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

[commit-bot: I haz the power, 2017-05-12 11:29:59]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[blundell, 2017-05-12 12:32:42]

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

[blundell, 2017-05-12 12:32:42]

Jochen,

This is what I was talking about over chat. Does this make sense?

[blundell, 2017-05-12 13:51:02]

blundell@chromium.org changed reviewers:
+ alexander.shalamov@intel.com

[blundell, 2017-05-12 13:51:02]

+alexander.shalamov@intel.com for OWNERS

[jochen (gone - plz use gerrit), 2017-05-15 05:34:55]

lgtm

https://codereview.chromium.org/2875283002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/nfc/NFC.h (right):

https://codereview.chromium.org/2875283002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/nfc/NFC.h:70: bool
IsSupportedInContext(ExecutionContext*, String&);
nit. should be String& error_message, as it's not obvious what that parameter is
otherwise

[shalamov, 2017-05-15 06:34:19]

Thanks!

lgtm

[blundell, 2017-05-15 07:27:45]

The CQ bit was checked by blundell@chromium.org

[blundell, 2017-05-15 07:27:45]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
alexander.shalamov@intel.com
Link to the patchset: https://codereview.chromium.org/2875283002/#ps40001
(title: "Rebase")

[blundell, 2017-05-15 07:27:55]

https://codereview.chromium.org/2875283002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/nfc/NFC.h (right):

https://codereview.chromium.org/2875283002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/nfc/NFC.h:70: bool
IsSupportedInContext(ExecutionContext*, String&);
On 2017/05/15 05:34:55, jochen wrote:
> nit. should be String& error_message, as it's not obvious what that parameter
is
> otherwise

Done.

[commit-bot: I haz the power, 2017-05-15 07:28:05]

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-05-15 09:08:31]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1494833265457960,
"parent_rev": "bb0640b700217f492e6d68bd2f3a31afea8c8c98", "commit_rev":
"b167a1b1183710ff042fd72c8ca53aae3bb81bd3"}

[commit-bot: I haz the power, 2017-05-15 09:08:40]

Description was changed from

==========
[NFC] Only connect to NFC impl if ExecutionContext supports it

NFC has the spec property that it can only be accessed from the main
frame. Currently, NFC.cpp unconditionally connects to the NFC impl in
its constructor and then implements this spec by disallowing requests
made in the context of nested frames in the implementations of the IDL
invocations (e.g., push).

An upcoming refactor of NFC for servicification would be eased by the
simplifying property that the browser receives NFC requests only from
the main frame. This CL adds this property by having NFC.cpp connect
to the NFC implementation only if its ExecutionContext supports it.
This change will have no user-visible effect (all of the IDL invocations
will fail in the same way as they did before, as the CL does not change
the checks that they are making), but makes the servicification task
significantly easier.

BUG=680924
==========

to

==========
[NFC] Only connect to NFC impl if ExecutionContext supports it

NFC has the spec property that it can only be accessed from the main
frame. Currently, NFC.cpp unconditionally connects to the NFC impl in
its constructor and then implements this spec by disallowing requests
made in the context of nested frames in the implementations of the IDL
invocations (e.g., push).

An upcoming refactor of NFC for servicification would be eased by the
simplifying property that the browser receives NFC requests only from
the main frame. This CL adds this property by having NFC.cpp connect
to the NFC implementation only if its ExecutionContext supports it.
This change will have no user-visible effect (all of the IDL invocations
will fail in the same way as they did before, as the CL does not change
the checks that they are making), but makes the servicification task
significantly easier.

BUG=680924

Review-Url: https://codereview.chromium.org/2875283002
Cr-Commit-Position: refs/heads/master@{#471707}
Committed:
https://chromium.googlesource.com/chromium/src/+/b167a1b1183710ff042fd72c8ca5...
==========

[commit-bot: I haz the power, 2017-05-15 09:08:41]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/b167a1b1183710ff042fd72c8ca5...