Closed Bug 1022804 Opened 6 years ago Closed 6 years ago

[B2G][Settings][Download Manager] When deleting the ringtone and restarting the phone, the music still appears in the download manger

Categories

(Firefox OS Graveyard :: Gaia::Settings, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:1.4+, b2g-v1.4 fixed, b2g-v2.0 verified, b2g-v2.1 verified)

RESOLVED FIXED
2.0 S5 (4july)
blocking-b2g 1.4+
Tracking Status
b2g-v1.4 --- fixed
b2g-v2.0 --- verified
b2g-v2.1 --- verified

People

(Reporter: psiphantong, Assigned: aus)

References

()

Details

(Whiteboard: [systemsfe])

Attachments

(3 files)

Attached file rt.txt
Description:
When the user sets a ringtone in download manger and restarts the phone, the music's name/tittle still appears in the download manger 


Repro Steps:
1) Update a Flame to 20140609040203
2) Download music from the Browser
3) Go to Settings > Downloads 
4) Select the music, Set as ringtone > Save
5) Restart the phone
6) Go to Settings > Downloads 


Actual:
After deleting the music and restarting the phone, the music's name still appears


Expected:
After deleting the music and restarting the phone, the music's name should not appear 

The issue occurs on the following devices:

Environmental Variables:
Device: Flame 2.0
Build ID: 20140609040203
Gaia: 12af93123c5db55212d51fe235d39f21209a1eaa
Gecko: 9305a8ec77fe
Version: 32.0a1 (2.0) 
Firmware Version: v10G-2

Flame 1.4
Build ID: 20140606000204
Gecko: https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/960c48b5a5fa
Gaia: 183efb374be1bcbf92f1fb3d0212a68b564a6d3e
Platform Version: 30.0
Firmware Version: V10g-2


The issue does not occur on the following devices:

Buri 2.0
Build ID: 20140606043015
Gecko: https://hg.mozilla.org/mozilla-central/rev/c8288d0c7a15
Gaia: 857129928b6e56a809cee9d5445effb8fa9f1c2c
Platform Version: 32.0a1
Firmware Version: V1.2-device.cfg


User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0


Notes: When the user opens the music file, the file does not exist 


Repro frequency: 100%
See attached: video clip, logcat,  https://www.youtube.com/watch?v=_jOu7oAHmdM
Component: Gaia::Settings → Gaia::Ringtones
It's not clear to me where the deletion should be taking place?

This seems more like a download manager bug anyway, unless there's something else happening that I'm not aware of...
The STR looks incorrect here. There's no mention of deleting of ringtones in the STR. Can you rewrite the STR here?
Flags: needinfo?(psiphantong)
Updated Repro Steps:
1) Update a Flame to 20140609040203
2) Download music from the Browser
3) Go to Settings > Downloads 
4) Select the music, Set as ringtone > Save
5) Delete the music
5) Restart the phone
6) Go to Settings > Downloads
Flags: needinfo?(psiphantong)
(In reply to Jim Porter (:squib) from comment #1)
> It's not clear to me where the deletion should be taking place?
> 
> This seems more like a download manager bug anyway, unless there's something
> else happening that I'm not aware of...

Right - this sounds more like a download manager bug.
Component: Gaia::Ringtones → Gaia::Settings
Aus - The behavior here seems weird, so I'd like a second opinion on what's going on here.

So we download a music file. I set it as my main ringtone. I restart the phone and the music file isn't deleted.

What I think might be happening here is that the deletion is failing to occur in the underlying API because the file is actively in use by another app. The settings app however sends the user perception that the file was deleted, when in reality, it wasn't deleted at all.
Flags: needinfo?(aus)
(In reply to Jason Smith [:jsmith] from comment #5)
> So we download a music file. I set it as my main ringtone. I restart the
> phone and the music file isn't deleted.

That should say "So we download a music file. I set it as my ringtone. I delete it. I restart my phone. I notice the file wasn't deleted."
:jsmith, Sounds like we're not reporting the failure to the user. If the file is indeed not deleted because it's in use, it is something we should be able to report. This sounds like a valid bug to me.

However, we would have a hard time telling the user exactly why the file is in use. Which in this case, is because they set it as their ringtone.

:djf, what happens exactly when a user sets an audio file as their ringtone? Do my comments above reflect reality?
Flags: needinfo?(aus) → needinfo?(dflanagan)
The particulars of Blobs are a bit of a mystery to me, but once the ringtones app receives the activity, we take the Blob and then eventually save it here: <https://github.com/mozilla-b2g/gaia/blob/master/apps/ringtones/js/share.js#L207> which calls this: <https://github.com/mozilla-b2g/gaia/blob/master/apps/ringtones/js/custom_ringtones.js#L218-L252>.
Ringtone management in 2.0 and later is completely different than in 1.4. Jim knows the 2.0 code. It looks like he's storing the file into an IndexedDB. This would make a private copy if it, so this should not be holding on to the original file.  At this point, the ringtone app that handles the "share as ringtone" activity has exited, so even if it had open file handles, they would have been released.

In 1.4 when you share a ringtone, the blob just gets copied into the settings db. Again, no handle to the original file is retained.  

Note that the original description of the bug says "When the user opens the music file, the file does not exist". That says to me that the file is actually being deleted. But the Download manager is not correctly recording the fact that it is deleted and somehow thinks it still exists.
Flags: needinfo?(dflanagan)
blocking-b2g: --- → 1.4?
Whiteboard: [systemsfe]
The STR is still not clear. Where did you delete the ringtone?
Flags: needinfo?(psiphantong)
Keywords: steps-wanted
QA Contact: jharvey
I deleted the ringtone/music in the download list. Updated video, https://www.youtube.com/watch?v=FXdXN94x_h0

Updated Repro Steps:
2) Have a mp3 downloaded on the phone
3) Go to Settings > Downloads 
4) Select the music, Set as ringtone > Save
5) Tap edit > select the mp3 > delete 
5) Restart the phone
6) Go to Settings > Downloads

the mp3 name/tittle still remains. When the user opens the mp3, the file does not exist
Flags: needinfo?(psiphantong)
Keywords: steps-wanted
This is a good one but can live with in 1.4. Nom'ing for 2.0 as ringtone is a feature in that release.
blocking-b2g: 1.4? → 2.0?
(In reply to Preeti Raghunath(:Preeti) from comment #12)
> This is a good one but can live with in 1.4. Nom'ing for 2.0 as ringtone is
> a feature in that release.

This is a critical feature for 1.4 & with a broken use case here, so I disagree.
blocking-b2g: 2.0? → 1.4?
(In reply to Jason Smith [:jsmith] from comment #13)
> (In reply to Preeti Raghunath(:Preeti) from comment #12)
> > This is a good one but can live with in 1.4. Nom'ing for 2.0 as ringtone is
> > a feature in that release.
> 
> This is a critical feature for 1.4 & with a broken use case here, so I
> disagree.

I agree with this being a lower set of features in 1.4 and will now block on it.
blocking-b2g: 1.4? → 1.4+
Ping, this need some help from your team.
Flags: needinfo?(anygregor)
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #15)
> Ping, this need some help from your team.

Gregor is on PTO, so I'm redirecting to Candice.
Flags: needinfo?(anygregor) → needinfo?(cserran)
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #15)
> Ping, this need some help from your team.

I will raise this in our standup and get someone on it...thanks
Flags: needinfo?(cserran)
Aus,

Can you take a look at this please?
Flags: needinfo?(aus)
Assignee: nobody → aus
Target Milestone: --- → 2.0 S5 (4july)
There's a race condition between the Downloads API initializing and the DownloadManager in Gaia asking it to clear all completed downloads. In some instances, notifications of completed downloads would still make it through if the user did not click on the notification from the toaster or notification tray.

I've fixed this by clearing the downloads api reference to the download when we take ownership by adding it to the gaia download store.

https://github.com/mozilla-b2g/gaia/pull/21069
Flags: needinfo?(aus)
blocking-b2g: 1.4+ → 2.0?
I would like to land this on 2.0 and 1.4. The issue happens on 2.1 as well, but it's harder to reproduce on the Flame because it's a higher performance device.
Status: NEW → ASSIGNED
Aus - What's the reason for moving this to a 2.0 nom? Was that accidental or intentional?
Flags: needinfo?(aus)
I wasn't sure if I needed to ask for nom for 2.0 if it had 1.4+ already. This fix should land in 2.0 as well.
Flags: needinfo?(aus)
(In reply to Ghislain Aus Lacroix [:aus] from comment #23)
> I wasn't sure if I needed to ask for nom for 2.0 if it had 1.4+ already.
> This fix should land in 2.0 as well.

I'd leave this as a 1.4+ bug & ask for 2.0 approval after this lands.
blocking-b2g: 2.0? → 1.4+
Whiteboard: [systemsfe] → [systemsfe][2.0-approval-needed]
(In reply to Ghislain Aus Lacroix [:aus] from comment #23)
> I wasn't sure if I needed to ask for nom for 2.0 if it had 1.4+ already.
> This fix should land in 2.0 as well.

I also feel like it should be treated with the same severity as it is for 1.4. :)
Comment on attachment 8446708 [details] [review]
Pull Request - Clear download from Downloads API when Download Store takes ownership.

ok, thanks a lot, LGTM, r+. Good job as usual
Attachment #8446708 - Flags: review?(crdlc) → review+
Per conversation with Lawrence and Bhavana we should not be requiring approvals for 1.4 to 2.0 uplift *before* FC.
Keywords: checkin-needed
Whiteboard: [systemsfe][2.0-approval-needed] → [systemsfe]
Depends on: 1055352
Attached video Verify_Video_Flame.MP4
This issue has been verified successfully on Flame 2.0 & 2.1.
See attachment: Verify_Video_Flame.MP4
Reproducing rate: 0/5

Flame v2.0 version:
Gaia-Rev        8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/c756bd8bf3c3
Build-ID        20141130000204
Version         32.0

Flame v2.1 version:
Gaia-Rev        ccb49abe412c978a4045f0c75abff534372716c4
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/18fb67530b22
Build-ID        20141130001203
Version         34.0
You need to log in before you can comment on or make changes to this bug.