1054139 - B2G NFC: Move getNFCPeer to MozNFCManager

Status: RESOLVED FIXED

(Firefox OS Graveyard :: NFC, defect)

Description

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

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.

Comment 1

[Kamil Leszczuk [:kamituel]]

Attachment: 0001-move-methods-to-nfc-manager.patch

Moves |getNFCPeer| and |getNFCTag| to NfcManager and adds the "nfc-manager" permission check.

Comment 2

[Kamil Leszczuk [:kamituel]]

Attachment: 0002-message-undefined.patch

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?

Comment 3

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

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

Comment 4

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

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

Comment 5

[Kamil Leszczuk [:kamituel]]

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.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Do you have a Try link for this by chance? :)

Comment 7

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 8

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

(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.

Comment 9

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

Kamil, can you land this patch?

Comment 10

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

https://hg.mozilla.org/integration/b2g-inbound/rev/3ab678e588cc

Comment 11

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

Attachment: Bug1054139.patch

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3ab678e588cc