[Device Service] Decouple NFC implementation from //content

[Device Service] Decouple NFC implementation from //content

This CL ports the NFC implementation from being hosted by //content
to being hosted by the Device Service. This task is challenging because
the NFC implementation needs access to the Activity of the WebContents
that is associated with the requesting render frame. Moreore, that
Activity can change if the WebContents is reparented (e.g., moved from
being in a custom tab to being in Chrome by the user).

To handle these challenges, this CL takes a similar approach to that
taken for WakeLock: information is passed to the Device Service that
allows the NFC implementation to map a host ID to the Activity associated
with that ID. Specifically:

- The NFCProvider Mojo interface is introduced. It supports connecting to
  an NFC instance associated with a given host ID.
- The NFCDelegate.java interface is introduced. This interface supports
  mapping from host IDs to Activity, including tracking when the Activity
  associated with a host ID changes. This interface is passed into the
  Device Service by its embedder.
- NfcImpl.java uses the NFCDelegate to access the Activity that it needs.
- //content provides a ContentNFCDelegate implementation. This implementation
  maps host IDs to WebContents instances via an NfcHost internal helper
  class.
- //content/browser now handles requests to connect to NFC by forwarding
  them on to the Device Service (i.e., NFCProvider), passing the host
  ID associated with the WebContents in question in order to allow the
  NFC implementation to map back to that WebContents via NFCDelegate.

The net sum of the above is that //content's dependence on the NFC
implementation is removed.

Note that in addition to the below-described test, I also tested reparenting as
follows:
- Close Chromium on the device
- On the host, execute:
  adb shell "am start -a 'android.intent.action.VIEW' -n org.chromium.chrome/com.google.android.apps.chrome.Main -d https://colinblundell.github.io/nfc.html --es \"android.support.customtabs.extra.SESSION\" \"bla\""
- On the phone, you should see a custom tab pop up. Push a message.
- Reparent the tab by tapping "Open in Chromium." Verify that the tab was
  reparented, i.e., there was no reload operation.
- Bring the device close to the NFC tag and verify that you see "Message
  pushed" on the webpage.

BUG=680924
TEST=On Chrome on Android, go to about://flags and turn on "Experimental Web
Platform features" and "WebNFC Android". Restart Chrome. Obtain a physical
NFC tag.  Go to https://colinblundell.github.io/nfc.html and verify that the
page says "NFC API found". Enter a message on the webpage and press "Push
message to tag". Bring the phone near the tag and verifiy that the page outputs
"Message pushed." Press "Read data from tag", bring the phone near the tag,
and verify that the webpage outputs the message that you just pushed to the
tag.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2865653002
Cr-Commit-Position: refs/heads/master@{#471727}
Committed: https://chromium.googlesource.com/chromium/src/+/f5316fccc7bac30a357a1254aef8dc4a605bc045

[blundell, 2017-05-05 09:30:13]

Description was changed from

==========
[Device Service] Decouple NFC implementation from //content

TODO(blundell): Fill in description.

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

to

==========
[Device Service] Decouple NFC implementation from //content

TODO(blundell): Fill in description.

BUG=680924
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[blundell, 2017-05-05 10:03:13]

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

[commit-bot: I haz the power, 2017-05-05 10:03:25]

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-05 10:33:30]

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

[commit-bot: I haz the power, 2017-05-05 10:33:31]

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

[blundell, 2017-05-05 12:21:01]

Description was changed from

==========
[Device Service] Decouple NFC implementation from //content

TODO(blundell): Fill in description.

BUG=680924
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
[Device Service] Decouple NFC implementation from //content

This CL ports the NFC implementation from being hosted by //content
to being hosted by the Device Service. This task is challenging because
the NFC implementation needs access to the Activity of the WebContents
that is associated with the requesting render frame. Moreore, that
Activity can change if the WebContents is reparented (e.g., moved from
being in a custom tab to being in Chrome by the user).

To handle these challenges, this CL takes a similar approach to that
taken for WakeLock: information is passed to the Device Service that
allows the NFC implementation to map a host ID to the Activity associated
with that ID. Specifically:

- The NFCProvider Mojo interface is introduced. It supports connecting to
  an NFC instance associated with a given host ID.
- The NFCDelegate.java interface is introduced. This interface supports
  mapping from host IDs to Activity, including tracking when the Activity
  associated with a host ID changes. This interface is passed into the
  Device Service by its embedder.
- NfcImpl.java uses the NFCDelegate to access the Activity that it needs.
- //content provides a ContentNFCDelegate implementation. This implementation
  maps host IDs to WebContents instances via an NfcHost internal helper
  class.
- //content/browser now handles requests to connect to NFC by forwarding
  them on to the Device Service (i.e., NFCProvider), passing the host
  ID associated with the WebContents in question in order to allow the
  NFC implementation to map back to that WebContents via NFCDelegate.

The net sum of the above is that //content's dependence on the NFC
implementation is removed.

Note that in addition to the below-described test, I also tested reparenting as
follows:
- Close Chromium on the device
- On the host, execute:
  adb shell "am start -a 'android.intent.action.VIEW' -n org.chromium.chrome/  
>com.google.android.apps.chrome.Main -d https://colinblundell.github.io/nfc.   
>html --es \"android.support.customtabs.extra.SESSION\" \"bla\""
- On the phone, you should see a custom tab pop up. Push a message.
- Reparent the tab by tapping "Open in Chromium." Verify that the tab was
  reparented, i.e., there was no reload operation.
- Bring the device close to the NFC tag and verify that you see "Message
  pushed" on the webpage.

BUG=680924
TEST=On Chrome on Android, go to about://flags and turn on "Experimental Web
Platform features" and "WebNFC Android". Restart Chrome. Obtain a physical
NFC tag.  Go to https://colinblundell.github.io/nfc.html and verify that the
page says "NFC API found". Enter a message on the webpage and press "Push
message to tag". Bring the phone near the tag and verifiy that the page outputs
"Message pushed." Push "Read data from tag", bring the phone near the tag,
and verify that the webpage outputs the message that you just pushed to the
tag.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[blundell, 2017-05-05 12:21:58]

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

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

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-05 13:00:23]

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

[commit-bot: I haz the power, 2017-05-05 13:00:26]

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-05 14:01:04]

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

[commit-bot: I haz the power, 2017-05-05 14:01:18]

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-05 15:07:34]

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

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

Dry run: This issue passed the CQ dry run.

[blundell, 2017-05-05 16:11:49]

Description was changed from

==========
[Device Service] Decouple NFC implementation from //content

This CL ports the NFC implementation from being hosted by //content
to being hosted by the Device Service. This task is challenging because
the NFC implementation needs access to the Activity of the WebContents
that is associated with the requesting render frame. Moreore, that
Activity can change if the WebContents is reparented (e.g., moved from
being in a custom tab to being in Chrome by the user).

To handle these challenges, this CL takes a similar approach to that
taken for WakeLock: information is passed to the Device Service that
allows the NFC implementation to map a host ID to the Activity associated
with that ID. Specifically:

- The NFCProvider Mojo interface is introduced. It supports connecting to
  an NFC instance associated with a given host ID.
- The NFCDelegate.java interface is introduced. This interface supports
  mapping from host IDs to Activity, including tracking when the Activity
  associated with a host ID changes. This interface is passed into the
  Device Service by its embedder.
- NfcImpl.java uses the NFCDelegate to access the Activity that it needs.
- //content provides a ContentNFCDelegate implementation. This implementation
  maps host IDs to WebContents instances via an NfcHost internal helper
  class.
- //content/browser now handles requests to connect to NFC by forwarding
  them on to the Device Service (i.e., NFCProvider), passing the host
  ID associated with the WebContents in question in order to allow the
  NFC implementation to map back to that WebContents via NFCDelegate.

The net sum of the above is that //content's dependence on the NFC
implementation is removed.

Note that in addition to the below-described test, I also tested reparenting as
follows:
- Close Chromium on the device
- On the host, execute:
  adb shell "am start -a 'android.intent.action.VIEW' -n org.chromium.chrome/  
>com.google.android.apps.chrome.Main -d https://colinblundell.github.io/nfc.   
>html --es \"android.support.customtabs.extra.SESSION\" \"bla\""
- On the phone, you should see a custom tab pop up. Push a message.
- Reparent the tab by tapping "Open in Chromium." Verify that the tab was
  reparented, i.e., there was no reload operation.
- Bring the device close to the NFC tag and verify that you see "Message
  pushed" on the webpage.

BUG=680924
TEST=On Chrome on Android, go to about://flags and turn on "Experimental Web
Platform features" and "WebNFC Android". Restart Chrome. Obtain a physical
NFC tag.  Go to https://colinblundell.github.io/nfc.html and verify that the
page says "NFC API found". Enter a message on the webpage and press "Push
message to tag". Bring the phone near the tag and verifiy that the page outputs
"Message pushed." Push "Read data from tag", bring the phone near the tag,
and verify that the webpage outputs the message that you just pushed to the
tag.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
[Device Service] Decouple NFC implementation from //content

This CL ports the NFC implementation from being hosted by //content
to being hosted by the Device Service. This task is challenging because
the NFC implementation needs access to the Activity of the WebContents
that is associated with the requesting render frame. Moreore, that
Activity can change if the WebContents is reparented (e.g., moved from
being in a custom tab to being in Chrome by the user).

To handle these challenges, this CL takes a similar approach to that
taken for WakeLock: information is passed to the Device Service that
allows the NFC implementation to map a host ID to the Activity associated
with that ID. Specifically:

- The NFCProvider Mojo interface is introduced. It supports connecting to
  an NFC instance associated with a given host ID.
- The NFCDelegate.java interface is introduced. This interface supports
  mapping from host IDs to Activity, including tracking when the Activity
  associated with a host ID changes. This interface is passed into the
  Device Service by its embedder.
- NfcImpl.java uses the NFCDelegate to access the Activity that it needs.
- //content provides a ContentNFCDelegate implementation. This implementation
  maps host IDs to WebContents instances via an NfcHost internal helper
  class.
- //content/browser now handles requests to connect to NFC by forwarding
  them on to the Device Service (i.e., NFCProvider), passing the host
  ID associated with the WebContents in question in order to allow the
  NFC implementation to map back to that WebContents via NFCDelegate.

The net sum of the above is that //content's dependence on the NFC
implementation is removed.

Note that in addition to the below-described test, I also tested reparenting as
follows:
- Close Chromium on the device
- On the host, execute:
  adb shell "am start -a 'android.intent.action.VIEW' -n
org.chromium.chrome/com.google.android.apps.chrome.Main -d
https://colinblundell.github.io/nfc.html --es
\"android.support.customtabs.extra.SESSION\" \"bla\""
- On the phone, you should see a custom tab pop up. Push a message.
- Reparent the tab by tapping "Open in Chromium." Verify that the tab was
  reparented, i.e., there was no reload operation.
- Bring the device close to the NFC tag and verify that you see "Message
  pushed" on the webpage.

BUG=680924
TEST=On Chrome on Android, go to about://flags and turn on "Experimental Web
Platform features" and "WebNFC Android". Restart Chrome. Obtain a physical
NFC tag.  Go to https://colinblundell.github.io/nfc.html and verify that the
page says "NFC API found". Enter a message on the webpage and press "Push
message to tag". Bring the phone near the tag and verifiy that the page outputs
"Message pushed." Push "Read data from tag", bring the phone near the tag,
and verify that the webpage outputs the message that you just pushed to the
tag.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[blundell, 2017-05-05 16:12:52]

Description was changed from

==========
[Device Service] Decouple NFC implementation from //content

This CL ports the NFC implementation from being hosted by //content
to being hosted by the Device Service. This task is challenging because
the NFC implementation needs access to the Activity of the WebContents
that is associated with the requesting render frame. Moreore, that
Activity can change if the WebContents is reparented (e.g., moved from
being in a custom tab to being in Chrome by the user).

To handle these challenges, this CL takes a similar approach to that
taken for WakeLock: information is passed to the Device Service that
allows the NFC implementation to map a host ID to the Activity associated
with that ID. Specifically:

- The NFCProvider Mojo interface is introduced. It supports connecting to
  an NFC instance associated with a given host ID.
- The NFCDelegate.java interface is introduced. This interface supports
  mapping from host IDs to Activity, including tracking when the Activity
  associated with a host ID changes. This interface is passed into the
  Device Service by its embedder.
- NfcImpl.java uses the NFCDelegate to access the Activity that it needs.
- //content provides a ContentNFCDelegate implementation. This implementation
  maps host IDs to WebContents instances via an NfcHost internal helper
  class.
- //content/browser now handles requests to connect to NFC by forwarding
  them on to the Device Service (i.e., NFCProvider), passing the host
  ID associated with the WebContents in question in order to allow the
  NFC implementation to map back to that WebContents via NFCDelegate.

The net sum of the above is that //content's dependence on the NFC
implementation is removed.

Note that in addition to the below-described test, I also tested reparenting as
follows:
- Close Chromium on the device
- On the host, execute:
  adb shell "am start -a 'android.intent.action.VIEW' -n
org.chromium.chrome/com.google.android.apps.chrome.Main -d
https://colinblundell.github.io/nfc.html --es
\"android.support.customtabs.extra.SESSION\" \"bla\""
- On the phone, you should see a custom tab pop up. Push a message.
- Reparent the tab by tapping "Open in Chromium." Verify that the tab was
  reparented, i.e., there was no reload operation.
- Bring the device close to the NFC tag and verify that you see "Message
  pushed" on the webpage.

BUG=680924
TEST=On Chrome on Android, go to about://flags and turn on "Experimental Web
Platform features" and "WebNFC Android". Restart Chrome. Obtain a physical
NFC tag.  Go to https://colinblundell.github.io/nfc.html and verify that the
page says "NFC API found". Enter a message on the webpage and press "Push
message to tag". Bring the phone near the tag and verifiy that the page outputs
"Message pushed." Push "Read data from tag", bring the phone near the tag,
and verify that the webpage outputs the message that you just pushed to the
tag.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
[Device Service] Decouple NFC implementation from //content

This CL ports the NFC implementation from being hosted by //content
to being hosted by the Device Service. This task is challenging because
the NFC implementation needs access to the Activity of the WebContents
that is associated with the requesting render frame. Moreore, that
Activity can change if the WebContents is reparented (e.g., moved from
being in a custom tab to being in Chrome by the user).

To handle these challenges, this CL takes a similar approach to that
taken for WakeLock: information is passed to the Device Service that
allows the NFC implementation to map a host ID to the Activity associated
with that ID. Specifically:

- The NFCProvider Mojo interface is introduced. It supports connecting to
  an NFC instance associated with a given host ID.
- The NFCDelegate.java interface is introduced. This interface supports
  mapping from host IDs to Activity, including tracking when the Activity
  associated with a host ID changes. This interface is passed into the
  Device Service by its embedder.
- NfcImpl.java uses the NFCDelegate to access the Activity that it needs.
- //content provides a ContentNFCDelegate implementation. This implementation
  maps host IDs to WebContents instances via an NfcHost internal helper
  class.
- //content/browser now handles requests to connect to NFC by forwarding
  them on to the Device Service (i.e., NFCProvider), passing the host
  ID associated with the WebContents in question in order to allow the
  NFC implementation to map back to that WebContents via NFCDelegate.

The net sum of the above is that //content's dependence on the NFC
implementation is removed.

Note that in addition to the below-described test, I also tested reparenting as
follows:
- Close Chromium on the device
- On the host, execute:
  adb shell "am start -a 'android.intent.action.VIEW' -n
org.chromium.chrome/com.google.android.apps.chrome.Main -d
https://colinblundell.github.io/nfc.html --es
\"android.support.customtabs.extra.SESSION\" \"bla\""
- On the phone, you should see a custom tab pop up. Push a message.
- Reparent the tab by tapping "Open in Chromium." Verify that the tab was
  reparented, i.e., there was no reload operation.
- Bring the device close to the NFC tag and verify that you see "Message
  pushed" on the webpage.

BUG=680924
TEST=On Chrome on Android, go to about://flags and turn on "Experimental Web
Platform features" and "WebNFC Android". Restart Chrome. Obtain a physical
NFC tag.  Go to https://colinblundell.github.io/nfc.html and verify that the
page says "NFC API found". Enter a message on the webpage and press "Push
message to tag". Bring the phone near the tag and verifiy that the page outputs
"Message pushed." Press "Read data from tag", bring the phone near the tag,
and verify that the webpage outputs the message that you just pushed to the
tag.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[blundell, 2017-05-05 16:15:35]

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

[blundell, 2017-05-05 16:15:36]

https://www.youtube.com/watch?v=xJBB7l7cOqc

[Ken Rockot(use gerrit already), 2017-05-08 15:45:07]

I think this looks right to me! LGTM.

https://codereview.chromium.org/2865653002/diff/100001/content/public/android...
File
content/public/android/java/src/org/chromium/content/browser/InterfaceRegistrarImpl.java
(right):

https://codereview.chromium.org/2865653002/diff/100001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/InterfaceRegistrarImpl.java:60:
InterfaceRegistrar.Registry.addWebContentsRegistrar(
can we kill this?

https://codereview.chromium.org/2865653002/diff/100001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/InterfaceRegistrarImpl.java:78:
private static class ContentWebContentsInterfaceRegistrar
and this?

https://codereview.chromium.org/2865653002/diff/100001/device/nfc/nfc_provide...
File device/nfc/nfc_provider.mojom (right):

https://codereview.chromium.org/2865653002/diff/100001/device/nfc/nfc_provide...
device/nfc/nfc_provider.mojom:11: 
nit: no vertical space

[blundell, 2017-05-09 14:46:18]

blundell@chromium.org changed reviewers:
+ dcheng@chromium.org, jam@chromium.org, tedchoc@chromium.org

[blundell, 2017-05-09 14:46:19]

+tedchoc@ for //content/public/android OWNERS
+jam@ for //content OWNERS outside of the java stuff
+dcheng@ for security review on the mojom and manifest changes

Thanks!

https://codereview.chromium.org/2865653002/diff/100001/content/public/android...
File
content/public/android/java/src/org/chromium/content/browser/InterfaceRegistrarImpl.java
(right):

https://codereview.chromium.org/2865653002/diff/100001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/InterfaceRegistrarImpl.java:60:
InterfaceRegistrar.Registry.addWebContentsRegistrar(
On 2017/05/08 15:45:07, Ken Rockot wrote:
> can we kill this?

Done.

https://codereview.chromium.org/2865653002/diff/100001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/InterfaceRegistrarImpl.java:78:
private static class ContentWebContentsInterfaceRegistrar
On 2017/05/08 15:45:07, Ken Rockot wrote:
> and this?

Done.

https://codereview.chromium.org/2865653002/diff/100001/device/nfc/nfc_provide...
File device/nfc/nfc_provider.mojom (right):

https://codereview.chromium.org/2865653002/diff/100001/device/nfc/nfc_provide...
device/nfc/nfc_provider.mojom:11: 
On 2017/05/08 15:45:07, Ken Rockot wrote:
> nit: no vertical space

heh, I don't know why I have this impulse to do this.

[blundell, 2017-05-09 14:46:26]

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

[commit-bot: I haz the power, 2017-05-09 14:46:47]

Dry run: CQ is trying da patch.

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

[jam, 2017-05-09 15:11:17]

lgtm

curious why a bunch of the code isn't behind android ifdefs? I thought NFC is
only there?

https://codereview.chromium.org/2865653002/diff/140001/content/browser/nfc/nf...
File content/browser/nfc/nfc_host.cc (right):

https://codereview.chromium.org/2865653002/diff/140001/content/browser/nfc/nf...
content/browser/nfc/nfc_host.cc:37: DCHECK(connector);
nit: since the pointer is used in the next line, this DCHECK is gratuitous (i.e.
does nothing in release, and in debug the crash is as useful of a signal)

[commit-bot: I haz the power, 2017-05-09 17:39:43]

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

[commit-bot: I haz the power, 2017-05-09 17:39:44]

Dry run: This issue passed the CQ dry run.

[dcheng, 2017-05-10 06:20:18]

mojo LGTM, though I have the same question as rockot@ regarding #ifdefs

https://codereview.chromium.org/2865653002/diff/140001/content/browser/nfc/nf...
File content/browser/nfc/nfc_host.cc (right):

https://codereview.chromium.org/2865653002/diff/140001/content/browser/nfc/nf...
content/browser/nfc/nfc_host.cc:47: nfc_provider_->GetNFCForHost(id_,
std::move(request));
Nit: #include <utility>

[blundell, 2017-05-10 13:15:25]

Thanks for the reviews! Changed so that the NFC code in //content is entirely
Android-only (I originally developed the patch on Android, and when I first
compiled it on Linux, I just did the minimal work possible to get it to
compile).

https://codereview.chromium.org/2865653002/diff/140001/content/browser/nfc/nf...
File content/browser/nfc/nfc_host.cc (right):

https://codereview.chromium.org/2865653002/diff/140001/content/browser/nfc/nf...
content/browser/nfc/nfc_host.cc:37: DCHECK(connector);
On 2017/05/09 15:11:16, jam wrote:
> nit: since the pointer is used in the next line, this DCHECK is gratuitous
(i.e.
> does nothing in release, and in debug the crash is as useful of a signal)

Done.

https://codereview.chromium.org/2865653002/diff/140001/content/browser/nfc/nf...
content/browser/nfc/nfc_host.cc:47: nfc_provider_->GetNFCForHost(id_,
std::move(request));
On 2017/05/10 06:20:18, dcheng wrote:
> Nit: #include <utility>

Done.

[Ted C, 2017-05-10 14:37:41]

https://codereview.chromium.org/2865653002/diff/160001/content/browser/nfc/nf...
File content/browser/nfc/nfc_host.h (right):

https://codereview.chromium.org/2865653002/diff/160001/content/browser/nfc/nf...
content/browser/nfc/nfc_host.h:5: #ifndef CONTENT_BROWSER_NFC_NFC_HOST_H_
In the same vein as the ifdef comments, should this be in
content/browser/android/nfc?  Or called NFCHostAndroid?

Without those, you'd need to look at the BUILD.gn file to see this is android
specific (or read the class level comment :-P)

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
File
content/public/android/java/src/org/chromium/content/browser/ContentNfcDelegate.java
(right):

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/ContentNfcDelegate.java:21:
static ContentNfcDelegate create() {
can this be private?

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/ContentNfcDelegate.java:25:
public void trackActivityForHost(int hostId, Callback<Activity> callback) {
should have an @Override here and below

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/ContentNfcDelegate.java:27:
if (host == null) {
FYI, if the statement and condition all fit on a single line in java, you can do

if (host == null) return;

Although, clang format might be broken at the moment and preventing that.

Is it valid for the host to be null?  should we be asserting that it isn't?

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
File content/public/android/java/src/org/chromium/content/browser/NfcHost.java
(right):

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/NfcHost.java:32:
private static final SparseArray<NfcHost> mContextHostsMap = new
SparseArray<NfcHost>();
non-constant static in java start with a lowercase s instead of m

I would put statics at the top of the file as well.

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/NfcHost.java:46:
NfcHost(WebContents webContents, int contextId) {
you'll need to do:

super(webContents)

Otherwise, the webcontentsobserver#destroy will never be called.

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/NfcHost.java:47:
mCallback = null;
no need for this, it will default to null in java

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/NfcHost.java:48:
mWebContents = webContents;
looks like mWebContents, mContentViewCore, and mContextId can all be final

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/NfcHost.java:54:
/** Called by the NFC implementation (via ContentNfcDelegate) to allow
If you have a /** comment header, it should be on its own line

You can also wrap comment in java to 100 chars

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/NfcHost.java:68: if
(mContentViewCore != null) {
What is the timeline of the NfcHost?  I'm more wondering if the constructor vs
trackActivityChanges will happen far enough apart that we could lazily build the
reference to CVC here.

If CVC is null, NFC really won't work, so I'm wondering if that should be more
explicit (i.e. should the Delegate have a "supportsNfc" or something that this
would essentially check for a null CVC.)  If null isn't valid and we expect that
never to happen for this, we should add an assert for not null in the
constructor for CVC

The problem I see is that if a CVC is ever constructed for the web contents
after the NfcHost is built, then this would never get notified.  I don't really
think that is drastically different than the existing behavior though.

But if we think trackActivityChanges happens much later than the constructor, we
could consider setting mContentViewCore here (and null in stop).

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/NfcHost.java:78:
public void stopTrackingActivityChanges() {
javadoc non-private methods

Although here, I suspect you could so something as simple as:

/**
 * @see org.chromium.device.nfc.NfcDelegate#stopTrackingActivityForHost(int)
 */

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/NfcHost.java:88:
mCallback = null;
maybe replace these three following lines with stopTrackingActivityChanges?

[blundell, 2017-05-11 15:13:52]

Thanks, Ted!

Question back to you in the comments about lifetime relationship between
WebContents and ContentViewCore. I'm wondering if I need to be fully general
about the possibility of there being a WebContents instance with no CVC instance
at any given point, or if there are some assumptions I can make to simplify
things.

https://codereview.chromium.org/2865653002/diff/160001/content/browser/nfc/nf...
File content/browser/nfc/nfc_host.h (right):

https://codereview.chromium.org/2865653002/diff/160001/content/browser/nfc/nf...
content/browser/nfc/nfc_host.h:5: #ifndef CONTENT_BROWSER_NFC_NFC_HOST_H_
On 2017/05/10 14:37:40, Ted C wrote:
> In the same vein as the ifdef comments, should this be in
> content/browser/android/nfc?  Or called NFCHostAndroid?
> 
> Without those, you'd need to look at the BUILD.gn file to see this is android
> specific (or read the class level comment :-P)

Moved to //content/browser/android. Didn't put in a subdir there as there are no
others for native code.

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
File
content/public/android/java/src/org/chromium/content/browser/ContentNfcDelegate.java
(right):

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/ContentNfcDelegate.java:21:
static ContentNfcDelegate create() {
On 2017/05/10 14:37:40, Ted C wrote:
> can this be private?

Done.

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/ContentNfcDelegate.java:27:
if (host == null) {
On 2017/05/10 14:37:41, Ted C wrote:
> FYI, if the statement and condition all fit on a single line in java, you can
do
> 
> if (host == null) return;
> 
> Although, clang format might be broken at the moment and preventing that.
> 
> Is it valid for the host to be null?  should we be asserting that it isn't?

It is valid for the host to be null: a request might come in from the NFCImpl
that races with the WebContents being torn down, for example.

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
File content/public/android/java/src/org/chromium/content/browser/NfcHost.java
(right):

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/NfcHost.java:32:
private static final SparseArray<NfcHost> mContextHostsMap = new
SparseArray<NfcHost>();
On 2017/05/10 14:37:41, Ted C wrote:
> non-constant static in java start with a lowercase s instead of m
> 
> I would put statics at the top of the file as well.

Done.

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/NfcHost.java:46:
NfcHost(WebContents webContents, int contextId) {
On 2017/05/10 14:37:41, Ted C wrote:
> you'll need to do:
> 
> super(webContents)
> 
> Otherwise, the webcontentsobserver#destroy will never be called.

Done.

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/NfcHost.java:47:
mCallback = null;
On 2017/05/10 14:37:41, Ted C wrote:
> no need for this, it will default to null in java

Done.

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/NfcHost.java:48:
mWebContents = webContents;
On 2017/05/10 14:37:41, Ted C wrote:
> looks like mWebContents, mContentViewCore, and mContextId can all be final

Done.

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/NfcHost.java:54:
/** Called by the NFC implementation (via ContentNfcDelegate) to allow
On 2017/05/10 14:37:41, Ted C wrote:
> If you have a /** comment header, it should be on its own line
>

Done.
 
> You can also wrap comment in java to 100 chars

Thanks, working on getting my vim configuration to wrap java at 100 chars and
then will rewrap the comments here.

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/NfcHost.java:68: if
(mContentViewCore != null) {
On 2017/05/10 14:37:41, Ted C wrote:
> What is the timeline of the NfcHost?  I'm more wondering if the constructor vs
> trackActivityChanges will happen far enough apart that we could lazily build
the
> reference to CVC here.
> 
> If CVC is null, NFC really won't work, so I'm wondering if that should be more
> explicit (i.e. should the Delegate have a "supportsNfc" or something that this
> would essentially check for a null CVC.)  If null isn't valid and we expect
that
> never to happen for this, we should add an assert for not null in the
> constructor for CVC
> 
> The problem I see is that if a CVC is ever constructed for the web contents
> after the NfcHost is built, then this would never get notified.  I don't
really
> think that is drastically different than the existing behavior though.
> 
> But if we think trackActivityChanges happens much later than the constructor,
we
> could consider setting mContentViewCore here (and null in stop).

NfcHost is constructed in response to a request to connect to NFC from the
renderer. If the CVC isn't constructed at this point, I don't see a reason that
it would necessarily be created when the NfcHost subsequently receives the
request from the NFC impl to track Activity changes (that request is sent by
NfcImpl on its creation). 

Note that in the codebase as it currently stands, the NFC factory checks whether
the CVC is null on receiving a NFC connection request from the renderer and
drops the request if so. I didn't preserve this behavior, as you noted. What is
the relationship between CVC and WebContents on Android? i.e., in what cases can
there not be a CVC for a WebContents instance? It would be nice to not make this
code more complex than it has to be.

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/NfcHost.java:78:
public void stopTrackingActivityChanges() {
On 2017/05/10 14:37:41, Ted C wrote:
> javadoc non-private methods
> 
> Although here, I suspect you could so something as simple as:
> 
> /**
>  * @see org.chromium.device.nfc.NfcDelegate#stopTrackingActivityForHost(int)
>  */ 

Done.

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/NfcHost.java:88:
mCallback = null;
On 2017/05/10 14:37:41, Ted C wrote:
> maybe replace these three following lines with stopTrackingActivityChanges?

Done.

[blundell, 2017-05-11 15:14:01]

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

[commit-bot: I haz the power, 2017-05-11 15:14:23]

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-11 16:24:19]

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

[commit-bot: I haz the power, 2017-05-11 16:24:20]

Dry run: This issue passed the CQ dry run.

[Ted C, 2017-05-11 16:36:49]

lgtm

There are cases where no CVC is present, but I suspect NFC as a feature would
not be executed in any of them.  If we need a null check, I would move it back
earlier and have these classes assume non-null as you've moved towards.

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
File content/public/android/java/src/org/chromium/content/browser/NfcHost.java
(right):

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/NfcHost.java:68: if
(mContentViewCore != null) {
On 2017/05/11 15:13:52, blundell wrote:
> On 2017/05/10 14:37:41, Ted C wrote:
> > What is the timeline of the NfcHost?  I'm more wondering if the constructor
vs
> > trackActivityChanges will happen far enough apart that we could lazily build
> the
> > reference to CVC here.
> > 
> > If CVC is null, NFC really won't work, so I'm wondering if that should be
more
> > explicit (i.e. should the Delegate have a "supportsNfc" or something that
this
> > would essentially check for a null CVC.)  If null isn't valid and we expect
> that
> > never to happen for this, we should add an assert for not null in the
> > constructor for CVC
> > 
> > The problem I see is that if a CVC is ever constructed for the web contents
> > after the NfcHost is built, then this would never get notified.  I don't
> really
> > think that is drastically different than the existing behavior though.
> > 
> > But if we think trackActivityChanges happens much later than the
constructor,
> we
> > could consider setting mContentViewCore here (and null in stop).
> 
> NfcHost is constructed in response to a request to connect to NFC from the
> renderer. If the CVC isn't constructed at this point, I don't see a reason
that
> it would necessarily be created when the NfcHost subsequently receives the
> request from the NFC impl to track Activity changes (that request is sent by
> NfcImpl on its creation). 
> 
> Note that in the codebase as it currently stands, the NFC factory checks
whether
> the CVC is null on receiving a NFC connection request from the renderer and
> drops the request if so. I didn't preserve this behavior, as you noted. What
is
> the relationship between CVC and WebContents on Android? i.e., in what cases
can
> there not be a CVC for a WebContents instance? It would be nice to not make
this
> code more complex than it has to be.

CVC has historically been the glue between the webcontents and the android view
system (although slowly changing to be closer to other platforms).  Cases like
pre-render wouldn't have CVC.

Any time a webcontents is created by the renderer, a CVC would only be created
once the browser side is aware of it.

I know there are people investigating other background loading options on
Android,
and I'm not sure if they will have a CVC or not.  Is NFC gated by any user
interaction?  If so, then I think it would be safe to assume a CVC (in general,
I
think NFC could probably get away with assuming CVC regardless).

If we have the assert above that it is non-null, then let's remove the null
check
here and crash so we can address it if needed.

https://codereview.chromium.org/2865653002/diff/200001/content/public/android...
File
content/public/android/java/src/org/chromium/content/browser/ContentNfcDelegate.java
(right):

https://codereview.chromium.org/2865653002/diff/200001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/ContentNfcDelegate.java:28:
@Override
FYI, @Override inherits the javadoc of what it overrides, so you don't need to
add anymore unless it is helpful.  Not a critique of these comments, just to let
you know.

https://codereview.chromium.org/2865653002/diff/200001/content/public/android...
File content/public/android/java/src/org/chromium/content/browser/NfcHost.java
(right):

https://codereview.chromium.org/2865653002/diff/200001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/NfcHost.java:32:
private static final SparseArray<NfcHost> sContextHostsMap = new
SparseArray<NfcHost>();
I think the other part of my comment got lost, but I would order the variables
as:

static finals
statics mutables

class finals
class mutables

https://codereview.chromium.org/2865653002/diff/200001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/NfcHost.java:86: if
(mContentViewCore != null) {
same, let's drop the null check here.

[blundell, 2017-05-12 10:55:18]

Patchset #12 (id:220001) has been deleted

[blundell, 2017-05-12 10:58:21]

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:58:36]

Dry run: CQ is trying da patch.

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

[blundell, 2017-05-12 11:01:15]

Thanks! All comments addressed, and comments rewrapped.

I discovered a somewhat subtle issue with how Blink is handling NFC requests
from iframes, which I need to resolve before landing this CL
(https://codereview.chromium.org/2875283002/).

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
File content/public/android/java/src/org/chromium/content/browser/NfcHost.java
(right):

https://codereview.chromium.org/2865653002/diff/160001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/NfcHost.java:68: if
(mContentViewCore != null) {
On 2017/05/11 16:36:49, Ted C wrote:
> On 2017/05/11 15:13:52, blundell wrote:
> > On 2017/05/10 14:37:41, Ted C wrote:
> > > What is the timeline of the NfcHost?  I'm more wondering if the
constructor
> vs
> > > trackActivityChanges will happen far enough apart that we could lazily
build
> > the
> > > reference to CVC here.
> > > 
> > > If CVC is null, NFC really won't work, so I'm wondering if that should be
> more
> > > explicit (i.e. should the Delegate have a "supportsNfc" or something that
> this
> > > would essentially check for a null CVC.)  If null isn't valid and we
expect
> > that
> > > never to happen for this, we should add an assert for not null in the
> > > constructor for CVC
> > > 
> > > The problem I see is that if a CVC is ever constructed for the web
contents
> > > after the NfcHost is built, then this would never get notified.  I don't
> > really
> > > think that is drastically different than the existing behavior though.
> > > 
> > > But if we think trackActivityChanges happens much later than the
> constructor,
> > we
> > > could consider setting mContentViewCore here (and null in stop).
> > 
> > NfcHost is constructed in response to a request to connect to NFC from the
> > renderer. If the CVC isn't constructed at this point, I don't see a reason
> that
> > it would necessarily be created when the NfcHost subsequently receives the
> > request from the NFC impl to track Activity changes (that request is sent by
> > NfcImpl on its creation). 
> > 
> > Note that in the codebase as it currently stands, the NFC factory checks
> whether
> > the CVC is null on receiving a NFC connection request from the renderer and
> > drops the request if so. I didn't preserve this behavior, as you noted. What
> is
> > the relationship between CVC and WebContents on Android? i.e., in what cases
> can
> > there not be a CVC for a WebContents instance? It would be nice to not make
> this
> > code more complex than it has to be.
> 
> CVC has historically been the glue between the webcontents and the android
view
> system (although slowly changing to be closer to other platforms).  Cases like
> pre-render wouldn't have CVC.
> 
> Any time a webcontents is created by the renderer, a CVC would only be created
> once the browser side is aware of it.
> 
> I know there are people investigating other background loading options on
> Android,
> and I'm not sure if they will have a CVC or not.  Is NFC gated by any user
> interaction?  If so, then I think it would be safe to assume a CVC (in
general,
> I
> think NFC could probably get away with assuming CVC regardless).
> 
> If we have the assert above that it is non-null, then let's remove the null
> check
> here and crash so we can address it if needed.

Makes sense to me. I added a comment explaining the current assumption and that
we would handle the case where there is no CVC by simply dropping the request if
that case ever comes to pass.

It looks like by the spec (and enforced by Blink) NFC is only allowed to be used
when the page is visible, which implies to me based on your explanation that
there *will* always be a CVC.

https://codereview.chromium.org/2865653002/diff/200001/content/public/android...
File
content/public/android/java/src/org/chromium/content/browser/ContentNfcDelegate.java
(right):

https://codereview.chromium.org/2865653002/diff/200001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/ContentNfcDelegate.java:28:
@Override
On 2017/05/11 16:36:49, Ted C wrote:
> FYI, @Override inherits the javadoc of what it overrides, so you don't need to
> add anymore unless it is helpful.  Not a critique of these comments, just to
let
> you know.

Ah nice, I don't think these comments are needed then.

https://codereview.chromium.org/2865653002/diff/200001/content/public/android...
File content/public/android/java/src/org/chromium/content/browser/NfcHost.java
(right):

https://codereview.chromium.org/2865653002/diff/200001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/NfcHost.java:32:
private static final SparseArray<NfcHost> sContextHostsMap = new
SparseArray<NfcHost>();
On 2017/05/11 16:36:49, Ted C wrote:
> I think the other part of my comment got lost, but I would order the variables
> as:
> 
> static finals
> statics mutables
> 
> class finals
> class mutables

Done.

https://codereview.chromium.org/2865653002/diff/200001/content/public/android...
content/public/android/java/src/org/chromium/content/browser/NfcHost.java:86: if
(mContentViewCore != null) {
On 2017/05/11 16:36:49, Ted C wrote:
> same, let's drop the null check here.

Done.

[commit-bot: I haz the power, 2017-05-12 12:19:45]

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

[commit-bot: I haz the power, 2017-05-12 12:19:46]

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-15 07:32:18]

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

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

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-15 09:10:37]

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

[commit-bot: I haz the power, 2017-05-15 09:10:39]

Dry run: This issue passed the CQ dry run.

[blundell, 2017-05-15 11:43:25]

The CQ bit was checked by blundell@chromium.org

[blundell, 2017-05-15 11:43:26]

The patchset sent to the CQ was uploaded after l-g-t-m from rockot@chromium.org,
jam@chromium.org, dcheng@chromium.org, tedchoc@chromium.org
Link to the patchset: https://codereview.chromium.org/2865653002/#ps260001
(title: "Rebase")

[commit-bot: I haz the power, 2017-05-15 11:43:43]

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 11:49:08]

CQ is committing da patch.

Bot data: {"patchset_id": 260001, "attempt_start_ts": 1494848605469830,
"parent_rev": "d0458b1c5e117db5cb8b63688ef39af43fd4fc97", "commit_rev":
"f5316fccc7bac30a357a1254aef8dc4a605bc045"}

[commit-bot: I haz the power, 2017-05-15 11:49:26]

Description was changed from

==========
[Device Service] Decouple NFC implementation from //content

This CL ports the NFC implementation from being hosted by //content
to being hosted by the Device Service. This task is challenging because
the NFC implementation needs access to the Activity of the WebContents
that is associated with the requesting render frame. Moreore, that
Activity can change if the WebContents is reparented (e.g., moved from
being in a custom tab to being in Chrome by the user).

To handle these challenges, this CL takes a similar approach to that
taken for WakeLock: information is passed to the Device Service that
allows the NFC implementation to map a host ID to the Activity associated
with that ID. Specifically:

- The NFCProvider Mojo interface is introduced. It supports connecting to
  an NFC instance associated with a given host ID.
- The NFCDelegate.java interface is introduced. This interface supports
  mapping from host IDs to Activity, including tracking when the Activity
  associated with a host ID changes. This interface is passed into the
  Device Service by its embedder.
- NfcImpl.java uses the NFCDelegate to access the Activity that it needs.
- //content provides a ContentNFCDelegate implementation. This implementation
  maps host IDs to WebContents instances via an NfcHost internal helper
  class.
- //content/browser now handles requests to connect to NFC by forwarding
  them on to the Device Service (i.e., NFCProvider), passing the host
  ID associated with the WebContents in question in order to allow the
  NFC implementation to map back to that WebContents via NFCDelegate.

The net sum of the above is that //content's dependence on the NFC
implementation is removed.

Note that in addition to the below-described test, I also tested reparenting as
follows:
- Close Chromium on the device
- On the host, execute:
  adb shell "am start -a 'android.intent.action.VIEW' -n
org.chromium.chrome/com.google.android.apps.chrome.Main -d
https://colinblundell.github.io/nfc.html --es
\"android.support.customtabs.extra.SESSION\" \"bla\""
- On the phone, you should see a custom tab pop up. Push a message.
- Reparent the tab by tapping "Open in Chromium." Verify that the tab was
  reparented, i.e., there was no reload operation.
- Bring the device close to the NFC tag and verify that you see "Message
  pushed" on the webpage.

BUG=680924
TEST=On Chrome on Android, go to about://flags and turn on "Experimental Web
Platform features" and "WebNFC Android". Restart Chrome. Obtain a physical
NFC tag.  Go to https://colinblundell.github.io/nfc.html and verify that the
page says "NFC API found". Enter a message on the webpage and press "Push
message to tag". Bring the phone near the tag and verifiy that the page outputs
"Message pushed." Press "Read data from tag", bring the phone near the tag,
and verify that the webpage outputs the message that you just pushed to the
tag.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
[Device Service] Decouple NFC implementation from //content

This CL ports the NFC implementation from being hosted by //content
to being hosted by the Device Service. This task is challenging because
the NFC implementation needs access to the Activity of the WebContents
that is associated with the requesting render frame. Moreore, that
Activity can change if the WebContents is reparented (e.g., moved from
being in a custom tab to being in Chrome by the user).

To handle these challenges, this CL takes a similar approach to that
taken for WakeLock: information is passed to the Device Service that
allows the NFC implementation to map a host ID to the Activity associated
with that ID. Specifically:

- The NFCProvider Mojo interface is introduced. It supports connecting to
  an NFC instance associated with a given host ID.
- The NFCDelegate.java interface is introduced. This interface supports
  mapping from host IDs to Activity, including tracking when the Activity
  associated with a host ID changes. This interface is passed into the
  Device Service by its embedder.
- NfcImpl.java uses the NFCDelegate to access the Activity that it needs.
- //content provides a ContentNFCDelegate implementation. This implementation
  maps host IDs to WebContents instances via an NfcHost internal helper
  class.
- //content/browser now handles requests to connect to NFC by forwarding
  them on to the Device Service (i.e., NFCProvider), passing the host
  ID associated with the WebContents in question in order to allow the
  NFC implementation to map back to that WebContents via NFCDelegate.

The net sum of the above is that //content's dependence on the NFC
implementation is removed.

Note that in addition to the below-described test, I also tested reparenting as
follows:
- Close Chromium on the device
- On the host, execute:
  adb shell "am start -a 'android.intent.action.VIEW' -n
org.chromium.chrome/com.google.android.apps.chrome.Main -d
https://colinblundell.github.io/nfc.html --es
\"android.support.customtabs.extra.SESSION\" \"bla\""
- On the phone, you should see a custom tab pop up. Push a message.
- Reparent the tab by tapping "Open in Chromium." Verify that the tab was
  reparented, i.e., there was no reload operation.
- Bring the device close to the NFC tag and verify that you see "Message
  pushed" on the webpage.

BUG=680924
TEST=On Chrome on Android, go to about://flags and turn on "Experimental Web
Platform features" and "WebNFC Android". Restart Chrome. Obtain a physical
NFC tag.  Go to https://colinblundell.github.io/nfc.html and verify that the
page says "NFC API found". Enter a message on the webpage and press "Push
message to tag". Bring the phone near the tag and verifiy that the page outputs
"Message pushed." Press "Read data from tag", bring the phone near the tag,
and verify that the webpage outputs the message that you just pushed to the
tag.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[commit-bot: I haz the power, 2017-05-15 11:49:27]

Committed patchset #13 (id:260001) as
https://chromium.googlesource.com/chromium/src/+/f5316fccc7bac30a357a1254aef8...