860910 - [B2G][Gaia][NFC] NFC Manager

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Integration of gaia demo app and system call for activities for NFC

Comment 1

[Garner Lee]

Attachment: Patch 1 (v1) - NFC Demo and manifests

Demo code for NFC. Read/write/P2P NDEF tags, and manifest activities support.

Comment 2

[Garner Lee]

Attachment: Patch 2 (v1) - NFC Demo JQuery resources

Comment 3

[Garner Lee]

Attachment: Patch 3 (v1) - NFC System app integration: activities and messages

Modifies System app to redirect messages to registered activities in application manifests.

Comment 4

[Garner Lee]

Attachment: Patch 4 (v1) - Changes to Browser, dailer, and email manifests to accept NFC activitiy events

Proposed, and starting point, changes to other certified apps to allow basic NFC activity support for dialer, contacts, and browser URL.

Comment 5

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

I'm not qualified to review any of this. We need to find someone on the gaia team to take a look at it, and probably after bug 860906 lands since there may be changes in that which change this.

Comment 6

[Garner Lee]

Attachment: Patch 1 (v2) - NFC Demo and manifests

Update for dom name change: MozNdefRecord --> NdefRecord

Comment 7

[Garner Lee]

Attachment: Patch 1 (v3) - NFC Demo and manifests

Update for NfcEvent field name change to "message".

Comment 8

[Garner Lee]

Attachment: Patch 1 (v4) - NFC Demo and manifests

Rebased on top of new mozillaorg master.

Comment 9

[Garner Lee]

Attachment: Patch 2 (v2) - NFC Demo JQuery resources

Comment 10

[Garner Lee]

Attachment: Patch 3 (v2) - NFC System app integration: activities and messages

Comment 11

[Garner Lee]

Attachment: Patch 4 (v2) - Changes to Browser, dialer, and email manifests to accept NFC activity events.

Comment 12

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Just had a very quick skim on this - the demo makes heavy use of web activities which worries me (noting that any web content, including web pages can initiate web activities). Maybe this is a just a demo though and we don't care so much about security though ? I was expecting that eventually there would be a separate NFC app that has the NFC permission and listens to the appropriate system messages - is the the plan, or are we planing to keep NFC in the system app?

Comment 13

[Daniel Veditz [:dveditz]]

This is a different logic layer than covered by a security review of "WebNFC" (bug 674741, bug 749325) and needs a separate sec-review (probably smallish).

Comment 14

[Paul Theriault [:pauljt] (no longer reading bugmail)]

I changed the secreview(bug 749325) to block the parent of 674741, which is actually the meta bug that includes this bug and many other NFC bugs which all need at least some review I think. So removing this flag since it will be covered by the existing secreview.

Comment 15

[Garner Lee]

Hi,

I note Contacts should be one of the user stories that should be suppported. In the System App/nfc.js patch code, I have this:

+  // FIXME, incomplete mapping/formatting. App (or full featured vcard parser
+  // library should parse the full ndef vcard.
+  // VCARD 2.1:
+  function handleVCardRecord(record) {

This suggests some bit of architecure: Will there be a generally available VCARD parser library, service, xpcom component, etc. between contacts, and the system nfc manager? It would seem there should be a already vetted VCARD parser already. If so, we can get rid of the FIXME with common code.

Comment 16

[Garner Lee]

(In reply to Paul Theriault [:pauljt] from comment #12)
> Just had a very quick skim on this - the demo makes heavy use of web
> activities which worries me (noting that any web content, including web
> pages can initiate web activities). Maybe this is a just a demo though and
> we don't care so much about security though ? I was expecting that
> eventually there would be a separate NFC app that has the NFC permission and
> listens to the appropriate system messages - is the the plan, or are we
> planing to keep NFC in the system app?

nfc-demo is currently sort of intended to be a catch all for registered nfc-ndef-discovered messages, it probably shouldn't ship, at least, not as nfc-demo. It's currently the only app that can write tags however, so it's not quite purely a demo app anymore. The system app can deliver VCARDs and URLs to Contacts and Browser. If more than one is registered, then the user needs to select one.

From the security perspective, app/system/nfc.js (or soon: nfc_manager.js), will gain "nfc-manager" permissions to process techdiscovery/techlost messages from the NFC deamon, and we'll remove techdiscovery/lost from the DOM eventually.

The current plans are: From there, it routes the message via web activities to the apps (popping up a message to choose the app). From that perspecitve, it's just launching any application registered for that type, similar to how Android Intents work. The selected app then can use DOM calls (connect/disconnect/read/write/push/details). 

Applications themselves will require a "connect(mozNfc.NDEF)", to ensure a 1 to 1 connection to hardware (at least, until you switch foreground NFC apps. That needs some discussion from both UX and security: how long is an app allowed to stay connected, without disconnection, locking a resource). Currently, certified apps are needed, but that will change.

Comment 17

[Paul Theriault [:pauljt] (no longer reading bugmail)]

> The current plans are: From there, it routes the message via web activities
> to the apps (popping up a message to choose the app). From that perspecitve,
> it's just launching any application registered for that type, similar to how
> Android Intents work. The selected app then can use DOM calls
> (connect/disconnect/read/write/push/details). 

This is what sounds dangerous to me, and also what I am really having trouble following. Unlike Android Intents, Web Activities can not be explicitly routed to a given application - the user chooses. Furthermore, if there is only one suitable application registered to handle a specific web activity action/filter combination, it will be chosen without prompting the user at all.

Maybe this has been thought of, but what prevents the following threats:

- A web page starting one of the web activities sent by the system app? (is it up to the receiving app to verify the origin of the web activity initiator?)

- A web app registering to handle one of the activities used (nfc-ndef-discovered, nfc-write-request-status-disabled, nfc-ndefdisconnected-disabled) since any web app can register to handle any web activity

Does this design take into account these threats?

Comment 18

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Note, since you asked in email - an app can currently only register themselves as handling a specific activity by declaring them in the manifest. Handling web activities in regular web pages is not yet supported, although this was discussed as a possibility. See https://developer.mozilla.org/en-US/docs/WebAPI/Web_Activities#Register_an_activity and also bug 775181. 

The main concern I have from a security perspective is that any hosted app could handle an activity (i.e. self installed, not reviewed by marketplace).

I don't you can register wildcard (*) for the activity name, but filters support wildcards I think.

Comment 19

[(inactive after 6/18) Alive Kuo [:alive]]

Hi, I do have some concern about launching app via activity.
In the future we should let each app be able to use NFC,
but we definitely don't want to launch each app by activity hardcoded in system app every we have a new app.

I expect that NFC filter is done in gecko layer,
and use system message with a specific parameter like web activity to tell window manager to open the app.

With current implementation it's impossible to do https://bugzilla.mozilla.org/show_bug.cgi?id=920882

Comment 20

[Ken Chang[:kenkai]]

Hi Garner, any feedback?

Comment 21

[(inactive after 6/18) Alive Kuo [:alive]]

I discussed this with Fabrice in mozilla summit.
We both agree we should move the logic into gecko but it's uncertain how to do that now.

Fabrice said he would cowork with you guys to resolve gecko part.

Comment 22

[Garner Lee]

We actually have a meeting with Fabrice on Activities today on limitations and solutions, will update.

Comment 23

[Garner Lee]

Ahead of the meeting, the system app (via system/js/nfc_manager.js) routes the messages to different apps based on filtering. Is security inherently weaker in the system app (gaia), than through the parent gecko/dom app process?

Comment 24

[(inactive after 6/18) Alive Kuo [:alive]]

What matters is not security issue.

Comment 25

[Garner Lee]

Attachment: Patch (v5) NFC Demo and manifests (no JQuery)

Comment 26

[Garner Lee]

Attachment: Patch (v3) NFC System app integration: activities and messages, airplane mode.

NFC Manager routes messages from the parent gecko/gonk/nfcd processes.

Comment 27

[Ken Chang[:kenkai]]

We will use this bug for NFC manager in Gaia.

Comment 28

[Garner Lee]

Attachment: Patch (v4) NFC System app integration: activities and messages, airplane mode.

Merged 11/4 code. Code and manifest changes. Initail reviewer: Assigning to Alive.

Comment 29

[Ken Chang[:kenkai]]

Garner, should we remove those demonstrating patches from this bug?

Comment 30

[Garner Lee]

Demo/Example code removed.

Comment 31

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 827569 [details] [diff] [review]
Patch (v4) NFC System app integration: activities and messages,  airplane mode.

Review of attachment 827569 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry, the readability, quality and testability of nfc manager is really low.
Please rewrite it into object literal instead of module pattern.

See other module in system app for hint.

I don't really know what you do to mediadb, pass review to David.

::: apps/system/js/nfc_manager.js
@@ +6,5 @@
> + * License, v. 2.0. If a copy of the MPL was not distributed with this file,
> + * You can obtain one at http://mozilla.org/MPL/2.0/.
> + */
> +
> +/* Copyright © 2013, Deutsche Telekom, Inc. */

Please remove this line.

@@ +11,5 @@
> +
> +'use strict';
> +
> +(function() {
> +  var DEBUG = true;

nit: Do you mean to enable DEBUG?

@@ +61,5 @@
> +    RFUAction: 0x03,  // Reserved from 0x03 to 0xFF
> +
> +    uris: new Array(),
> +
> +    init: function() {

give function a name

@@ +137,5 @@
> +    if (acceptEvents) {
> +      powerLevel = NFC_POWER_LEVEL_ENABLED;
> +    }
> +    var request = navigator.mozSettings.createLock().set({
> +      'nfc.powerlevel': powerLevel

What's this for?

@@ +154,5 @@
> +  function handleEvent(evt) {
> +    debug('XXXXXXXXX Handle Event evt.type' + evt.type);
> +    switch (evt.type) {
> +      case 'screenchange':
> +        screenEnabled = evt.detail.screenEnabled;

Don't need to record this.

@@ +271,5 @@
> +   * Tags, and fallback tag handling.
> +   */
> +  function acceptNfcEvents() {
> +    // Policy:
> +    if (screenEnabled && !screenLocked) {

ScreenManager.screenEnabled && !LockScreen.locked

@@ +438,5 @@
> +
> +  function handleTechLost(command) {
> +    debug('Technology Lost: ' + JSON.stringify(command));
> +    var a = new MozActivity({
> +      name: 'nfc-tech-lost',

Why do we invoke an activity here????

Comment 32

[Kai-Chih Hu [:khu]]

Hi, do we have a clear view on the target milestone on this bug now? Thanks.

Comment 33

[David Flanagan [:djf]]

Comment on attachment 827569 [details] [diff] [review]
Patch (v4) NFC System app integration: activities and messages,  airplane mode.

Review of attachment 827569 [details] [diff] [review]:
-----------------------------------------------------------------

::: shared/js/mediadb.js
@@ +1309,5 @@
> +    };
> +
> +    enumerateNextStorage();
> +    return cursor;
> +  }

This code doesn't seem to have anything to do with the NFC patch.  This is old code that has been removed from mediadb.js, I think. It should not be here.

I suspect that this is unintentional and the patch just needs to be rebased.

Comment 34

[Garner Lee]

That appears to be the case. Yes, I'll rebase. I wanted to get the code up somewhat early for people to comment on first, so it was more or less a first pass.

The NFC DOM itself has been modified quite a bit, with nfc_manager along for the ride, so I'll clean it up.

(In reply to David Flanagan [:djf] from comment #33)
> Comment on attachment 827569 [details] [diff] [review]
> Patch (v4) NFC System app integration: activities and messages,  airplane
> mode.
> 
> Review of attachment 827569 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: shared/js/mediadb.js
> @@ +1309,5 @@
> > +    };
> > +
> > +    enumerateNextStorage();
> > +    return cursor;
> > +  }
> 
> This code doesn't seem to have anything to do with the NFC patch.  This is
> old code that has been removed from mediadb.js, I think. It should not be
> here.
> 
> I suspect that this is unintentional and the patch just needs to be rebased.

Comment 35

[Ken Chang[:kenkai]]

It seems better to finish this bug before sprint5. If you think it isn't reasonable, please update target milestone.

Comment 37

[Garner Lee]

Attachment: 0001-Patch-v5-NFC-System-app-integration-activies-and-mes.patch

WIP. Implmented most changes. Mainly convert nfc_manager.js to class based impl.

Added apache header. TechLost is currently used by nfc-demo, though we can still decide to remove it. Powerlevel is still needed while new settings values are added to properly manage the power on nfcd (User UI switch changes preference, and nfc manager goes and makes it effective on NFCD).

Comment 38

[Garner Lee]

Comment on attachment 827569 [details] [diff] [review]
Patch (v4) NFC System app integration: activities and messages,  airplane mode.

Obsoleting. Patch v4 is still usable in case someone is using it. The new patch requires nfc_manager to use Uint8Array, and a matching NFC DOM implementation.

Comment 39

[Garner Lee]

Comment on attachment 832008 [details] [diff] [review]
0001-Patch-v5-NFC-System-app-integration-activies-and-mes.patch

Changing feedback to review request, as per email.

Comment 40

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 832008 [details] [diff] [review]
0001-Patch-v5-NFC-System-app-integration-activies-and-mes.patch

Review of attachment 832008 [details] [diff] [review]:
-----------------------------------------------------------------

Basically not r- but pay attention to the comments.

BTW we still have something to do, please file followup bugs.
For example, it doesn't make sense to use settings API to tell gecko the so-called power level.

::: apps/system/js/nfc_manager.js
@@ +157,5 @@
> +    this._debug('Initializing NFC Message');
> +    var self = this;
> +    window.navigator.mozSetMessageHandler(
> +      'nfc-manager-tech-discovered',
> +      function callTechnologyDiscovered(message) {

this.handleTechnologyDiscovered.bind(this)

@@ +162,5 @@
> +         self.handleTechnologyDiscovered(message);
> +      });
> +    window.navigator.mozSetMessageHandler(
> +      'nfc-manager-tech-lost',
> +      function callHandleTechnologyLost(message) {

this.self.handleTechLost.bind(this)

@@ +551,5 @@
> +         / "NOTE" / "PRODID" / "REV" / "SOUND" / "UID" / "CLIENTPIDMAP"
> +         / "URL" / "KEY" / "FBURL" / "CALADRURI" / "CALURI" / "XML"
> +         / iana-token / x-name
> +     */
> +    var fn = findKey('FN:', payload);

I think think this method call works.

@@ +646,5 @@
> +
> +  handleSmartPosterAction: function nm_nm_handleSmartPosterAction(record) {
> +    // The recommended action has an application specific meaning:
> +    var smartaction = record.payload[0];
> +    var activityText = {

Why not put all these activityText inside an object and access it via specific key?

Comment 42

[Garner Lee]

I'll make the fixes and updates.

Concerning powerlevel, see Bug 933184 (we'll fix it there):
There's current plans to consider the W3C version of the DOM (and the related security questions), or just send an event down to Nfc.js in system/gonk. That particular fix is needed to fix a race condition on managing startup and power states on the nfc hardware.

Comment 43

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 832008 [details] [diff] [review]
0001-Patch-v5-NFC-System-app-integration-activies-and-mes.patch

Review of attachment 832008 [details] [diff] [review]:
-----------------------------------------------------------------

Changed my mind since I think I need to be careful to avoid we have something nobody really understand "why does this work" or "why do we have something doesn't really work but in the code".

::: apps/system/js/nfc_manager.js
@@ +186,5 @@
> +    if (acceptEvents) {
> +      powerLevel = this.NFC_POWER_LEVEL_ENABLED;
> +    }
> +    var request = navigator.mozSettings.createLock().set({
> +      'nfc.powerlevel': powerLevel

Send a mozContentEvent to shell.js and notify nfcd to stop instead of polluting settings API.

@@ +208,5 @@
> +  },
> +
> +  // An NDEF Message is an array of one or more NDEF records.
> +  handleNdefMessages: function nm_handleNdefMessages(ndefmessages) {
> +    var action = new Array();

actions

@@ +297,5 @@
> +
> +    this._debug('handleNdefDiscovered: ' + JSON.stringify(ndefMsg));
> +    var records = ndefMsg;
> +    this._debug('handleNdefDiscovered: ' + JSON.stringify(records));
> +    var action = this.handleNdefMessages(records);

actions

@@ +315,5 @@
> +  // TODO:
> +  handleNdefFormattableDiscovered:
> +    function nm_handleNdefFormattableDiscovered(tech, session) {
> +
> +    return this.handleNdefDiscoveredNotification(tech, session);

one more indent

@@ +316,5 @@
> +  handleNdefFormattableDiscovered:
> +    function nm_handleNdefFormattableDiscovered(tech, session) {
> +
> +    return this.handleNdefDiscoveredNotification(tech, session);
> +  },

one more indent

```js
var A = {
  tooooolongfunnnnnctionnnnnname:
    function justootoototlongfunctionname() {
      var a = 1;
    }
};

```

@@ +331,5 @@
> +
> +    // Check for tech types:
> +    this._debug('command.tech: ' + command.tech);
> +    var handled = false;
> +    var techs = command.tech;

Is this an array or what?

@@ +334,5 @@
> +    var handled = false;
> +    var techs = command.tech;
> +    if (command.ndef.length) {
> +      // Pick the first NDEF message for now.
> +      var ndefMsg = command.ndef[0];

What? the scope of this variable is only in the braces.
Also what about command.ndef[1+] ?

@@ +336,5 @@
> +    if (command.ndef.length) {
> +      // Pick the first NDEF message for now.
> +      var ndefMsg = command.ndef[0];
> +    } else {
> +      var ndefMsg = [];

scope problem

@@ +343,5 @@
> +    // Force Tech Priority:
> +    var pri = ['P2P', 'NDEF', 'NDEF_FORMATTABLE', 'NFC_A', 'MIFARE_ULTRALIGHT'];
> +    for (var ti = 0; ti < pri.length; ti++) {
> +      this._debug('Going through NFC Technologies: ' + ti);
> +      var i = techs.indexOf(pri[ti]);

I totally don't understand the meaning of this statement.

@@ +344,5 @@
> +    var pri = ['P2P', 'NDEF', 'NDEF_FORMATTABLE', 'NFC_A', 'MIFARE_ULTRALIGHT'];
> +    for (var ti = 0; ti < pri.length; ti++) {
> +      this._debug('Going through NFC Technologies: ' + ti);
> +      var i = techs.indexOf(pri[ti]);
> +      if (i != -1) {

if (i < 0) {
  return;
}

@@ +346,5 @@
> +      this._debug('Going through NFC Technologies: ' + ti);
> +      var i = techs.indexOf(pri[ti]);
> +      if (i != -1) {
> +        if (techs[i] == 'P2P') {
> +          if (!ndefMsg) {

You don't have ndefMsg here!

@@ +359,5 @@
> +            //       system application notifies gecko of the top most window.
> +
> +            // Notify gecko of User's acknowledgement.
> +            var nfcdom = window.navigator.mozNfc;
> +            nfcdom.setPeerWindow(window.top);

I don't see this in API doc. I do think you should pass window.top

@@ +384,5 @@
> +        }
> +      }
> +    }
> +
> +    if (handled == false) {

===

@@ +551,5 @@
> +         / "NOTE" / "PRODID" / "REV" / "SOUND" / "UID" / "CLIENTPIDMAP"
> +         / "URL" / "KEY" / "FBURL" / "CALADRURI" / "CALURI" / "XML"
> +         / iana-token / x-name
> +     */
> +    var fn = findKey('FN:', payload);

I don't *

@@ +557,5 @@
> +    var p = fn.indexOf(' ');
> +    var first = null;
> +    var last = null;
> +    if (p == 0) {
> +      var first = fn.substring(0, fn.indexOf(' '));

scope error!

@@ +559,5 @@
> +    var last = null;
> +    if (p == 0) {
> +      var first = fn.substring(0, fn.indexOf(' '));
> +    } else {
> +      var first = fn.substring(0, fn.indexOf(' '));

scope error!

@@ +560,5 @@
> +    if (p == 0) {
> +      var first = fn.substring(0, fn.indexOf(' '));
> +    } else {
> +      var first = fn.substring(0, fn.indexOf(' '));
> +      var last = fn.substring(fn.indexOf(' ') + 1);

scope error!

@@ +572,5 @@
> +    var note = findKey('NOTE:', payload);
> +    var address = findKey('ADR;HOME:', payload);
> +    var company = findKey('ADR;WORK:', payload);
> +
> +    // Add fields:

Why not use an objec/array to iterate this.findKey and doing assign instead of all if below

Comment 44

[Garner Lee]

Hi Alive,

I'm working on this, but do you prefer to post a link to the pull request to github, or do the review here (it seems to go both ways in Gaia?).

To answer the question about tech priority:
An NFC tag can present itself as multiple different types. For example, the tags we have "NFC MiFare Ultralight" technology tags, with 142 KB free, can be read as a NFC_A tag, a MiFare tag (NXP proprietary), or a standard NDEF tag. We just choose P2P (for sharing), and NDEF as the supported technologies for now.

The tech list, is currently presented as "tech" from the NFC Chrome Worker you pointed out (We should probably fix that next time the NFC Chrome Worker is patched...).

Comment 45

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Garner Lee from comment #44)
> Hi Alive,
> 
> I'm working on this, but do you prefer to post a link to the pull request to
> github, or do the review here (it seems to go both ways in Gaia?).

Please do pull request. THX.

> 
> To answer the question about tech priority:
> An NFC tag can present itself as multiple different types. For example, the
> tags we have "NFC MiFare Ultralight" technology tags, with 142 KB free, can
> be read as a NFC_A tag, a MiFare tag (NXP proprietary), or a standard NDEF
> tag. We just choose P2P (for sharing), and NDEF as the supported
> technologies for now.
> 
> The tech list, is currently presented as "tech" from the NFC Chrome Worker
> you pointed out (We should probably fix that next time the NFC Chrome Worker
> is patched...).

This naming is really bad for understanding the code...

Comment 46

[Garner Lee]

Attachment: NFC System app integration: activies and messages, and airplane mode (v6)

Comment 47

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 833399 [details] [review]
NFC System app integration: activies and messages, and airplane mode (v6)

1. Remove power level settings or patch gecko before landing this. NO settings API pollution.
2. Please look and read all your handleXXXXX functions again:
   (1) Some of them handle something.
   (2) Some of them format parameter object.
   (3) Some of them do nothing but call another function.
3. DOM Request are asynchronous.

Comment 48

[Garner Lee]

Flip dependency: this bug absolutely depends on Bug 933585 on the NFC DOM to operate, which depends on Bug 933497

Comment 49

[Garner Lee]

Switch dependency to more active bug on the same topic: Bug 934835

Comment 50

[Garner Lee]

Attachment: NFC Manager Implemenation

This patch includes power state changes via messages that have just been pushed to Gecko and Gaia.

Comment 51

[Garner Lee]

NFC UI/UX Question (There's no user story covering this):

Some of this can be partially initiated by NFC Manager:

Should the UI do anything when the tag/P2P device is becomes available (handleTechDiscovered)?
When the tag/P2P device becomes unavailable (handleTechLost)?

A sound or vibrate makes some sense when the tag is found or lost (Accessability), etc.)

// Play sound, or do something tactile:
window.navigator.vibrate([200]);

Also, techLost sort of makes no sense to apps. There's nothing for the app to do (except update a UI).

Comment 52

[Garner Lee]

Just attaching anyone for UX comment for now...

Comment 53

[Garner Lee]

I've updated the NFC Manager on the same pull branch request (for review). I didn't wait for the P2P updates.

Comment 54

[Ken Chang[:kenkai]]

(In reply to Garner Lee from comment #51)
> NFC UI/UX Question (There's no user story covering this):
> 
> Some of this can be partially initiated by NFC Manager:
> 
> Should the UI do anything when the tag/P2P device is becomes available
> (handleTechDiscovered)?
> When the tag/P2P device becomes unavailable (handleTechLost)?
> 
> A sound or vibrate makes some sense when the tag is found or lost
> (Accessability), etc.)
> 
> // Play sound, or do something tactile:
> window.navigator.vibrate([200]);
> 
> Also, techLost sort of makes no sense to apps. There's nothing for the app
> to do (except update a UI).

Let's add this later....:).

Comment 55

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8335027 [details] [review]
NFC Manager Implemenation

Basically r+, please track the followups.

Thanks!

Comment 56

[Ken Chang[:kenkai]]

(In reply to Garner Lee from comment #53)
> I've updated the NFC Manager on the same pull branch request (for review). I
> didn't wait for the P2P updates.

Should we create a follow up bug?

Comment 57

[Garner Lee]

Pull request updated with review comments. Concerning gecko knowledge of the security lock screen state, I've only currently found mozPower to know about WakeLock, but not security lock. If that's not true, I'll open a new bug for followup. If all checks fine, let me know, and I'll add r=alive.

Comment 58

[Garner Lee]

Attachment: NFC Manager implementation.

This new pull is a cherry-pick commit top of a new and current merge-able commit base.

Comment 59

[Garner Lee]

Comment on attachment 8337181 [details] [review]
NFC Manager implementation.

Same commit change as before, just rebased.

Comment 60

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8337181 [details] [review]
NFC Manager implementation.

Still one test failed.
https://travis-ci.org/mozilla-b2g/gaia/jobs/14402274

Please try again.

Comment 61

[Garner Lee]

Green with re-push!
https://travis-ci.org/mozilla-b2g/gaia/builds/14515364

What is going on with the persona test case causing it to fail?

Comment 62

[(inactive after 6/18) Alive Kuo [:alive]]

Don't know. BTW if you need to merge, needinfo me.

Comment 63

[Garner Lee]

Check-in needed (in Gaia).

Comment 64

[(inactive after 6/18) Alive Kuo [:alive]]

Merged
https://github.com/mozilla-b2g/gaia/commit/5376482f9e7f1724195001146eb132f98dd53209