Closed Bug 1002391 Opened 7 years ago Closed 7 years ago

[Flame] Could not send image/video via NFC

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.0+, b2g-v2.0 verified)

VERIFIED FIXED
2.0 S3 (6june)
blocking-b2g 2.0+
Tracking Status
b2g-v2.0 --- verified

People

(Reporter: ashiue, Assigned: echou)

References

Details

(Whiteboard: [p=8][ETA:6/6])

Attachments

(2 files, 1 obsolete file)

Attached file nfc_send_image.log
Device    Flame
Gaia      265e1ac4ee71ad6190335c974bfce33f783edfce
Gecko     https://hg.mozilla.org/mozilla-central/rev/ea25086073c8
BuildID   20140427160202
Version   31.0a1


STR:
1. launch "settings" app for 2 phones
2. turn on "NFC" for 2 phones
3. open an image on phone A 
4. tap 2 phones
5. swipe shrinking UI on phone A

Expected result:
phone B receive image shared by phone A

Actual result:
phone B could not receive image
Assignee: nobody → allstars.chh
We are going to use flame as a reference device.
blocking-b2g: --- → 2.0+
After discussed with Eric, this is a Bluetooth bug.
re-assign to him.
Assignee: allstars.chh → nobody
Component: NFC → Bluetooth
Summary: [NFC][Flame] Could not send image/video via NFC → [Flame] Could not send image/video via NFC
Assignee: nobody → echou
Blocks: b2g-NFC-2.0
It is a blocker for NFC 2.0 release.
Target Milestone: --- → 2.0 S3 (6june)
Whiteboard: [p=8][ETA:5/30]
Keywords: verifyme
Device    Nexus 4
BuildID   20140516110816
Version   32.0a1
Git Commit info 2014-05-05 16:59:23 d33e4f15

I tested above on nexus 4 and the issue is not present
(In reply to Mishra from comment #4)
> Device    Nexus 4
> BuildID   20140516110816
> Version   32.0a1
> Git Commit info 2014-05-05 16:59:23 d33e4f15
> 
> I tested above on nexus 4 and the issue is not present

Yes, this is BlueZ specific bug so it won't be reproducible on Nexus 4/5 which use Bluedroid as bt stack.

Thanks for your test. :)
update ETA to 6/9 per discussion with Eric.
Whiteboard: [p=8][ETA:5/30] → [p=8][ETA:6/9]
Whiteboard: [p=8][ETA:6/9] → [p=8][ETA:6/6]
There is a javascript "let is a reserved identifier" error in ndef_utils.js when trying to send file
Depends on: 1020213
I've been working on this these two days and I've already had a solution. I'll provide a patch tomorrow to fix this.
Tested on my Flame.
Attachment #8435555 - Flags: review?(shuang)
Comment on attachment 8435555 [details] [diff] [review]
patch 1: v1: Get the latest SDP records of the target

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

It looks good to me. In general, I think we need to consider if the remote is using rfcomm secured socket with MITM in the long term, for non-MITM case, I'm ok with it :)

::: dom/bluetooth/bluez/BluetoothDBusService.cpp
@@ +3647,5 @@
>      MOZ_ASSERT(!sAdapterPath.IsEmpty());
>  
> +    // We first guess that the device doesn't exist at all. So we use BlueZ
> +    // API "CreateDevice" to create an object path for the BluetoothDevice
> +    // object.

nits:Can we add something like "Update SDP records before calling DBUS API 'CreateDevice for NFC use case" to make comments easy to understand? In a normal use case, we don't need to do 'CreateDevice' before query SDP records before pairing procedure always performed earlier.

@@ +3648,5 @@
>  
> +    // We first guess that the device doesn't exist at all. So we use BlueZ
> +    // API "CreateDevice" to create an object path for the BluetoothDevice
> +    // object.
> +    NS_ConvertUTF16toUTF8 address(mDeviceAddress);

nits:Please validate mDeviceAddress in the beginning.

@@ +3679,5 @@
> +      // SDP records then we have to do "DiscoverServices"
> +      BT_LOGR("%s", NS_ConvertUTF16toUTF8(errorString).get());
> +
> +      OnUpdateSdpRecordsRunnable* r =
> +        static_cast<OnUpdateSdpRecordsRunnable*>(aData);

OnUpdateSdpRecordsRunnable shall be moved outside if-block, right?

@@ +3686,5 @@
> +      r->GetDeviceAddress(deviceAddress);
> +
> +      const nsString objectPath =
> +        GetObjectPathFromAddress(sAdapterPath, deviceAddress);
> +

We need to add if-guard to make sure Bluetooth is not in the middle of turning on/off. Otherwise, I'm afraid that we might see Bug 1015810. I think they are the similar scenario.
Attachment #8435555 - Flags: review?(shuang)
* Patch updated.
Attachment #8435555 - Attachment is obsolete: true
Attachment #8435680 - Flags: review?(shuang)
Comment on attachment 8435680 [details] [diff] [review]
patch 1: v2: Get the latest SDP records of the target

LGTM, thanks.
Attachment #8435680 - Flags: review?(shuang) → review+
(In reply to Shawn Huang [:shuang] [:shawnjohnjr] from comment #10)
> Comment on attachment 8435555 [details] [diff] [review]
> patch 1: v1: Get the latest SDP records of the target
> 
> Review of attachment 8435555 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> 
> We need to add if-guard to make sure Bluetooth is not in the middle of
> turning on/off. Otherwise, I'm afraid that we might see Bug 1015810. I think
> they are the similar scenario.
My bad, Bug 1015810 won't happen, because now we handle things in I/O thread.
(In reply to Shawn Huang [:shuang] [:shawnjohnjr] from comment #12)
> Comment on attachment 8435680 [details] [diff] [review]
> patch 1: v2: Get the latest SDP records of the target
> 
> LGTM, thanks.

Thanks! :D
https://hg.mozilla.org/mozilla-central/rev/03e02a8a1677
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Verified on
Device    Flame (Nexus 4)
Gaia      2e5636e852a9354a5f8072b179cf16f72647cfd6
Gecko     https://hg.mozilla.org/mozilla-central/rev/8bd92dc9ef59
BuildID   20140608160201
Version   32.0a1
Status: RESOLVED → VERIFIED
Verified fixed. Issue does not occur in latest Flame 2.0.
NFC is able to properly transfer image and video files.

Environmental Variables:
Device: Flame 2.0
BuildID: 20140910000203
Gaia: 3f4c635106c5364228782d12b1cb76b0c105b971
Gecko: 02a5b9234c13
Version: 32.0 (2.0)
Firmware: V123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(pbylenga)
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.