Closed Bug 960790 Opened 11 years ago Closed 11 years ago

[B2G][NFC] Fix P2P presence discovery handling.

Categories

(Firefox OS Graveyard :: NFC, defect)

x86_64
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
1.3 C3/1.4 S3(31jan)

People

(Reporter: dgarnerlee, Assigned: dgarnerlee)

References

Details

Attachments

(2 files, 2 obsolete files)

Attached patch tempFix.patch (obsolete) (deleted) — Splinter Review
The change to single level records broke P2P message parsing:
https://bugzilla.mozilla.org/show_bug.cgi?id=958423#c10

Specifically, P2P discovery does not have an NDEF Message to parse.

Attached is a small temporary patch to get things going, since I believe there was some discusson weeks ago about whether to send all NDEF messages, or just the first one, where available.
Blocks: b2g-nfc
Arno, Yoshi, comments? Otherwise, it's a small change (P2P ShrinkUI works again with the temporary fix.).
Flags: needinfo?(allstars.chh)
Summary: [B2G][NFC] Fix P2P prescense discovery handling. → [B2G][NFC] Fix P2P presence discovery handling.
Comment on attachment 8361359 [details] [diff] [review]
tempFix.patch

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

::: dom/system/gonk/nfc_worker.js
@@ +368,4 @@
>  NfcWorker[NFC_NOTIFICATION_TECH_DISCOVERED] = function NFC_NOTIFICATION_TECH_DISCOVERED() {
>    debug("NFC_NOTIFICATION_TECH_DISCOVERED");
>    let techList  = [];
> +  let records   = [];

I think we could just leave it undefined here so we don't have to create an extra array where there's NdefRecords in the parcel.
(unMarshallNdefMessage will also return a [], which make the [] here not neccesary)
Also if the records is an array by default, the check in 
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/nfc_manager.js#L378

if (records != null) is not neccesary.




But in that case we also need to update nfc_manager, since if it's undefined, records.length will throw TypeError.
Flags: needinfo?(allstars.chh)
Blocks: b2g-NFC-2.0
Modify patch to use review comment for tempFix.patch.
Attachment #8362502 - Flags: review?(allstars.chh)
Attachment #8361359 - Attachment is obsolete: true
Attachment #8362506 - Flags: review?(alive)
Attachment #8362502 - Flags: review?(allstars.chh) → review+
Attachment #8362502 - Attachment description: Bug 960790: Fix P2P presence discovery handling (gecko) → Bug 960790: Fix P2P presence discovery handling (gecko) r=allstars.chh
Code dependency: The gaia side should land before the gecko part.
Help Garner to add r=me.
Attachment #8362502 - Attachment is obsolete: true
Assignee: nobody → dgarnerlee
Attachment #8362506 - Flags: review?(alive) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/d5a73fc7cf9e
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.3 C3/1.4 S3(31jan)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: