Closed Bug 1054139 Opened 5 years ago Closed 5 years ago

B2G NFC: Move getNFCPeer to MozNFCManager

Categories

(Firefox OS Graveyard :: NFC, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
2.1 S9 (21Nov)

People

(Reporter: allstars.chh, Assigned: kamituel)

References

Details

(Whiteboard: [good first bug])

Attachments

(1 file, 2 obsolete files)

In Bug 1046554 we pass MozNFCPeer in onpeerready, so apps should get the MozNFCPeer through that callback, not from getNFCPeer.
So getNFCPeer right now should be only used by System app, i.e. app with "nfc-manager" permission.
Whiteboard: [good first bug]
Assignee: nobody → kamituel
Moves |getNFCPeer| and |getNFCTag| to NfcManager and adds the "nfc-manager" permission check.
Attachment #8479330 - Flags: review?(allstars.chh)
Attached patch 0002-message-undefined.patch (obsolete) — Splinter Review
I've also noticed that "message is undefined" is being printed. This is a fix for that. Can it be fixed in this bug, or should I file a new one?
Attachment #8479332 - Flags: review?(allstars.chh)
Comment on attachment 8479330 [details] [diff] [review]
0001-move-methods-to-nfc-manager.patch

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

forward r? to DOM peer, smaug.

::: dom/webidl/MozNFC.webidl
@@ +44,4 @@
>     /**
>      * Returns MozNFCTag object or null in case of invalid sessionToken
>      */
> +   [CheckPermissions="nfc-manager"]

NfcManager already has this check, 
so adding again is not necessary.

@@ +49,5 @@
>  
>     /**
>      * Returns MozNFCPeer object or null in case of invalid sessionToken
>      */
> +   [CheckPermissions="nfc-manager"]

ditto
Attachment #8479330 - Flags: review?(bugs)
Attachment #8479330 - Flags: review?(allstars.chh)
Attachment #8479330 - Flags: review+
Attachment #8479332 - Flags: review?(allstars.chh) → review+
Comment on attachment 8479332 [details] [diff] [review]
0002-message-undefined.patch

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

Hi Kamil
Sorry I also met another msg/message naming problem today so I'll fix them all in Bug 1059102
Attachment #8479330 - Flags: review?(bugs) → review+
Checking needed for 0001-move-methods-to-nfc-manager.patch. Please don't merge the second one - it has been fixed by Yoshi in 1059102.
Keywords: checkin-needed
Do you have a Try link for this by chance? :)
Keywords: checkin-needed
Comment on attachment 8479332 [details] [diff] [review]
0002-message-undefined.patch

(In reply to Kamil Leszczuk [:kamituel] from comment #5)
> Checking needed for 0001-move-methods-to-nfc-manager.patch. Please don't
> merge the second one - it has been fixed by Yoshi in 1059102.

You can mark obsolete patches as such in the future to avoid any possible confusion.
Attachment #8479332 - Attachment is obsolete: true
(In reply to Yoshi Huang[:allstars.chh] from comment #3)
> > +   [CheckPermissions="nfc-manager"]
> 
> NfcManager already has this check, 
> so adding again is not necessary.
Kamil mentioned that this check is actually neccesary because it's called from MozNFC and not from MozNFCManager. He will update the patch and send r? again.
Kamil, can you land this patch?
Flags: needinfo?(kamituel)
Attached patch Bug1054139.patchSplinter Review
Attachment #8479330 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/3ab678e588cc
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S9 (21Nov)
Flags: needinfo?(kamituel)
You need to log in before you can comment on or make changes to this bug.