933093 - [Gecko] Add Handover DOM API to NFC

Status: RESOLVED FIXED

(Firefox OS Graveyard :: NFC, defect)

Description

[Garner Lee]

Related to Bug 903259 (Gaia):

NFCPeer needs a new API to handle handover requests. NFC is used for setting up the connection, but does not do the actual transfers. The Pairing is done by tapping devices together and exchanging relevant bits of connection data (hardware address), and the specific case of file transfer is done by transferring a data blob from one device to another over that static or negotiated connection.

From a discussion:

> 1. BT in gecko needs a new pairing API which argument is the
> address(currently it's the device).
> 2. BT app in gaia needs some change to activity message handler to deal with
> background launched case.
> 3. ActivityWindowFactory/ActivityWindow needs to manipulate the new
> disposition and launches the activity in background.
> 4. WebNFC needs some new API: sendFile, handover?.
> 5. NFCManager in gaia::system needs some change.
>

Proposed usage:
window.navigator.mozNfc.onpeerfound = function(nfcPeer) {
  // Negotiates WiFi or BT connection, presenting UI 
  DOMRequest r = nfcPeer.sendFile(blob); 
  r.onsuccess = function(e) {
    // Transfer successfully completed, update UI
  };
  r.onerror = function(e) {
    // Transfer did not succeed, update UI 
  };
};

Comment 1

[(inactive after 6/18) Alive Kuo [:alive]]

Proposal for file transfer under webNFC:
https://docs.google.com/document/d/1X00d3aB5ZltWfaqbFEWtB_h_fFpHAQUXVgiyh9yxeMg/edit?usp=sharing

Comment 2

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

Just filed a bug for point 1. (bug 933113)

Comment 3

[(inactive after 6/18) Alive Kuo [:alive]]

About the sendFile API, refer this
http://mxr.mozilla.org/mozilla-central/source/dom/bluetooth/BluetoothAdapter.h#128
http://mxr.mozilla.org/mozilla-central/source/dom/webidl/BluetoothAdapter.webidl#118

Comment 4

[Ken Chang[:kenkai]]

I am sure it's 1.3+. The target milestone should be 11/8. If you don't agree this date, please provide your suggestion.

Comment 5

[Ken Chang[:kenkai]]

Garner, when can you provide a patch for this bug?

Comment 6

[Garner Lee]

^--Sid?

Comment 7

[Garner Lee]

Actually, Arno is working on the DOM part. Static handover (bluetooth headset) may be done this week. 

We'd need some BT APIs to actually negotiate the handover transfer connection (Eric will provide API?). A negotiated handover (NDEF messages between devices to determine a common transfer connection) may take longer.

The  MozNFCPeer.sendNDEF(ndefrecs) API has fewer dependencies, and should start to be integrated into the apps once the DOM lands in Bug 674741 (which blocks Bug 933136).

Comment 8

[Ken Chang[:kenkai]]

(In reply to Garner Lee from comment #7)
> Actually, Arno is working on the DOM part. Static handover (bluetooth
> headset) may be done this week. 
Should we assign this bug to Arno...:)

Comment 9

[Garner Lee]

(In reply to Ken Chang from comment #8)
> Should we assign this bug to Arno...:)
Assigning to Arno :)

Comment 10

[arno]

I've just added a dependency to Bug 903305. The static handover is so closely related to the negotiated handover (in terms of PDU exchange) that 903305 needs to land first. Once that happened, implementing the DOM API should be easy.

Comment 11

[arno]

Eric: what API do I need to use for incoming file transfers assuming I have the BT MAC address from the sending side and the sending side initiated the handover?

Comment 12

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

(In reply to arno from comment #11)
> Eric: what API do I need to use for incoming file transfers assuming I have
> the BT MAC address from the sending side and the sending side initiated the
> handover?

First, please listen to the following system messages:

  'bluetooth-opp-receiving-file-confirmation',
  'bluetooth-opp-transfer-start',
  'bluetooth-opp-update-progress',
  'bluetooth-opp-transfer-complete'

Gaia Reference: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/bluetooth_transfer.js

****************************

bluetooth-opp-transfer-start

= Description = 
Indicates that sending / receiving process has started

= Parameter  =
address : [string] Bluetooth remote device address
received : [bool] true - receiving, false - sending
fileName : [string] file name
fileLength : [uint32_t] file length (if the length is 0 it means unknown length)
contentType : [string] File MIME Type (if it's empty it means the MIME type is not available)

****************************

bluetooth-opp-transfer-complete

= Description = 
Indicates that sending / receiving process has ended

= Parameter  =
address : [string] Bluetooth remote device address
success : [bool] true - success, false - fail (for some reason)
received : [bool] true - receiving, false - sending
fileName : [string] file name
fileLength : [uint32_t] file length (if the length is 0 it means unknown length)
contentType : [string] File MIME Type (if it's empty it means the MIME type is not available)

****************************

bluetooth-opp-update-progress

= Description = 
Would be sent when sending / receiving every 50 kb

= Parameter  =
address : [string] Bluetooth remote device address
received : [bool] true - receiving, false - sending
processedLength : [uint32_t] the length of file content which has been sent.
fileLength : [uint32_t] file length (if the length is 0 it means unknown length)

****************************

bluetooth-opp-receiving-file-confirmation

= Description = 
Indicates that we have received a "file pushing" request from a remote Bluetooth device.

After this system message was sent, Gecko would be blocked until Gaia calls

  defaultAdapter.confirmReceivingFile([remote address], [accept?]);

= Parameter  =
address : [string] Bluetooth remote device address
fileName : [string] file name
fileLength : [uint32_t] file length (if the length is 0 it means unknown length)
contentType : [string] File MIME Type (if it's empty it means the MIME type is not available)

*****************************

Please note that Bluetooth file transfer can't work at all on Nexus 4 for now (bug 915533). Users are not only unable to share files but also unable to receive files.

Comment 13

[arno]

Attachment: Implementation of file transfer via NFC Handover

This patch extends the BT Pairing implementation of HandoverManager by adding an implementation for file transfer. This patch only adds to nfc_manager.js and nfc_handover.js. A separate patch for wiring this up with the DOM API will be submitted.

Comment 14

[Evelyn Hung]

Comment on attachment 8339220 [details] [review]
Implementation of file transfer via NFC Handover

r-
1. Please rebase your PR first, leave changes handling file transfer and remove irrelevant changes since they are belongs to bug 903305.
2. Why don't you utilize bluetooth file transfer app?

Comment 15

[arno]

(In reply to Evelyn Hung [:evelyn] from comment #14)
> Comment on attachment 8339220 [details] [review]
> Implementation of file transfer via NFC Handover
> 
> r-
> 1. Please rebase your PR first, leave changes handling file transfer and
> remove irrelevant changes since they are belongs to bug 903305.

will do.

> 2. Why don't you utilize bluetooth file transfer app?

I'm using the API that Eric communicated to me. Note that a file transfer via a NFC handover will require modifications to apps that can send files (video, images, etc). The apps basically register for onpeerready() and then do a sendFile() in the callback in response to the P2P "shrinking UI". This might be the reason Eric hasn't mentioned the bluetooth file transfer app.

Comment 16

[arno]

Eric: can you please respond to Evelyn's comment #14 and my answer in #15?

Comment 17

[arno]

Attachment: Implementation of HandoverManager for file transfer

This pull request contains the changes to HandoverManager to handle a negotiated handover via NFC. Since it is based on the initial version of HandoverManager that is still under review (bug 903305) there are two commits in this pull request.

Comment 18

[Jason Smith [:jsmith]]

NFC isn't a committed feature for 1.3, so this should not be blocking the release. It's only targeted.

Comment 19

[Ken Chang[:kenkai]]

Hi Evenly, Thanks for your suggestion. I wonder if it is possible to use the mechanism of what Arno provide for BT file transfer currently. And we can use the 933116 as a followup bug to improve this design later. What do you think?

Comment 20

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

Hi Arno,

> > 2. Why don't you utilize bluetooth file transfer app?
> 
> I'm using the API that Eric communicated to me. Note that a file transfer
> via a NFC handover will require modifications to apps that can send files
> (video, images, etc). The apps basically register for onpeerready() and then
> do a sendFile() in the callback in response to the P2P "shrinking UI". This
> might be the reason Eric hasn't mentioned the bluetooth file transfer app.

I did mention Bluetooth file transfer app, which is a centralized app used
for Bluetooth file transfer in FxOS system. There are two ways to do Bluetooth
file transfer. The first one is handling all events and call BT functions 
in NFCManager, and the other is NFC telling BT app which device should the 
file be sent to so that BT app can deal with all BT stuff for you. The main
problem of the latter function would be the interface of NFC Manager and BT
app has to be designed well, so that BT app can know 'which device should I
send to (also remember not showing UI for user to choose a device)', 'which
file is going to be sent', etc. I wouldn't say if the first way is better or
not since I'm not a Gaia professional, however both should be able to work.

Comment 21

[Evelyn Hung]

(In reply to Ken Chang from comment #19)
> Hi Evenly, Thanks for your suggestion. I wonder if it is possible to use the
> mechanism of what Arno provide for BT file transfer currently. And we can
> use the 933116 as a followup bug to improve this design later. What do you
> think?

I'd like to have a discussion with Alive and Ian (BT app owner). I don't think redo every BT file transfer stuff here will be easier than reuse BT app. 

BTW, so is this bug for gecko implementation or Gaia? I'm confused by bug summary. :-(

Comment 22

[Evelyn Hung]

Comment on attachment 8340740 [details] [review]
Implementation of HandoverManager for file transfer

redirect to Ian who knows more BT file transfer things than me. Ian will review your patch per our discussion on "reusing BT app or not".

Comment 23

[[e-mail], [e-mail]]

Comment on attachment 8340740 [details] [review]
Implementation of HandoverManager for file transfer

Hi Arno,

As previous discussion with Evelyn, Alive, and Ken, we agree and suggest to do sending file via BT in system app. And we will help to guide the implementation in your patch. We won't pass the sending file request to Bluetooth App via web activity.(bug 933116)


I have a quickly review for send/receive file via bluetooth section. I think the architecture should be as following.

* Send file
  - You could provide a send file API in system/js/bluetooth_transfer.js(https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/bluetooth_transfer.js). 
  - And it will need to do the same procedure for fire a fake event like "iac-bluetoothTransfercomms". In fact, we could call onFileSending() directly while NFC module create the sending file request. Then, the function onFilesSending() will do follow up sending file flow. Otherwise, you will break the sending file queue. And the notification message won't be pop out in this case.

* Receive file
  - You have to identify the incoming file which is come from NFC or not. If yes, we should not pop out a confirmation dialog. Because we have no way to identify this case via bluetooth system message "bluetooth-opp-receiving-file-confirmation". I think you have to maintain a flag or mode to check the situation. And confirm the incoming file request directly in function onReceivingFileConfirmation().(https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/bluetooth_transfer.js#L149) Once you do confirmReceivingFile()(https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/bluetooth_transfer.js#L205), the UI flow will sync with original BT file transfer.

Please revise the patch to sync the architecture. And set the flag to me again when your patch is ready. Thank you so much.

Comment 24

[arno]

(In reply to Ian Liu [:ianliu] from comment #23)
> * Send file
>   - You could provide a send file API in
> system/js/bluetooth_transfer.js(https://github.com/mozilla-b2g/gaia/blob/
> master/apps/system/js/bluetooth_transfer.js). 
>   - And it will need to do the same procedure for fire a fake event like
> "iac-bluetoothTransfercomms". In fact, we could call onFileSending()
> directly while NFC module create the sending file request. Then, the
> function onFilesSending() will do follow up sending file flow. Otherwise,
> you will break the sending file queue. And the notification message won't be
> pop out in this case.

Two quick questions on the sending file side:
(1) for onFilesSending(evt), can you please tell me the properties I need to set in 'evt' to do the fake event?
(2) do I still need to call DefaultAdapter.sendFile() or is this done implicitly somewhere else?

> * Receive file
> [...]

I got this to work by making the modifications you suggested. :)

Comment 25

[[e-mail], [e-mail]]

(In reply to arno from comment #24)
> (In reply to Ian Liu [:ianliu] from comment #23)
> > * Send file
> >   - You could provide a send file API in
> > system/js/bluetooth_transfer.js(https://github.com/mozilla-b2g/gaia/blob/
> > master/apps/system/js/bluetooth_transfer.js). 
> >   - And it will need to do the same procedure for fire a fake event like
> > "iac-bluetoothTransfercomms". In fact, we could call onFileSending()
> > directly while NFC module create the sending file request. Then, the
> > function onFilesSending() will do follow up sending file flow. Otherwise,
> > you will break the sending file queue. And the notification message won't be
> > pop out in this case.
> 
> Two quick questions on the sending file side:
> (1) for onFilesSending(evt), can you please tell me the properties I need to
> set in 'evt' to do the fake event?
You could follow up "sendingFilesSchedule" object(https://github.com/mozilla-b2g/gaia/blob/master/apps/bluetooth/js/transfer.js#L145) for the evt parameter.
> (2) do I still need to call DefaultAdapter.sendFile() or is this done
> implicitly somewhere else?
You have to expose(implement) the "DefaultAdapter.sendFile()" API in the object(https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/bluetooth_transfer.js#L8). And reuse Bluetooth.getAdapter() again. Then, I suppose the wrapper is ready, you will call BluetoothTransfer.sendFile() and give the "sendingFilesSchedule" object in the same time. That would be more maintainable in the future.
> 
> > * Receive file
> > [...]
> 
> I got this to work by making the modifications you suggested. :)
Nice catch:)

Comment 26

[arno]

(In reply to Ian Liu [:ianliu] from comment #25)
> > Two quick questions on the sending file side:
> > (1) for onFilesSending(evt), can you please tell me the properties I need to
> > set in 'evt' to do the fake event?
> You could follow up "sendingFilesSchedule"
> object(https://github.com/mozilla-b2g/gaia/blob/master/apps/bluetooth/js/
> transfer.js#L145) for the evt parameter.

Thanks. But how do I get the filename? Blob does not seem to have a property for that and in transfer.js there seem to be two different sources for the filename and the blob: activity.source.data.filenames (line 146) and activity.source.data.blob (line 153). Note that in the NFC API there is only a sendFile(blob) (with no explicit filename parameter).

> > (2) do I still need to call DefaultAdapter.sendFile() or is this done
> > implicitly somewhere else?
> You have to expose(implement) the "DefaultAdapter.sendFile()" API in the
> object(https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/
> bluetooth_transfer.js#L8). And reuse Bluetooth.getAdapter() again. Then, I
> suppose the wrapper is ready, you will call BluetoothTransfer.sendFile() and
> give the "sendingFilesSchedule" object in the same time. That would be more
> maintainable in the future.

For the BT pairing implementation (bug 903305) I listen to 'adapteradded' events to retrieve the DefaultAdapter. This patch has been r+'ed and serves as the foundation for this bug. I hope you will accept that solution for now.

Comment 27

[[e-mail], [e-mail]]

(In reply to arno from comment #26)
> (In reply to Ian Liu [:ianliu] from comment #25)
> > > Two quick questions on the sending file side:
> > > (1) for onFilesSending(evt), can you please tell me the properties I need to
> > > set in 'evt' to do the fake event?
> > You could follow up "sendingFilesSchedule"
> Thanks. But how do I get the filename? Blob does not seem to have a property
> for that and in transfer.js there seem to be two different sources for the
> filename and the blob: activity.source.data.filenames (line 146) and
> activity.source.data.blob (line 153). Note that in the NFC API there is only
> a sendFile(blob) (with no explicit filename parameter).
> 
Looks like you're not able to get the filename. The the filenames are useless in transfer.js. I create a bug 946134 for removing the property. And will pass the number of files(numberOfFiles) for reference count. The patch is in reviewing process. You could suppose to base on the patch and pass the number of files for it.(https://bugzilla.mozilla.org/show_bug.cgi?id=946134#c1)
> > > (2) do I still need to call DefaultAdapter.sendFile() or is this done
> > > implicitly somewhere else?
> > You have to expose(implement) the "DefaultAdapter.sendFile()" API in the
> > object(https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/
> > bluetooth_transfer.js#L8). And reuse Bluetooth.getAdapter() again. Then, I
> > suppose the wrapper is ready, you will call BluetoothTransfer.sendFile() and
> > give the "sendingFilesSchedule" object in the same time. That would be more
> > maintainable in the future.
> 
> For the BT pairing implementation (bug 903305) I listen to 'adapteradded'
> events to retrieve the DefaultAdapter. This patch has been r+'ed and serves
> as the foundation for this bug. I hope you will accept that solution for now.
For reusing adapter, if Evelyn has reviewed it in part of pairing device, that would be okay for me. :) Thanks.

Comment 28

[[e-mail], [e-mail]]

Clear the ni flag via comment 27.

Comment 29

[arno]

Ben: I'm having some issues with automatic pairing of two devices during a file transfer via a NFC handover. The NFC handover exchanges the MAC addresses between the two devices. On FFOS, I call adapter.pair(remote_mac), but I still get the pairing dialog on both FFOS and the Android device (that is trying to send a file). Note that when you do the same between two Android devices, the pairing happens implicitly without a pairing dialog popping up. Here is how you can reproduce it:

- Gaia: https://github.com/svic/gaia/tree/handover-devel
- Gecko: https://github.com/svic/mozilla-central/tree/master
- After flashing the device, go to Settings and enable NFC
- On the FFOS, unlock screen and stay on homescreen
- On an Android device, go to Gallery and open any picture.
- Approach the two devices back-to-back. Android will do the "shrinking UI"
- On the Android device, tap on the "shrinking UI". This will trigger the file transfer

When you do this, you will note that both FFOS and Android show the pairing dialog (this shouldn't happen). Here is the location where the HandoverManager triggers the pairing after the MAC addresses have been exchanged between the two devices (via appropriate NFC NDEF messages):
https://github.com/svic/gaia/blob/handover-devel/apps/system/js/nfc_handover.js#L597

Comment 30

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

Attachment: Insecure-Socket-for-NFC-pairing-temp-linkkey.patch

Hi Arno,

Please apply my patch to your codebase. Pairing UI shouldn't appear afterwards.

Please let me know if it works. Thank you.

Comment 31

[arno]

(In reply to Eric Chou [:ericchou] [:echou] from comment #30)
> Created attachment 8342998 [details] [diff] [review]
> Insecure-Socket-for-NFC-pairing-temp-linkkey.patch
> 
> Hi Arno,
> 
> Please apply my patch to your codebase. Pairing UI shouldn't appear
> afterwards.
> 
> Please let me know if it works. Thank you.

Eric: yes, that works. For incoming file transfers I no longer get the pairing dialog. However, is it possible that a similar patch is needed for outgoing file transfers? When FFOS is the initiator, I once again get the pairing request.

Comment 32

[arno]

Attachment: Support for file transfer in HandoverManager

This patch adds support for file transfer to HandoverManager. It uses the BT file transfer app to handle the request. You can test incoming file transfers by using an Android device and beaming an image to the FFOS device. The sending side requires the DOM API to be wired up with HandoverManager.handleFileTransfer(). That patch will be provided separately.

Comment 33

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

(In reply to arno from comment #31)
> (In reply to Eric Chou [:ericchou] [:echou] from comment #30)
> > Created attachment 8342998 [details] [diff] [review]
> > Insecure-Socket-for-NFC-pairing-temp-linkkey.patch
> > 
> > Hi Arno,
> > 
> > Please apply my patch to your codebase. Pairing UI shouldn't appear
> > afterwards.
> > 
> > Please let me know if it works. Thank you.
> 
> Eric: yes, that works. For incoming file transfers I no longer get the
> pairing dialog. However, is it possible that a similar patch is needed for
> outgoing file transfers? When FFOS is the initiator, I once again get the
> pairing request.

Sure, I'll make both outgoing and incoming work. Since BT file transfer app on Android uses temp link key, we have to do the same thing to avoid prompting pairing dialog, which is sad.

I'll update bug number here once the it's filed.

Comment 34

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

(In reply to Eric Chou [:ericchou] [:echou] from comment #33)
> (In reply to arno from comment #31)
> > (In reply to Eric Chou [:ericchou] [:echou] from comment #30)
> > > Created attachment 8342998 [details] [diff] [review]
> > > Insecure-Socket-for-NFC-pairing-temp-linkkey.patch
> > > 
> > > Hi Arno,
> > > 
> > > Please apply my patch to your codebase. Pairing UI shouldn't appear
> > > afterwards.
> > > 
> > > Please let me know if it works. Thank you.
> > 
> > Eric: yes, that works. For incoming file transfers I no longer get the
> > pairing dialog. However, is it possible that a similar patch is needed for
> > outgoing file transfers? When FFOS is the initiator, I once again get the
> > pairing request.
> 
> Sure, I'll make both outgoing and incoming work. Since BT file transfer app
> on Android uses temp link key, we have to do the same thing to avoid
> prompting pairing dialog, which is sad.
> 
> I'll update bug number here once the it's filed.

I'll resolve bug 947060 as soon as possible.

Comment 35

[[e-mail], [e-mail]]

Comment on attachment 8343207 [details] [review]
Support for file transfer in HandoverManager

I have reviewed send file section via bluetooth. And leave some comment on Github. For receiving file section, it's working fine. For sending file, please check them again. It's better to have follow up patch for organisation these modules. Once you have revised it, please set review again for me. Thanks.

Comment 36

[arno]

Attachment: Support for file transfer in HandoverManager

Fixed all of Ian's Github comments.

Comment 37

[Garner Lee]

Question: What's the performance like for sendFile? When passing blob, is that raw data IPC copied between processes/apps/chrome?

Comment 38

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

(In reply to Garner Lee from comment #37)
> Question: What's the performance like for sendFile? When passing blob, is
> that raw data IPC copied between processes/apps/chrome?

Passing blob does not pass the whole file content between different process. Only necessary information would be passed, otherwise it may be a super heavy operation.

We don't have performance analysis about sending/receiving files via Bluetooth.

Comment 39

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

(In reply to Eric Chou [:ericchou] [:echou] from comment #38)
> (In reply to Garner Lee from comment #37)
> > Question: What's the performance like for sendFile? When passing blob, is
> > that raw data IPC copied between processes/apps/chrome?
> 
> Passing blob does not pass the whole file content between different process.
> Only necessary information would be passed, otherwise it may be a super
> heavy operation.
> 
> We don't have performance analysis about sending/receiving files via
> Bluetooth.

To elaborate slightly, it depends on where the data for the blob lives.  If it lives in a file on disk we just duplicate the file descriptor and hand that over with some meta data.  If the file lives in memory we do copy the data and send that over the wire.

Comment 40

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

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #39)
> (In reply to Eric Chou [:ericchou] [:echou] from comment #38)
> > (In reply to Garner Lee from comment #37)
> > > Question: What's the performance like for sendFile? When passing blob, is
> > > that raw data IPC copied between processes/apps/chrome?
> > 
> > Passing blob does not pass the whole file content between different process.
> > Only necessary information would be passed, otherwise it may be a super
> > heavy operation.
> > 
> > We don't have performance analysis about sending/receiving files via
> > Bluetooth.
> 
> To elaborate slightly, it depends on where the data for the blob lives.  If
> it lives in a file on disk we just duplicate the file descriptor and hand
> that over with some meta data.  If the file lives in memory we do copy the
> data and send that over the wire.

Clearer. Thanks, Kyle.

Comment 41

[Garner Lee]

Thanks Eric, Kyle. Makes sense.

Comment 42

[[e-mail], [e-mail]]

Comment on attachment 8344231 [details] [review]
Support for file transfer in HandoverManager

Looks like you have migrated the Bluetooth sendFile API to bluetooth_transfer.js. That would be maintainable in the future. Good work:)

For the 'this' code pattern, I think you have revised. We have too many times to access 'self' instead of 'this'. That will make the function scoop un-maintainable. Also leave some suggestion on Github. Thanks.

Comment 43

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

Defer to 1.4+ per triage meeting.

Comment 44

[[e-mail], [e-mail]]

Hi Arno,

According to bug 948874(https://bugzilla.mozilla.org/show_bug.cgi?id=948874#c0), we did not receive any event in nfc_handover.js. with debug=true. Is there any other blocking in NFC manager or Gecko error? Thanks.

Comment 45

[arno]

Attachment: Support for file transfer in HandoverManager

Fixes for all of Ian's comments. Amongst others, I've completely removed 'self'. I personally find the code less readable now, but no more 'self'. :)

Comment 46

[[e-mail], [e-mail]]

(In reply to arno from comment #45)
> Fixes for all of Ian's comments. Amongst others, I've completely removed
> 'self'. I personally find the code less readable now, but no more 'self'. :)
Thanks for your revising effort. But you use constructor to define many methods/functions. It will make the performance lower. I leave some comment on Github.

Comment 47

[[e-mail], [e-mail]]

Comment on attachment 8346371 [details] [review]
Support for file transfer in HandoverManager

For Bluetooth file transfer section, that is okay for me. But the whole "HandoverManager" module, I recommend Alive to be the reviewer. He is the system app peer. Pass the decision to him.

Comment 48

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8346371 [details] [review]
Support for file transfer in HandoverManager

Canceling, please set review to evelyn since she reviewed this.

My suggestions:
1. Try not to have a big constructor function. Use prototype.
2. Try not to put different module in different file, e.g. Buffer constructor.
3. Remove redundant codes. (parse). It would ease your life when you want to move them to gecko.
4. Unit test is highly recommended.

Comment 49

[arno]

Attachment: Support for file transfer in HandoverManager

Evelyn: it seems the hot potato has come back to you! In this pull request I reverted some changes where I misunderstood Ian. I addressed all of his comments based on my interpretation of his feedback. Lets take this from the top and discuss the changes you would like to have done. One note on an earlier question/comment from you: this bug is about adding new DOM API for sending a blob. This also requires changes to the Gaia system app where the handover is handled. Sid will very soon upload the matching Gecko patches. If you prefer, I will create a new bug just for the Gaia changes with the next iteration of your feedback.

Comment 50

[Ken Chang[:kenkai]]

It is not 1.3 now.

Comment 51

[psiddh]

Attachment: (v1) Part 1: SendFile DOM API

Add support for SendFile Nfc DOM API

Comment 52

[psiddh]

Attachment: (v1) Part 2: SendFile support in chrome process

Comment 53

[psiddh]

Attachment: (v1) Part 3: New System message to initiate BT send-file

Comment 54

[arno]

Eric: we have two issues with the BT handover:
(1) the patch for bypassing the pairing dialog does not seem to work. Even with the patch applied, we get the pairing dialog for outgoing file transfers.
(2) we manage to crash the device when trying defaultAdapter.sendFile(mac, new Blob([], {type: 'image/png'}));

Comment 55

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

Hi Arno,

(In reply to arno from comment #54)
> Eric: we have two issues with the BT handover:
> (1) the patch for bypassing the pairing dialog does not seem to work. Even
> with the patch applied, we get the pairing dialog for outgoing file
> transfers.

Please rebase your branch onto the latest m-c, or you can cherry-pick the patch of bug 947060 manually. (https://hg.mozilla.org/mozilla-central/rev/c1a13692027d)

> (2) we manage to crash the device when trying defaultAdapter.sendFile(mac,
> new Blob([], {type: 'image/png'}));

It's because Gecko Bluetooth only accepts nsIDOMFile object (http://mxr.mozilla.org/mozilla-central/source/content/base/public/nsIDOMFile.idl#30). So please pick an existing file from storage and give it another try. It should just work.

Comment 56

[Evelyn Hung]

(In reply to arno from comment #49)
> Created attachment 8346667 [details] [review]
> Support for file transfer in HandoverManager
> 
> Evelyn: it seems the hot potato has come back to you! In this pull request I
> reverted some changes where I misunderstood Ian. I addressed all of his
> comments based on my interpretation of his feedback. Lets take this from the
> top and discuss the changes you would like to have done. One note on an
> earlier question/comment from you: this bug is about adding new DOM API for
> sending a blob. This also requires changes to the Gaia system app where the
> handover is handled. Sid will very soon upload the matching Gecko patches.
> If you prefer, I will create a new bug just for the Gaia changes with the
> next iteration of your feedback.

I'm reviewing.. and yes, please file a bug for Gaia changes. Thanks.

Comment 57

[Evelyn Hung]

Quick feedback:

I don't want to be picky but Alive is not happy to see there is a function with 700-line code. I'm so sorry I didn't point this out when I was reviewing bug 903305, because I thought we were in a tight schedule. Considering unit test, it's better to use object pattern and move NdefHandoverCodec and rest parsing things out of HandoverManager. We should also move NdefUtils out and even remove redundant code in NFCManager. 

To be concrete:
1. move NdefHandoverCodec and rest parsing things out
2. move NdefUtils out, and file a follow-up bug for remove redundant code in NFCManager. (or do it in this patch, I'm fine)
3. use object pattern for HandoverManager. It's better for unit test, and also align with NFCManager. 
4. file bugs for adding unit test.

Please also checkout comment 48, I'm echoing Alive's opinion.

Comment 58

[Evelyn Hung]

Comment on attachment 8346667 [details] [review]
Support for file transfer in HandoverManager

I think it's good at BT file transfer part. We are looking for a code structure re-org to make the flow more clear and testable.

Comment 59

[Evelyn Hung]

(In reply to Evelyn Hung [:evelyn] from comment #56)
> (In reply to arno from comment #49)
> > Created attachment 8346667 [details] [review]
> > Support for file transfer in HandoverManager
> > 
> > Evelyn: it seems the hot potato has come back to you! In this pull request I
> > reverted some changes where I misunderstood Ian. I addressed all of his
> > comments based on my interpretation of his feedback. Lets take this from the
> > top and discuss the changes you would like to have done. One note on an
> > earlier question/comment from you: this bug is about adding new DOM API for
> > sending a blob. This also requires changes to the Gaia system app where the
> > handover is handled. Sid will very soon upload the matching Gecko patches.
> > If you prefer, I will create a new bug just for the Gaia changes with the
> > next iteration of your feedback.
> 
> I'm reviewing.. and yes, please file a bug for Gaia changes. Thanks.

Since the whole conversation has happened here and we are almost done, I'm fine *without* a separate bug for gaia. Please ignore my comment 56.

Comment 60

[arno]

Attachment: Support for file transfer in HandoverManager

I've done a major refactoring of the code, basically changing its structure back to when you told me not to clutter the global namespace. I've created one global object NfcUtil that now has all NDEF/Handover codecs. Please take a look at NfcUtil.createBuffer(). I didn't know a better way to do without adding more to the global namespace. I've not changed NFC Manager beyond what I had to do for this bug. We already filed bug 943691 a while ago. I will clean up NFC Manager once this bug lands.

Comment 61

[psiddh]

Hi Eric, 
To follow up on Arno's question, we still have same two issues.

Issue#1: We are still not able to create a proper File blob. B2G still crashes! (Perhaps, you may want to file a security bug to track this). Here is how we pick a file blob for testing purposes.
Can you tell us anything is wrong with this? 

var pick = new MozActivity({
     name: "pick",
     data: {
       type: ["image/png", "image/jpg", "image/jpeg"]
       }
   });
pick.onsuccess = function () {

  var fakeBlob = this.result.blob;
  var req = nfcPeer.sendFile(fakeBlob);
}

Issue#2 : We still get the 'pairing dialog' for outgoing pairing transfer. Please note that we do have the following patch in our code base: https://hg.mozilla.org/mozilla-central/rev/c1a13692027d

Comment 62

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

Hi Sid,

(In reply to Siddartha P from comment #61)
> Hi Eric, 
> To follow up on Arno's question, we still have same two issues.
> 
> Issue#1: We are still not able to create a proper File blob. B2G still
> crashes! (Perhaps, you may want to file a security bug to track this). Here
> is how we pick a file blob for testing purposes.
> Can you tell us anything is wrong with this? 
> 
> var pick = new MozActivity({
>      name: "pick",
>      data: {
>        type: ["image/png", "image/jpg", "image/jpeg"]
       }
>    });
> pick.onsuccess = function () {

>   var fakeBlob = this.result.blob;
>   var req = nfcPeer.sendFile(fakeBlob);
> }
> 

ok, I'll take a look today.

> Issue#2 : We still get the 'pairing dialog' for outgoing pairing transfer.
> Please note that we do have the following patch in our code base:
> https://hg.mozilla.org/mozilla-central/rev/c1a13692027d

Hm, then could you please try to apply the following patch?

diff --git a/dom/bluetooth/bluedroid/BluetoothOppManager.cpp b/dom/bluetooth/bluedroid/BluetoothOppManager.cpp
--- a/dom/bluetooth/bluedroid/BluetoothOppManager.cpp
+++ b/dom/bluetooth/bluedroid/BluetoothOppManager.cpp
@@ -263,17 +263,17 @@ BluetoothOppManager::ConnectInternal(con
 
   BluetoothService* bs = BluetoothService::Get();
   if (!bs || sInShutdown || mSocket) {
     OnSocketConnectError(mSocket);
     return;
   }
 
   mSocket =
-    new BluetoothSocket(this, BluetoothSocketType::RFCOMM, false, true);
+    new BluetoothSocket(this, BluetoothSocketType::RFCOMM, false, false);
   mSocket->Connect(aDeviceAddress, -1);
 }
 
 void
 BluetoothOppManager::HandleShutdown()
 {
   MOZ_ASSERT(NS_IsMainThread());
   sInShutdown = true;

Comment 63

[Evelyn Hung]

Comment on attachment 8348443 [details] [review]
Support for file transfer in HandoverManager

The code is clear to me now, thanks! r+ with the file name nit addressed. Do we have unit-test bugs?

Comment 64

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

Comment on attachment 8347823 [details] [diff] [review]
(v1) Part 2: SendFile support in chrome process

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

See https://bugzilla.mozilla.org/show_bug.cgi?id=948850#c1,
please update the documentation first.

I already got some Gaia developers asking me about your API.

Also try to add some diagrams here for better understanding,
which app will call sendFile, then which app will receive the system message.

Clear r? for the questions for blob and requestId.

::: dom/system/gonk/Nfc.js
@@ +295,5 @@
>            return null;
>          }
>  
>          // Add extra permission check for two IPC Peer events:
>          // 'NFC:CheckP2PRegistration' , 'NFC:NotifyUserAcceptedP2P'

update comment here.

@@ +581,5 @@
>          this.sendToWorker("close", message.json);
>          break;
> +      case "NFC:SendFile":
> +        // Chrome process is the arbitrator / mediator between Content process
> +        // that issued nfc 'sendFile' operation and system-process (content)

I guess you want to say
"system app (content process)"?

::: dom/system/gonk/NfcContentHelper.js
@@ +216,5 @@
>      });
>      return request;
>    },
>  
> +  sendFile: function sendFile(window, data, sessionToken) {

data? I thought it was 'blob'.

@@ +228,5 @@
> +
> +    cpmm.sendAsyncMessage("NFC:SendFile", {
> +      requestId: requestId,
> +      sessionToken: sessionToken,
> +      blob: data.blob

data.blob?

Can you elaborate the dict for the blob in the IDL?

::: dom/system/gonk/nsINfcContentHelper.idl
@@ +55,5 @@
> +  * Returns DOMRequest, if initiation of send file operation is successful
> +  * then 'onsuccess' is called else 'onerror'
> +  */
> +  nsIDOMDOMRequest sendFile(in nsIDOMWindow window,
> +                            in jsval aBlob,

blob, unless you want to rename all other arguments to start with 'a'.

@@ +123,5 @@
> +  * @param window
> +  *        Current window
> +  *
> +  * @param status
> +  *        Status of sendFile operation (SUCCESS : 0 / FAILURE: 1)

const these two intergers, as you did for NFC_EVENT_PEER_READY/LOST

@@ +130,5 @@
> +  *        Request ID of SendFile DOM Request
> +  */
> +  void notifySendFileStatus(in nsIDOMWindow window,
> +                            in octet status,
> +                            in DOMString requestId);

What's the requestId here?

Where does this come from?

Comment 65

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

Hi Arno,

> (2) we manage to crash the device when trying defaultAdapter.sendFile(mac,
> new Blob([], {type: 'image/png'}));

I just tried this command and no crash occurred on my device. In addition, I'd like to correct what I said in comment 55, Gecko Bluetooth is capable of dealing with both Blob and File. During my test, I did find a problem which is that the OBEX session won't get released if you send a blob with length 0. I'll take time fixing this but I don't think this will block NFC implementation. All you have to do is create a blob with content. For instance, you could start from sending a txt file which contains string 'hello world':

+    var blob = new Blob(['hello', ' world'], {type: 'text/plain'});
+    defaultAdapter.sendFile([mac], blob);

It works on my device. Of course you could also change the type to 'image/png', create a valid .png file and send. Would you mind trying again?

Comment 66

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

So why do we add nfc-send-file-status event?
Couldn't we expose just some API to the system app to send file?

(Actually, the same question for nfc-p2p-user-accept)

Comment 67

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

Comment on attachment 8347821 [details] [diff] [review]
(v1) Part 1: SendFile DOM API

...that would make using the API simpler.

Comment 68

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

(In reply to Eric Chou [:ericchou] [:echou] from comment #55)
> Hi Arno,
> 
> (In reply to arno from comment #54)
> > Eric: we have two issues with the BT handover:
> > (1) the patch for bypassing the pairing dialog does not seem to work. Even
> > with the patch applied, we get the pairing dialog for outgoing file
> > transfers.
> 
> Please rebase your branch onto the latest m-c, or you can cherry-pick the
> patch of bug 947060 manually.
> (https://hg.mozilla.org/mozilla-central/rev/c1a13692027d)
> 
> > (2) we manage to crash the device when trying defaultAdapter.sendFile(mac,
> > new Blob([], {type: 'image/png'}));
> 

Update: bug 915602 seems like this. We'll start to investigate why it would crash. To avoid this, you could try
  
  1) make devices paired with each other before transferring files

     or

  2) set a 3 sec timeout before calling sendFile(), just like what bluetooth app does now
     (https://github.com/mozilla-b2g/gaia/blob/master/apps/bluetooth/js/deviceList.js#L266)

Comment 69

[Ken Chang[:kenkai]]

>   2) set a 3 sec timeout before calling sendFile(), just like what bluetooth
> app does now
>     
> (https://github.com/mozilla-b2g/gaia/blob/master/apps/bluetooth/js/
> deviceList.js#L266)

Evenly, as talked with Eric, he is looking into this bug. But in order not to block the development process of NFC. I would like to suggest Arno using the current method used in deviceList.js to avoid this problem. Is it Okay for you?

Comment 70

[Evelyn Hung]

(In reply to Ken Chang[:ken] from comment #69)
> >   2) set a 3 sec timeout before calling sendFile(), just like what bluetooth
> > app does now
> >     
> > (https://github.com/mozilla-b2g/gaia/blob/master/apps/bluetooth/js/
> > deviceList.js#L266)
> 
> Evenly, as talked with Eric, he is looking into this bug. But in order not
> to block the development process of NFC. I would like to suggest Arno using
> the current method used in deviceList.js to avoid this problem. Is it Okay
> for you?

yes, please do. Thanks.

Comment 71

[psiddh]

Thanks Yoshi, Olli for your review comments. I will address them.

Yoshi, I have attached the pdf that briefly states the 'sendFile' flow between various processes. Please give your feedback.

Comment 72

[psiddh]

Attachment: SendFile flow

Comment 73

[psiddh]

Olli,
>> So why do we add nfc-send-file-status event?
>> Couldn't we expose just some API to the system app to send file?

>> (Actually, the same question for nfc-p2p-user-accept)

Do you think it is OK to add two new nfc DOM APIs that are visible to all nfc gaia applications. Though these nfc applications may not be able to use these apis (as the APIs will be protected by 'nfc-manager' perms). What do you suggest ?

Thanks,

Comment 74

[psiddh]

Hi Kyle, 

I have created a brief PDF file: SendFile.pdf flow describing its flow.
Could you please suggest us the way to send / relay a blob from Content process to Chrome process.
In the pdf, it is at STEP#4. Upon further testing on my side, it looks like the blob that I tried to send using cpmm (from NfcContentHelper.js) is not received correctly on the other side by Chrome (Nfc.js). Any suggestions? Note that this blob would be of type 'nsIDOMFile'.

Comment 75

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

(In reply to Siddartha P from comment #73)
> Do you think it is OK to add two new nfc DOM APIs that are visible to all
> nfc gaia applications. Though these nfc applications may not be able to use
> these apis (as the APIs will be protected by 'nfc-manager' perms). What do
> you suggest ?

We need a small separate API which is visible only when nfc-manager perm is set.

Comment 76

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

I think you can even just use the NFC API but have it implement
[NoInterfaceObject, Func="Navigator::HasNfcManagerSupport"]
interface NFCManager {...}


Then 
MozNfc implements NFCManager;


Or if that doesn't work, add the methods to
MozNFC but enable them with
Func="Navigator::HasNfcManagerSupport"


Navigator::HasNfcManagerSupport would do the permission check.

Comment 77

[Garner Lee]

(In reply to Olli Pettay [:smaug] from comment #76)
> I think you can even just use the NFC API but have it implement
> [NoInterfaceObject, Func="Navigator::HasNfcManagerSupport"]
> interface NFCManager {...}
> 
> 
> Then 
> MozNfc implements NFCManager;
> 

WebIDL Implementation question: Is it NoInterfaceObject hides that NFC Manager stuff, or the HasNfcManagerSupport somehow hide the base?

Comment 78

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

NoInterfaceObject hides the interface from the global scope.

So window.YourNoInterfaceObject would be undefined

Comment 79

[arno]

Attachment: Support for file transfer in NfcHandoverManager r=ehung

Sorry for taking a bit longer. Fixed all nits. I've added the delay in BluetoothTransfer since that is where sendFile() is called. I've added the identical comment as in deviceList.js.

Comment 80

[Evelyn Hung]

Comment on attachment 8349887 [details] [review]
Support for file transfer in NfcHandoverManager r=ehung

looks good to me. I don't have a device to test if it works, but the code logic is okay.

Comment 81

[psiddh]

Hi Yoshi,

>> please update the documentation first.
>> I already got some Gaia developers asking me about your API.
Done updated the wiki, with sendFile sniplet

>> Also try to add some diagrams here for better understanding,
>> which app will call sendFile, then which app will receive the system message.
Done . Attached the 'SendFile flow' attachment for your review

@@ +295,5 @@
>            return null;
>          }
>  
>          // Add extra permission check for two IPC Peer events:
>          // 'NFC:CheckP2PRegistration' , 'NFC:NotifyUserAcceptedP2P'
>> update comment here.
Done

>> I guess you want to say
>> "system app (content process)"?
done

> +  sendFile: function sendFile(window, data, sessionToken) {
>> data? I thought it was 'blob'.
Actually the blob that is being passed from nsNfc.js needs to 'object wrapped'. Hence had to use jsval.
If the blob is not object wrapped, I see the following exceptions in the logs at the time of calling
NfcContentHelper's sendFile interface.

E/GeckoConsole( 1031): [JavaScript Error: "Permission denied to access property 'toString'"]
E/GeckoConsole( 1031): [JavaScript Error: "Permission denied to access property 'message'"]
E/GeckoConsole( 1031): [JavaScript Error: "uncaught exception: unknown (can't convert to string)"]

> +
> +    cpmm.sendAsyncMessage("NFC:SendFile", {
> +      requestId: requestId,
> +      sessionToken: sessionToken,
> +      blob: data.blob

>> data.blob?
Same explanation as above point. Any suggestions please ?

>> Can you elaborate the dict for the blob in the IDL?
done.

>> blob, unless you want to rename all other arguments to start with 'a'.
done.

>> const these two intergers, as you did for NFC_EVENT_PEER_READY/LOST
('status' value is sent by System App).  The value of 'status' is not checked / processed in Nfc.js.
Eventually Nfc.js notifies the right content process (nfc gaia application) and simply relays the
'status' value. In NfcContentHelper.js, this value is checked against NFC.GECKO_NFC_ERROR_SUCCESS
Updated the idl with the same vals.

> +  void notifySendFileStatus(in nsIDOMWindow window,
> +                            in octet status,
> +                            in DOMString requestId);


>> What's the requestId here?
>> Where does this come from?
'requestId' is relayed by system app to Nfc.js which in turn relays this back to right / appropriate 
content process. NfcContentHelper uses this to handle the response and fires either success / failure
based on the 'status'. 
In the attached ' sendFile flow PDF' this requestID is created at STEP#4.

Comment 82

[psiddh]

Attachment: (v2) Part 1: SendFile DOM API' r=khuey, sr=smaug

Comment 83

[psiddh]

Attachment: (v2) Part 2: SendFile support in chrome process r=allstars.chh

Comment 84

[psiddh]

Hi Olli,

>> I think you can even just use the NFC API but have it implement
>> [NoInterfaceObject, Func="Navigator::HasNfcManagerSupport"]
>> interface NFCManager {...}
>> Then MozNfc implements NFCManager;

This comment will be addressed in a separate bug 
https://bugzilla.mozilla.org/show_bug.cgi?id=952217

The dom patch now contains only 'sendFile' changes.
Also note that I had to perform a 'slice()' operation on the 'blob' data. Otherwise, when NfcContentHelper.js relays this data to Nfc.js (Chrome process), the blob data seems to be getting deleted (I guess) immediately. So the blob the Nfc.js sends to system application is corrupted / freed.
What do you think ?

Comment 85

[psiddh]

Attachment: (v2) Part 1: SendFile DOM API' r=khuey, sr=smaug

Comment 86

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

Comment on attachment 8350791 [details] [diff] [review]
(v2) Part 2: SendFile support in chrome process r=allstars.chh

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

I think some part of this patch should also be moved to Bug 952217.

Comment 87

[psiddh]

Attachment: (v2b) Part 2: SendFile support in chrome process r=allstars.chh

Hi Yoshi, As suggested, I have removed 'NotifySendFileStatus' part from earlier patch.

Comment 88

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

Comment on attachment 8356227 [details] [diff] [review]
(v2b) Part 2: SendFile support in chrome process r=allstars.chh

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

::: dom/system/gonk/NfcContentHelper.js
@@ +49,5 @@
>    "NFC:ConnectResponse",
>    "NFC:CloseResponse",
>    "NFC:CheckP2PRegistrationResponse",
> +  "NFC:PeerEvent",
> +  "NFC:SendFileResponse"

This message will be received until Bug 952217 is landed,
either:
- Remove this message. And do it in Bug 952217.
or 
- Add a TODO here.

::: dom/system/gonk/nsINfcContentHelper.idl
@@ +40,5 @@
>    nsIDOMDOMRequest connect(in nsIDOMWindow window, in unsigned long techType, in DOMString sessionToken);
>    nsIDOMDOMRequest close(in nsIDOMWindow window, in DOMString sessionToken);
>  
>   /**
> +  * Initiate Send file operation

You didn't update UUID.

Comment 89

[psiddh]

Attachment: (v3) Part 2: SendFile support in chrome process r=allstars.chh

Fixed both the comments

Comment 90

[psiddh]

tryResults of the patches : https://tbpl.mozilla.org/?tree=Try&rev=f9e10d23b9ae

Comment 91

[Garner Lee]

Check-in needed for Gecko.

Waiting on Travis results for Gaia. Arno?

Comment 92

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/b211721ef063
https://hg.mozilla.org/integration/b2g-inbound/rev/18836f818241
https://hg.mozilla.org/integration/b2g-inbound/rev/0b211385b164

I will merge the PR when the b-i push is green.

Should this have tests?

Comment 93

[Ryan VanderMeulen [:RyanVM]]

(In reply to Garner Lee from comment #91)
> Waiting on Travis results for Gaia. Arno?

Travis has repeatable failures. Presumably because Gaia needs the underlying Gecko changes first? Let's re-run Travis tomorrow after this merges to m-c.

Comment 94

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b211721ef063
https://hg.mozilla.org/mozilla-central/rev/18836f818241
https://hg.mozilla.org/mozilla-central/rev/0b211385b164

Comment 95

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/9397654096519ce88b2b624278098cb1601724e4