Closed Bug 1109456 Opened 5 years ago Closed 5 years ago

Support NFC tag transceive WebAPI

Categories

(Firefox OS Graveyard :: NFC, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(feature-b2g:2.2+, tracking-b2g:+)

RESOLVED FIXED
2.2 S3 (9jan)
feature-b2g 2.2+
tracking-b2g +

People

(Reporter: dimi, Assigned: dimi)

References

Details

(Whiteboard: [priority])

Attachments

(1 file, 10 obsolete files)

To read proprietary data in different NFC card, we need to support transceive WebAPI for App
Assignee: nobody → dlee
tracking-b2g: --- → +
Whiteboard: [priority]
bug 1105666 may take a while, remove me from Assignee
Assignee: dlee → nobody
Assignee: nobody → dlee
Attachment #8538943 - Flags: review?(allstars.chh)
Attachment #8538945 - Flags: review?(allstars.chh)
Attachment #8538947 - Flags: review?(allstars.chh)
Comment on attachment 8538943 [details] [diff] [review]
Part1. Transceive WebIDL patch v1

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

We just need the trasceive interface in MozNFCTag, other interfaces should be done in other bugs in Bug 1082992.
Attachment #8538943 - Flags: review?(allstars.chh)
Attachment #8538945 - Flags: review?(allstars.chh)
Attachment #8538947 - Flags: review?(allstars.chh)
Comment on attachment 8538943 [details] [diff] [review]
Part1. Transceive WebIDL patch v1

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

::: dom/webidl/MozNFCTag.webidl
@@ +141,5 @@
> +  /**
> +   * Send raw command to tag and receive the response.
> +   */
> +  [Throws, ChromeOnly]
> +  Promise<sequence<byte>> transceive(NFCTechType tech, sequence<byte> command);

request feedback from smuag for whether we should use
Uint8Array or sequence<byte> here.

Or can you help to explain their difference ?

Thanks
Attachment #8538943 - Flags: feedback?(bugs)
Comment on attachment 8538947 [details] [diff] [review]
Part3. GECKO implementation. patch1

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

::: dom/nfc/gonk/NfcMessageHandler.cpp
@@ +260,5 @@
> +
> +  uint32_t length = aOptions.mCommand.Length();
> +  aParcel.writeInt32(length);
> +  for (uint32_t i = 0; i < length; i++) {
> +    aParcel.writeInt32(aOptions.mCommand[i]);

Should write an ByteArray, here you write Uint32Array.

@@ +419,5 @@
> +  uint32_t length = aParcel.readInt32();
> +  aOptions.mResponse.SetCapacity(length);
> +
> +  for (uint32_t i = 0; i < length; i++) {
> +    uint8_t data = aParcel.readInt32();

Ditto, here you read an Uint32Array.
Comment on attachment 8538947 [details] [diff] [review]
Part3. GECKO implementation. patch1

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

Hi, smaug
we'd like to request for your feedback here.

For the use of Promise API, I found that it only lists the type returned in OnResolve, but doesn't list the type in OnRejected.

For example, in this bug, 

Promise<sequeuce<byte>> transceive(...), sequence<byte> is the type returned in OnResolve, but what about the type returned in OnRejected?
So what's the common pattern we use in onReject? return DOMError , Error, or just DOMString ? 
And how do we help app developers to know how to do error handling of this interface?

Thanks

::: dom/nfc/nsNfc.js
@@ +89,5 @@
>        debug("can not find promise resolver for id: " + this._requestId +
>             ", errormsg: " + aErrorMsg);
>        return;
>      }
>      resolver.reject(aErrorMsg);

Here we reject with DOMString.

@@ +198,5 @@
>    },
>  
> +  transceive: function transceive(tech, cmd) {
> +    if (this.isLost) {
> +      throw new this._window.DOMError("InvalidStateError", "NFCTag object is invalid");

Here we reject with DOMError.
Attachment #8538947 - Flags: feedback?(bugs)
Attachment #8538945 - Flags: review?(allstars.chh)
Fix not using byte array in NfcMessageHandler.cpp
Attachment #8539911 - Flags: review?(allstars.chh)
Blocks: 916428
Comment on attachment 8538943 [details] [diff] [review]
Part1. Transceive WebIDL patch v1

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

I check current m-c mostly we use Uint8Array for this.
Attachment #8538943 - Flags: feedback?(bugs)
Comment on attachment 8538945 [details] [diff] [review]
Part2. WebIDL for message between gecko and nfcd v1

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

::: dom/webidl/NfcOptions.webidl
@@ +22,5 @@
>    boolean isP2P;
>    sequence<MozNDEFRecordOptions> records;
> +
> +  NFCTechType technology;
> +  sequence<byte> command;

Use Uint8Array.
Attachment #8538945 - Flags: review?(allstars.chh)
Comment on attachment 8538947 [details] [diff] [review]
Part3. GECKO implementation. patch1

Found Bug 1016560 has processed DOMError/String.
Attachment #8538947 - Flags: feedback?(bugs)
Attachment #8538947 - Attachment is obsolete: true
Use Uint8Array
Attachment #8538943 - Attachment is obsolete: true
Attachment #8545085 - Flags: review?(allstars.chh)
Use Uint8Array
Attachment #8538945 - Attachment is obsolete: true
Attachment #8545087 - Flags: review?(allstars.chh)
Attachment #8545085 - Attachment is obsolete: true
Attachment #8545085 - Flags: review?(allstars.chh)
Attachment #8539911 - Attachment is obsolete: true
Attachment #8539911 - Flags: review?(allstars.chh)
Attachment #8545087 - Attachment is obsolete: true
Attachment #8545087 - Flags: review?(allstars.chh)
Change in this patch
- Rebase
- use Uint8Array
Attachment #8545122 - Flags: review?(allstars.chh)
Change in this patch:
- Use Uint8Array
Attachment #8545125 - Flags: review?(allstars.chh)
Change in this patch
- rebase
Attachment #8545127 - Flags: review?(allstars.chh)
Comment on attachment 8545122 [details] [diff] [review]
Part1. Transceive WebIDL patch v3

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

::: dom/nfc/nsINfcContentHelper.idl
@@ +174,5 @@
> +   * @param command
> +   *        Command to send
> +   */
> +  void transceive(in DOMString sessionToken,
> +                  in nsINfcRequestCallback callback,

callback should be last arg.
Attachment #8545122 - Flags: review?(allstars.chh)
Attachment #8545125 - Flags: review?(allstars.chh) → review+
Attachment #8545127 - Flags: review?(allstars.chh) → review+
Attachment #8545122 - Attachment is obsolete: true
Attachment #8545191 - Flags: review?(bugs)
Attachment #8545191 - Flags: review?(allstars.chh)
Comment on attachment 8545125 [details] [diff] [review]
Part2. WebIDL for message between gecko and nfcd v3

Add r? to smug because of WebIDL change
Attachment #8545125 - Flags: review?(bugs)
Attachment #8545125 - Flags: review?(bugs) → review+
Attachment #8545191 - Flags: review?(bugs) → review+
Comment on attachment 8545191 [details] [diff] [review]
Part1. Transceive WebIDL patch v4

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

::: dom/webidl/MozNFCTag.webidl
@@ +104,5 @@
> +  /**
> +   * Send raw command to tag and receive the response.
> +   */
> +  [Throws, ChromeOnly]
> +  Promise<Uint8Array> transceive(NFCTechType tech, Uint8Array command);

Move this interface to the partial interface below.
Also I prefer move Throws to the last attr although this is not mandatory.

Or if you'd like to merge the partial interface into one is also fine by me.
Attachment #8545191 - Flags: review?(allstars.chh) → review+
Attachment #8545125 - Attachment is obsolete: true
Attachment #8545127 - Attachment is obsolete: true
Attachment #8545191 - Attachment is obsolete: true
Attachment #8545826 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/0c39bc01f634
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
feature-b2g: --- → 2.2+
You need to log in before you can comment on or make changes to this bug.