948721 - (meta) [B2G][NFC] Review WebNFC changes

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: NFC, defect)

Description

[Garner Lee]

There was an observation that the WebNFC APIs has changed since the last super review.

P2P has been added, as well as an API to determine which app controls the NFC hardware.

These APIs are:
   DOMRequest checkP2PRegistration(DOMString manifestUrl);

   attribute EventHandler onpeerready;
   attribute EventHandler onpeerlost;
   [ChromeOnly]
   void eventListenerWasAdded(DOMString aType);
   [ChromeOnly]
   void eventListenerWasRemoved(DOMString aType);

MozNFCPeer and MozNFCTag have been rearranged as well. Snapshot of source file here:
http://git.mozilla.org/?p=releases/gecko.git;a=blob;f=dom/webidl/MozNfc.webidl;h=d903ed588bd89a25eea5fca4204be167576e3663;hb=HEAD

Comment 1

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Also you forgot mentioning you added SessionToken.

Comment 2

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Bug 933585 is also landed without sr+.

Comment 3

[Ken Chang[:kenkai]]

(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #2)
> Bug 933585 is also landed without sr+.
Garner, can you please add this? Thanks.
It seems be better to have patches for this bug.

Comment 4

[Garner Lee]

Ken, good point. Patch links:

The Uint8Array updates to MozNdefRecord:
https://bugzilla.mozilla.org/show_bug.cgi?id=933585

MozNFCTag, MozNFCPeer (and MozNfc):
https://bugzilla.mozilla.org/show_bug.cgi?id=WebNFC
(attachment: https://bug674741.bugzilla.mozilla.org/attachment.cgi?id=828121,  Patch 1 (v15) NFC DOM WebIDL r=khuey)

P2P additions (Oili sr+'ed them);
https://bugzilla.mozilla.org/show_bug.cgi?id=933136
(attachment: https://bugzilla.mozilla.org/attachment.cgi?id=8341966,  (v8c) Part 1 : Add DOM event handlers 'onpeerready' 'onpeerlost' r=khuey, sr=smaug)

Comment 5

[Garner Lee]

Perhaps a new bug is needed, but one thing I think can be improved was the fact navigator.mozNfc has been getting some privilaged state management APIs becuase the Gaia's System app nfc_manager.js needed them to communicate to chrome and then to other apps.

We could refactor them into navigator.mozNfc and navigator.nfcManager to better hide NFC manager only APIs. However, then there will be a NFC Manager in both gecko and gaia. Not quite pretty either. Other suggestions?

Comment 6

[Ken Chang[:kenkai]]

This bug is a followup bug for bug 674741.

Comment 7

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

(In reply to Garner Lee from comment #4)
> Ken, good point. Patch links:
> 
> The Uint8Array updates to MozNdefRecord:
> https://bugzilla.mozilla.org/show_bug.cgi?id=933585
> 
> MozNFCTag, MozNFCPeer (and MozNfc):
> https://bugzilla.mozilla.org/show_bug.cgi?id=WebNFC
> (attachment:
> https://bug674741.bugzilla.mozilla.org/attachment.cgi?id=828121,  Patch 1
> (v15) NFC DOM WebIDL r=khuey)
> 
> P2P additions (Oili sr+'ed them);
> https://bugzilla.mozilla.org/show_bug.cgi?id=933136
> (attachment: https://bugzilla.mozilla.org/attachment.cgi?id=8341966,  (v8c)
> Part 1 : Add DOM event handlers 'onpeerready' 'onpeerlost' r=khuey, sr=smaug)

Even Bug 674741 is not superreviewed.

Comment 8

[Ken Chang[:kenkai]]

Garner, could you please comments all dependent bugs? Thanks.
Yoshi, per offline discussion, please help to do the first round review. Thanks.

Comment 9

[Garner Lee]

Ken, how do you want that done? I can do so on individual bugs (this is a metabug only, as I understand it).

Comment 10

[Ken Chang[:kenkai]]

(In reply to Garner Lee from comment #9)
> Ken, how do you want that done? I can do so on individual bugs (this is a
> metabug only, as I understand it).
Yes, I know. Sorry for no clear. I am talking about the dependent bugs of this meta bug.

Comment 11

[Ken Chang[:kenkai]]

This bug doesn't block 2.0.

Comment 12

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

We're going to make NFC API privilage in Bug 1042851, so we will review entire NFC API as a whole again, close as WONTFIX for now.