Closed Bug 948363 Opened 6 years ago Closed 6 years ago

[Gaia][Music] Music application: handle data transfer

Categories

(Firefox OS Graveyard :: Gaia::Music, defect, P1)

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:1.4+, b2g-v1.4 fixed)

RESOLVED FIXED
1.4 S1 (14feb)
blocking-b2g 1.4+
Tracking Status
b2g-v1.4 --- fixed

People

(Reporter: johnhu, Assigned: s.x.veerapandian)

References

Details

(Whiteboard: [FT:RIL] sharing video/audio/image [fxos:media])

Attachments

(3 files)

+++ This bug was initially created as a clone of Bug #903253 +++

During the music sharing via NFC, data transfer needs to be handle.

According to bug 933093, what we need to do is to put the following code at the correct place:
window.navigator.mozNfc.onpeerfound = function(nfcPeer) {
  // Negotiates WiFi or BT connection, presenting UI 
  nfcPeer.sendFile(blob); 
};

The correct place means where the app is under single file sharable context.
This is a cloned bug for music app. We don't need to depend on bug 903253
blocking-b2g: --- → 1.4?
No longer depends on: 903253
Assignee: nobody → dkuo
Bug 952217 is a blocker for this bugs.
Depends on: 952217
Whiteboard: [FT:RIL] sharing video/audio/image
Depends on: 961913
Depends on: 960790
No longer depends on: 961913
Is there any update for this? Thank you.
Flags: needinfo?(dkuo)
Target Milestone: --- → 1.3 C3/1.4 S3(31jan)
Priority: -- → P1
(In reply to Kevin Hu [:khu] from comment #3)
> Is there any update for this? Thank you.

Hi Kevin, currently John is working on bug 903253 for video, and the patch for music should be very similar to it, so most of the code can be used in music, I will sync with John today, thanks.
Flags: needinfo?(dkuo)
(In reply to Dominic Kuo [:dkuo] from comment #4)
> (In reply to Kevin Hu [:khu] from comment #3)
> > Is there any update for this? Thank you.
> 
> Hi Kevin, currently John is working on bug 903253 for video, and the patch
> for music should be very similar to it, so most of the code can be used in
> music, I will sync with John today, thanks.

Thank you, Dominic.
Depends on: 963471
Depends on: 964672
Depends on: 964693
Depends on: 962310
Sami is working on this so I am re-assigning this bug to him.
Assignee: dkuo → s.x.veerapandian
Attached file WIP
Sami has sent me the WIP offline, I will help him to complete and review the patch.
Dominic,

Sami's patch contains the patch for bug 963471. We should ask him to move that to bug 963471.
(In reply to John Hu [:johnhu][:johu][:醬糊小弟] from comment #8)
> Dominic,
> 
> Sami's patch contains the patch for bug 963471. We should ask him to move
> that to bug 963471.

Thanks John, I saw it, too, and I think I will just do this review in a formal way instead of discussing with Sami offline.
Attachment #8366700 - Flags: review?(dkuo)
Target Milestone: 1.3 C3/1.4 S3(31jan) → 1.4 S1 (14feb)
Whiteboard: [FT:RIL] sharing video/audio/image → [FT:RIL] sharing video/audio/image [fxos:media]
Just back from the holidays, I will review the patch tomorrow.
Comment on attachment 8366700 [details] [review]
WIP

Sami,

The code looks fine, it should work but I think we better make it more understandable, I guess some code patterns are from the gaia nfc example, but we can still adjust it to fit the music app code style, and not to confuse the other contributors. Please address the issues and feel free to assign the review to me again after you finish it, thanks.
Attachment #8366700 - Flags: review?(dkuo)
Attached file 16083[1]
Hi Dominic,

PR: https://github.com/mozilla-b2g/gaia/pull/16083

1.Comments are added properly.
2.Success/Error handler functions are removed for SendFile().

If we setup the Event Listener in the PlayerView.init () then, Once we come out from the Play Mode, It clears the onpeerhandler, but won’t set it again. 

We need to enable the onpeerhandler while we enter into PLAY_MODE only, other-wise make into NULL
Attachment #8372362 - Flags: review?(dkuo)
Comment on attachment 8372362 [details]
16083[1]

Sami,

Thanks for updating the patch, the code looks good to me. At first I didn't realize the mozNfc api checks the onpeerready function as a flag to decide to share the file or not, after I figured out I think you are right for this, thanks for pointing it out. Before landing this patch, there are some minor issues on github I would like you to address, after you finished, please ping me or comment on bug then I will land it for you!
Attachment #8372362 - Flags: review?(dkuo) → review+
Hi Dominic,

Please find modified patch as per your comments in github.

PR: https://github.com/mozilla-b2g/gaia/pull/16209

Thanks.
Attachment #8374790 - Flags: review?(dkuo)
blocking-b2g: 1.4? → 1.4+
Comment on attachment 8374790 [details] [diff] [review]
0001-Bug-948363-Music-application-handle-data-transfer.-r.patch

Sami,

thanks for updating the patch, all issues are addressed and travis got green now, I am going to land your patch.
Attachment #8374790 - Flags: review?(dkuo) → review+
Landed on master: 628b55adeb8f93da54b931a5bfaec020c9876573
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.