Closed Bug 964693 Opened 11 years ago Closed 11 years ago

Gecko Bluetooth always receives a blob but not a file while using NFC to do BT file transfer

Categories

(Firefox OS Graveyard :: NFC, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(feature-b2g:2.0, tracking-b2g:backlog)

VERIFIED FIXED
1.4 S5 (11apr)
feature-b2g 2.0
tracking-b2g backlog

People

(Reporter: echou, Assigned: psiddh)

References

Details

(Whiteboard: [FT:RIL])

Attachments

(1 file)

After applying several patches from Sid, I was finally be able to call into BluetoothOppManager::SendFile(). In the function it tried to resolve the file metadata such as file length, file name and MIME type but failed. I believe that's because the information was lost at some point of the whole NFC flow(Gaia->Gecko->Gaia). To prove the blob is truly a blog object, you can dump the type of msg.blob in window.navigator.mozSetMessageHandler('nfc-manager-send-file').
Ken and Arno, Do you have idea who can take this bug?
Flags: needinfo?(kchang)
Flags: needinfo?(arno)
(In reply to Wesley Huang [:wesley_huang] from comment #1) > Ken and Arno, > Do you have idea who can take this bug? I think this should be assigned to Eric. In his description he asked for some more information. I'll do a needs-info from Sid so he can provide what Eric asked for.
Flags: needinfo?(arno) → needinfo?(psiddh)
Yes Eric, you are right. Here is the blob flow for the reference from the Nfc App 1. Nfc Gaia App --> 2. Nfc DOM --> 3. NfcContentHelper.js --> 4. Nfc.js (Chrome Process) --> 5. Back To Gaia System App through a system message --> 6. SendFile BlueTooth (that accepts only blob of type 'file') At step #2, currently nfc dom does "blob.slice". After some analysis, it looks like this step converts the blob of type 'file' (originally sent by Nfc App) to 'blob' type. Since it remains a type of 'blob' after that, at step#6 it blows up. Now if I do not do "blob.slice", when we try to send blob (to NfcContentHelper.js) at STEP#2, I see the following error msgs in adb logs E/GeckoConsole( 1144): [JavaScript Error: "Permission denied to access property 'toString'"] E/GeckoConsole( 1144): [JavaScript Error: "Permission denied to access property 'message'"] E/GeckoConsole( 1144): [JavaScript Error: "uncaught exception: unknown (can't convert to string)"] Any suggestions please ?
Flags: needinfo?(psiddh)
Information has been provided.
Flags: needinfo?(kchang)
In short, the question is how should Nfc Dom code (JS implemented) send / pass the blob data (that it received from Nfc Gaia App) to NfcContentHelper.js. Note that in my current workspace, this is how sendFile interface at ContentHelper level (Step #3) is defined. 2) nsNfc.js (Dom code) sendFile: function sendFile(blob) { this._nfcContentHelper.sendFile(this._window, blob, this.session); } 2.1) nsINfcContentHelper.idl nsIDOMDOMRequest sendFile(in nsIDOMWindow window, in Blob blob, in DOMString sessionToken); 3) sendFile: function sendFile(window, blob, sessionToken) { . . } At Step#3, I see the following errors in adb logs as mentioned earlier, when Nfc DOM code attempts call into NfcContentHelper >E/GeckoConsole( 1144): [JavaScript Error: "Permission denied to access property 'toString'"] >E/GeckoConsole( 1144): [JavaScript Error: "Permission denied to access property 'message'"] >E/GeckoConsole( 1144): [JavaScript Error: "uncaught exception: unknown (can't convert to string)"]
> I think this should be assigned to Eric. In his description he asked for > some more information. I'll do a needs-info from Sid so he can provide what > Eric asked for. Actually no, I didn't mean that I'll take this bug myself. Waht I said was that Gecko Bluetooth couldn't receive a valid File object, and I thought it's because the metadata was lost at some point of the whole NFC flow which passes the argument back and forth between Gaia and Gecko.
(In reply to Siddartha P from comment #5) > In short, the question is how should Nfc Dom code (JS implemented) send / > pass the blob data (that it received from Nfc Gaia App) to > NfcContentHelper.js. > > Note that in my current workspace, this is how sendFile interface at > ContentHelper level (Step #3) is defined. > 2) nsNfc.js (Dom code) > sendFile: function sendFile(blob) { > this._nfcContentHelper.sendFile(this._window, blob, this.session); > } > 2.1) nsINfcContentHelper.idl > nsIDOMDOMRequest sendFile(in nsIDOMWindow window, > in Blob blob, > in DOMString sessionToken); > 3) sendFile: function sendFile(window, blob, sessionToken) { > . > . > } > I'm sorry that even I can understand the problem, I don't know how and when the file object is transformed to Blob. You need to investigate more or find someone familiar with Blob/File. Maybe Ben Turner can help. > At Step#3, I see the following errors in adb logs as mentioned earlier, when > Nfc DOM code attempts call into NfcContentHelper > > >E/GeckoConsole( 1144): [JavaScript Error: "Permission denied to access > property 'toString'"] > >E/GeckoConsole( 1144): [JavaScript Error: "Permission denied to access > property 'message'"] > >E/GeckoConsole( 1144): [JavaScript Error: "uncaught exception: unknown > (can't convert to string)"] No idea either. Did you see the error message with current codebase (without removing blob.slice)? If so, then this may be a different problem.
--> Current Code (as in Mozilla master): 2) nsNfc.js (DOM code) sendFile: function sendFile(blob) { let data = { "blob": blob.slice() }; this._nfcContentHelper.sendFile(this._window, data, this.session); } 2.1) nsINfcContentHelper.idl nsIDOMDOMRequest sendFile(in nsIDOMWindow window, in blob data, in DOMString sessionToken); 3) sendFile: function sendFile(window, data, sessionToken) { . . cpmm.sendAsyncMessage(..,data.blob,..) // send to Chrome } Problem: At Step#2, When we perform slice() on the blob, metadata seems to get cleared (No longer of type nsIDOMFile - File object, but instead it becomes 'blob object') and Bluetooth's sendFile() API doesn't seem to like blobs which are not of 'File object type' --> Current ongoing changes (Work in Progress) 2) nsNfc.js (DOM code) sendFile: function sendFile(blob) { this._nfcContentHelper.sendFile(this._window, blob, this.session); } 2.1) nsINfcContentHelper.idl nsIDOMDOMRequest sendFile(in nsIDOMWindow window, in Blob blob, in DOMString sessionToken); 3) sendFile: function sendFile(window, blob, sessionToken) { . . cpmm.sendAsyncMessage(..,blob,..) // send to Chrome } Note how the definition gets changed from 'jsval' to 'blob' in nsINfcContentHelper.idl for 'sendFile()' Problem: As soon as STEP#2 : 'this._nfcContentHelper.sendFile(...)' gets executed, I see the following error messages in adb logs >E/GeckoConsole( 1144): [JavaScript Error: "Permission denied to access property 'toString'"] >E/GeckoConsole( 1144): [JavaScript Error: "Permission denied to access property 'message'"] >E/GeckoConsole( 1144): [JavaScript Error: "uncaught exception: unknown (can't convert to string)"] Also note that it doesn't change much even if I do 'blob.slice()' at STEP#2 : 'this._nfcContentHelper.sendFile(this._window, blob.slice(), this.session);' I still the same error logs as mentioned above.
Please ignore my comment#8. There were some typos. Correcting my comment ************************************************************************************************* Current Code (as in Mozilla master): 2) nsNfc.js (DOM code) sendFile: function sendFile(blob) { let data = { "blob": blob.slice() }; this._nfcContentHelper.sendFile(this._window, data, this.session); } 2.1) nsINfcContentHelper.idl nsIDOMDOMRequest sendFile(in nsIDOMWindow window, in jsval data, in DOMString sessionToken); 3) sendFile: function sendFile(window, data, sessionToken) { . . cpmm.sendAsyncMessage(..,data.blob,..) // send to Chrome } Problem: At Step#2, When we perform slice() on the blob, metadata seems to get cleared (No longer of type nsIDOMFile - File object, but instead it becomes 'blob object') and Bluetooth's sendFile() API doesn't seem to like blobs which are not of 'File object type' ************************************************************************************************* Current ongoing changes (Work in Progress) 2) nsNfc.js (DOM code) sendFile: function sendFile(blob) { this._nfcContentHelper.sendFile(this._window, blob, this.session); } 2.1) nsINfcContentHelper.idl nsIDOMDOMRequest sendFile(in nsIDOMWindow window, in Blob blob, in DOMString sessionToken); 3) sendFile: function sendFile(window, blob, sessionToken) { . . cpmm.sendAsyncMessage(..,blob,..) // send to Chrome } Note how the definition gets changed from 'jsval' to 'blob' in nsINfcContentHelper.idl for 'sendFile()' Problem: As soon as STEP#2 : 'this._nfcContentHelper.sendFile(...)' gets executed, I see the following error messages in adb logs and nothing happens as far as functionality is concerned E/GeckoConsole( 1144): [JavaScript Error: "Permission denied to access property 'toString'"] E/GeckoConsole( 1144): [JavaScript Error: "Permission denied to access property 'message'"] E/GeckoConsole( 1144): [JavaScript Error: "uncaught exception: unknown (can't convert to string)"] Also note that it doesn't change much even if I do 'blob.slice()' at STEP#2 : 'this._nfcContentHelper.sendFile(this._window, blob.slice(), this.session);' I still see the same error logs as mentioned above.
Requesting Ben Turner to suggest on how to address the issue mentioned on comment#9
Flags: needinfo?(bent.mozilla)
Hey guys, sorry, but I'm gone until 2/14. Perhaps khuey can help?
Flags: needinfo?(bent.mozilla) → needinfo?(khuey)
The problem *might* be here: http://mxr.mozilla.org/mozilla-central/source/dom/ipc/Blob.cpp#958 We seem to always send NormalBlobConstructorParams, never FileBlobConstructorParams. Something to investigate anyway.
I'm in Taipei this week and more than a little busy, but someone can come find me if they need me.
Flags: needinfo?(khuey)
Who can take this bug? This blocks essential scenario for upcoming NFC demo, which we really need to get it done asap.
Here is a quick update from my side: I am able to send the file using Nfc BT handover to an Android device finally. Two Changes to my work-space were needed: 1) Pull the changes : https://bugzilla.mozilla.org/show_bug.cgi?id=962310 - final: Support in-process bt file transfer " (This change is in the process of getting landed) 2) Make quick **hack** in blob.cpp. Pass 'FileBlobConstructorParams' instead of NormalBlobConstructorParams as Ben Turner noted Could someone look into this and give a proper fix @ http://mxr.mozilla.org/mozilla-central/source/dom/ipc/Blob.cpp#958 ?
Kyle, except you, who do we can call for help? Thanks.
Flags: needinfo?(khuey)
Ben is on vacation this week so I think you are stuck with me. If this absolutely must be done this week Ken should come talk to me about it.
Flags: needinfo?(khuey)
I talked to Ken offline, this does not block the MWC demo, but it is still a priority.
Thanks, Kyle. I think you will take this bug. If I am wrong, correct me. It is a blocker for 1.4+ and effects the functionality of Audio, Video, and Image sharing.
Assignee: nobody → khuey
blocking-b2g: --- → 1.4+
Whiteboard: [FT:RIL]
Target Milestone: --- → 1.4 S2 (28feb)
Blocks: b2g-nfc
blocking-b2g: 1.4+ → ---
Target Milestone: 1.4 S2 (28feb) → ---
Did you intend to clear the blocking flag?
Flags: needinfo?(allstars.chh)
Sorry, No. :P My mistake.
blocking-b2g: --- → 1.4?
Flags: needinfo?(allstars.chh)
Target Milestone: --- → 1.4 S2 (28feb)
Hi Kyle, I wonder if this bug is able to fixed before this week. Then we could have a chance for demonstrating audio/vedio/image sharing in MWC. Of cause, it doesn't the must-have.
Flags: needinfo?(khuey)
blocking-b2g: 1.4? → 1.4+
No, I don't think so.
Flags: needinfo?(khuey)
No longer going to block - NFC needs to be preffed off in 1.4 per a drivers decision to cut scope to only DSDS & QC required features.
blocking-b2g: 1.4+ → 1.4?
move the blocking bug from duplicated bugs.
Blocks: 894320
Target Milestone: 1.4 S2 (28feb) → ---
Moving to 1.5? per de-scoped feature list in 1.4
blocking-b2g: 1.4? → 1.5?
Partner would like to have NFC handover mechanism ready in Gecko. Can we have this fix fixed in 1.4?
blocking-b2g: 1.5? → 1.4?
Flags: needinfo?(khuey)
Sandip, I know there are ongoing partner discussions here. Once the outcome of those discussions is known, please update the blocking flag accordingly.
Flags: needinfo?(skamat)
Per comment 26 NFC was cut from 1.4.
Flags: needinfo?(khuey)
blocking-b2g: 1.4? → backlog
(In reply to Peter Dolanjski [:pdol] from comment #30) > Sandip, I know there are ongoing partner discussions here. Once the outcome > of those discussions is known, please update the blocking flag accordingly. Partner discussions pending. Will update.
Flags: needinfo?(skamat)
Kyle, it's great to start this bug from now....:-). Could you please help? Thanks.
Flags: needinfo?(khuey)
I don't really understand what the issue here is. The bluetooth SendFile API accepts blobs, and we have a blob ... I'll be in Taipei next week so I can sit down with Yoshi or Eric or someone and figure out what is going on. Can you figure out who I should talk to?
Flags: needinfo?(khuey) → needinfo?(kchang)
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #34) > I'll be in Taipei next week so I can sit down with Yoshi or Eric or someone It's great. Welcome to Taipei. > and figure out what is going on. Can you figure out who I should talk to? Eric knows more details about this bug.
Flags: needinfo?(kchang)
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #34) > I don't really understand what the issue here is. The bluetooth SendFile > API accepts blobs, and we have a blob ... > > I'll be in Taipei next week so I can sit down with Yoshi or Eric or someone > and figure out what is going on. Can you figure out who I should talk to? No problem. Let's have a talk with me and Yoshi/Dimi next Monday.
I won't be in the office until Tuesday, but yeah, whenever.
Let's discuss if NFCPeer.sendFile is really needed first. Bug 986361.
Depends on: 986361
Blocks: b2g-NFC-2.0
No longer blocks: b2g-NFC-2.0
Getting only a Blob, and not a File, from calling slice() on a File is the expected behavior. The real question is why you are getting the security errors in comment 8 if you do not call slice(). Yoshi is going to try to reproduce and then we'll sit down and figure out what is going on in the debugger.
Flags: needinfo?(psiddh)
Hi Kyle, As you may have noticed, in nsNfc.js, we perform 'blob.slice()' and pass the blob to NfcContentHelper.js --> Nfc.js (Parent process) --> System App (nfc_handover_manager.js) --> bluetooth dom interface Bluetooth dom interface expects only blob of type 'file'. As you have also noted, 'slice' converts it to 'blob type'. Please ignore security errors in comment 8 as they were some changes which were in progress then. They may not be relevant now. I also made a quick work-around in Blob.cpp in the then code base @ http://mxr.mozilla.org/mozilla-central/source/dom/ipc/Blob.cpp#958 and changed the constructor params from 'NormalBlobConstructorParams' to 'FileBlobConstructorParams'. It seems to always pass 'NormalBlobConstructorParams' whenever 'slice()' is performed. I noticed that the System App was receiving 'blob' type and bluetooth dom interface 'sendFile' was able to honor the 'blob' and perform the handover successfully.
Flags: needinfo?(psiddh)
But why are you slicing the blob in the first place? Is it just to get around the security errors? Changing NormalBlobConstructorParams to FileBlobConstructorParams is not an acceptable fix, because it would cause us to violate the W3C File API spec, which states that slice() returns a Blob, regardless of whether it is called on a File or a Blob. We need to get rid of the slice() call.
Yes Kyle, slice() was added to get around security errors E/GeckoConsole( 1144): [JavaScript Error: "Permission denied to access property 'toString'"] E/GeckoConsole( 1144): [JavaScript Error: "Permission denied to access property 'message'"] E/GeckoConsole( 1144): [JavaScript Error: "uncaught exception: unknown (can't convert to string)"] Please suggest how the 'blob (file type)' could be relayed through nsNfc.js --> NfcContentHelper.js --> Nfc.js --> System App (gaia) ?
Kyle, I know you are busy in workweek. Could you please some updates for this bug? Thanks.
Flags: needinfo?(khuey)
Unfortunately we were not able to reproduce this today. There seems to be a gaia regression that is blocking the NFC flow or something. Yoshi has details.
Flags: needinfo?(khuey)
John, could you please check if this issue is still existence and blocks the bugs you own?
Flags: needinfo?(johu)
Flags: needinfo?(johu)
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #45) > Unfortunately we were not able to reproduce this today. There seems to be a > gaia regression that is blocking the NFC flow or something. Yoshi has > details. Yeah, I tried to reproduce it with the latest code on Tuesday, however there are several bugs. The device I use is Nexus-4, Camera app will crash, enable Bluetooth in Settings will crash, LockScreen doesn't pass unlock event (So NFC is always in low-power mode). I'll try again with today's (4/3) build.
> Camera app will crash, enable Bluetooth in Settings will crash, Ben, Could you please to check this problem as well?
Flags: needinfo?(btian)
BT won't crash on 4/3 build.
Flags: needinfo?(btian)
Found the regression in Bug 991585 happending on the receiving side.
By removing the 'slice()' call in nsNfc.js, I cannot reproduce the 'Security Error' mentioned by Sidd in Comment 43. Also, the File Transfer is working well now. I think the 'Security Error problem' is already fixed now.
\o/ Want to close this as WORKSFORME then?
We still need to remove the slice() call from nsNfc.js, I'll change assignee to Sid.
Assignee: khuey → psiddh
Sid, I set the target milestone of this bugs to be on 4/11. If it doesn't work for you, please directly update it. Thanks.
Target Milestone: --- → 1.4 S5 (11apr)
Attachment #8404341 - Flags: superreview?(bugs)
Attachment #8404341 - Flags: review?(khuey)
Comment on attachment 8404341 [details] [diff] [review] 0001-Bug-964693-Do-not-perform-slice-on-blob-as-slice-doe.patch (assuming there is no security exception which was mentioned earlier. no idea why this needs sr.)
Attachment #8404341 - Flags: superreview?(bugs) → superreview+
Keywords: checkin-needed
canceling check-in for this patch is still in r? to Kyle. Given that this patch doesn't have any (Web)IDL change so it's okay to have just one reviewer. Please update the patch to r=smaug. Or you'd like Kyle to review this, please commit this patch after Kyle reviews it, don't commit a patch while still r? to others.
Keywords: checkin-needed
Comment on attachment 8404341 [details] [diff] [review] 0001-Bug-964693-Do-not-perform-slice-on-blob-as-slice-doe.patch Review of attachment 8404341 [details] [diff] [review]: ----------------------------------------------------------------- r+ by smaug.
Attachment #8404341 - Flags: superreview+
Attachment #8404341 - Flags: review?(khuey)
Attachment #8404341 - Flags: review+
Thanks Yoshi, I will be mindful of that from next time.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Verified on pvt build (2014/4/16)
Status: RESOLVED → VERIFIED
feature-b2g: --- → 2.0
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: