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

Status: VERIFIED FIXED

(Firefox OS Graveyard :: NFC, defect)

Description

[Eric Chou [:ericchou] [:echou]]

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').

Comment 1

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Ken and Arno, 
Do you have idea who can take this bug?

Comment 2

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

Comment 3

[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 ?

Comment 4

[Ken Chang[:kenkai]]

Information has been provided.

Comment 5

[psiddh]

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)"]

Comment 6

[Eric Chou [:ericchou] [:echou]]

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

Comment 7

[Eric Chou [:ericchou] [:echou]]

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

Comment 8

[psiddh]

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

Comment 9

[psiddh]

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.

Comment 10

[psiddh]

Requesting Ben Turner to suggest on how to address the issue mentioned on comment#9

Comment 11

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Hey guys, sorry, but I'm gone until 2/14. Perhaps khuey can help?

Comment 12

[Ben Turner (not reading bugmail, use the needinfo flag!)]

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.

Comment 13

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I'm in Taipei this week and more than a little busy, but someone can come find me if they need me.

Comment 14

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Who can take this bug? 
This blocks essential scenario for upcoming NFC demo, which we really need to get it done asap.

Comment 15

[psiddh]

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 ?

Comment 16

[Ken Chang[:kenkai]]

Kyle, except you, who do we can call for help? Thanks.

Comment 17

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

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.

Comment 18

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I talked to Ken offline, this does not block the MWC demo, but it is still a priority.

Comment 19

[Ken Chang[:kenkai]]

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.

Comment 20

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Did you intend to clear the blocking flag?

Comment 21

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

Sorry, No. :P
My mistake.

Comment 22

[Ken Chang[:kenkai]]

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.

Comment 23

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

No, I don't think so.

Comment 26

[Jason Smith [:jsmith]]

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.

Comment 27

[John Hu [:johnhu][:johu][:醬糊小弟]]

move the blocking bug from duplicated bugs.

Comment 28

[Preeti Raghunath(:Preeti)]

Moving to 1.5? per de-scoped feature list in 1.4

Comment 29

[Ken Chang[:kenkai]]

Partner would like to have NFC handover mechanism ready in Gecko. Can we have this fix fixed in 1.4?

Comment 30

[Peter Dolanjski [:pdol]]

Sandip, I know there are ongoing partner discussions here.  Once the outcome of those discussions is known, please update the blocking flag accordingly.

Comment 31

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Per comment 26 NFC was cut from 1.4.

Comment 32

[Sandip Kamat]

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

Comment 33

[Ken Chang[:kenkai]]

Kyle, it's great to start this bug from now....:-). Could you please help? Thanks.

Comment 34

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

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?

Comment 35

[Ken Chang[:kenkai]]

(In reply to Kyle Huey [:khuey] ([e-mail]) 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.

Comment 36

[Eric Chou [:ericchou] [:echou]]

(In reply to Kyle Huey [:khuey] ([e-mail]) 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.

Comment 37

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I won't be in the office until Tuesday, but yeah, whenever.

Comment 38

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

Let's discuss if NFCPeer.sendFile is really needed first. Bug 986361.

Comment 39

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

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.

Comment 41

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

Comment 42

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

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.

Comment 43

[psiddh]

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) ?

Comment 44

[Ken Chang[:kenkai]]

Kyle, I know you are busy in workweek. Could you please some updates for this bug? Thanks.

Comment 45

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

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.

Comment 46

[Ken Chang[:kenkai]]

John, could you please check if this issue is still existence and blocks the bugs you own?

Comment 47

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

(In reply to Kyle Huey [:khuey] ([e-mail]) 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.

Comment 48

[Ken Chang[:kenkai]]

> Camera app will crash, enable Bluetooth in Settings will crash,
Ben, Could you please to check this problem as well?

Comment 49

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

BT won't crash on 4/3 build.

Comment 50

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

Found the regression in Bug 991585 happending on the receiving side.

Comment 51

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

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.

Comment 52

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

\o/

Want to close this as WORKSFORME then?

Comment 53

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

We still need to remove the slice() call from nsNfc.js, I'll change assignee to Sid.

Comment 54

[Ken Chang[:kenkai]]

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.

Comment 55

[psiddh]

Attachment: 0001-Bug-964693-Do-not-perform-slice-on-blob-as-slice-doe.patch

Comment 56

[Olli Pettay [:smaug][[e-mail]]]

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

Comment 57

[psiddh]

Try Server results : https://hg.mozilla.org/try/rev/61196bc0adba

Comment 58

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

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.

Comment 59

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

add r=smaug.
https://hg.mozilla.org/integration/b2g-inbound/rev/7673dac0f8f0

Comment 60

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

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.

Comment 61

[psiddh]

Thanks Yoshi, I will be mindful of that from next time.

Comment 62

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7673dac0f8f0

Comment 63

[Alison Shiue]

Verified on pvt build (2014/4/16)

Comment 64

[Alison Shiue]

https://moztrap.mozilla.org/manage/cases/?pagenumber=1&pagesize=20&sortfield=created_on&sortdirection=desc&filter-tag=2288&filter-suite=419