987224 - [B2G][Video]Unable to watch videos in video app after deleting two videos

Status: RESOLVED FIXED

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

Description

[Ross Kuhlman ([e-mail])]

Attachment: VidNoLaunch.txt

If the user deletes two videos from the video app using the selection view, there is a chance that they will not be able to watch any videos until the app has been closed and restarted.

Repro Steps:
*prerequisite: have at least 3 videos stored on the sd card.

1) Updated Buri to BuildID: 20140324000202
2) Launch video app.
3) Enter selection view. (Tap the box outline in the lower right corner)
4) Select two videos and delete them by tapping the trashcan icon.
5) Press the 'X' in the upper left corner to exit selection view.
6) Tap on a video to watch it.

Actual:
The video flashes to indicate that it has been tapped, but otherwise nothing happens. The list of videos is still displayed and no videos can be launched.

Expected:
The video launches and plays and all expected UI is present. (play/pause, slider, etc)

Environmental Variables:
Device: Buri v1.4 Moz RIL
BuildID: 20140324000202
Gaia: 730670951e40b2317a167fcd07c398bb662d6e87
Gecko: a44f8b39c2c8
Version: 30.0a2
Firmware Version: v1.2-device.cfg

Notes:

Repro frequency: 40%
See attached: logcat

Comment 1

[Jason Smith [:jsmith]]

Can we check what happens on 1.3?

Comment 2

[Ross Kuhlman ([e-mail])]

Issue does not occur in v1.3
Environmental Variables:
Device: Buri v1.3 Moz RIL
BuildID: 20140324004001
Gaia: f7742fb4929cc57c9f72955ce5cebb8279745ac0
Gecko: e42b778a010f
Version: 28.0
Firmware Version: v1.2-device.cfg

Comment 3

[Matthew Vaughan]

This issue started reproducing on the 12/13/14 1.4 build.

- Last Working -
Device: Buri Master v1.4 MOZ RIL
BuildID: 20131212040203
Gaia: 8952898bbc98dd31e25b647203791cf129867ff1
Gecko: 1ad9af3a2ab8
Version: 29.0a1
Firmware Version: V1.2-device.cfg

- First Broken -
Device: Buri Master 1.4 MOZ RIL
BuildID: 20131213040203
Gaia: 390b313a254a947d12e3cdbcde19d7d1619ff63c
Gecko: 8b5875dc7e31
Version: 29.0a1
Firmware Version: V1.2-device.cfg


**This looks to be a gaia issue**
last working gaia/first broken gecko = NO REPRO
Gaia: 8952898bbc98dd31e25b647203791cf129867ff1
Gecko: 8b5875dc7e31

first broken gaia/last working gecko = REPRO
Gaia: 390b313a254a947d12e3cdbcde19d7d1619ff63c
Gecko: 1ad9af3a2ab8

Push log: https://github.com/mozilla-b2g/gaia/compare/8952898bbc98dd31e25b647203791cf129867ff1...390b313a254a947d12e3cdbcde19d7d1619ff63c

Comment 4

[Jason Smith [:jsmith]]

Regressing cause - bug 903920.

Comment 5

[bhavana bajaj [:bajaj]]

If we can we should try to backout 903920 here to resolve this issue.

Comment 6

[Hema Koka [:hema]]

Johu,

Could you please do a quick investigation -- it looks like this is caused by the layout changes on video

Thanks
Hema

Comment 7

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

It is caused by bug 903920. I can provide a STR with 100% reproduce ratio:

Following rkuhlman's STR but only one step need to be changed: 

1) Updated Buri to BuildID: 20140324000202
2) Launch video app.
3) Enter selection view. (Tap the box outline in the lower right corner)
4*) Select two *adjacent* videos from *bottom to top* and delete them by tapping the trashcan icon.
5) Press the 'X' in the upper left corner to exit selection view.
6) Tap on a video to watch it.

And in the console, you may find the following log:
[JavaScript Error: "TypeError: old is undefined" {file: "app://video.gaiamobile.org/gaia_build_defer_index.js" line: 595}]

For bug 903920 (tablet), we need to keep the currentVideo variable valid to have the focus when we delete the currentVideo. resetCurrentVideo handles such kind of case. It is called at the deleteFile[1]. But the removal of thumbnail from thumbnail list is async function call which is triggered by  instead of the callback of mediadb[2].

The reason we need to select adjacent video from bottom to top is that: resetCurrentVideo find the next video file from UI to be the currentVideo. When we delete two videos at the same thing, the second deletion chooses the first deleted video as the currentVideo because the thumbnail isn't removed from thumbnail list.

That's the root cause. I think the patch may be simple like moving [1] to [3].

[1] https://github.com/mozilla-b2g/gaia/blob/624c781427ed48aec9907cffbc9158022bafb1b1/apps/video/js/video.js#L566-L571
[2] https://github.com/mozilla-b2g/gaia/blob/624c781427ed48aec9907cffbc9158022bafb1b1/apps/video/js/video.js#L576
[3] https://github.com/mozilla-b2g/gaia/blob/624c781427ed48aec9907cffbc9158022bafb1b1/apps/video/js/db.js#L130

Comment 8

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

Russ,

I don't have any capacity to take this one. Would you mind to take it? I think this is a easy bug. Please find more information at comment 7. If you need help, please need info me.

Comment 9

[Russ Nicoletti [:russn]]

Thanks, John. I will take it.

Comment 10

[Russ Nicoletti [:russn]]

Attachment: Pull request: https://github.com/mozilla-b2g/gaia/pull/17664

John, I implemented your suggestion from comment #7. My testing of the patch confirmed it fixed the problem.

Comment 11

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

Comment on attachment 8397248 [details] [review]
Pull request: https://github.com/mozilla-b2g/gaia/pull/17664

Thanks Russ. It looks good. The travis is red. Please rebase your code and check if there is anything wrong in travis. Thanks.

Comment 12

[Russ Nicoletti [:russn]]

I created a new PR with rebased code (https://github.com/mozilla-b2g/gaia/pull/17721) and travis is green. 

Merged into master:

https://github.com/mozilla-b2g/gaia/commit/8248f8da7a29757da9f7fd748a0d8d70f2bd4610

Comment 13

[Ryan VanderMeulen [:RyanVM]]

v1.4: https://github.com/mozilla-b2g/gaia/commit/2678db4850c2e40d4db036477654dbb40e05bea1