1028800 - Inconsistent multi-tasking behaviour in Media Picker

Status: VERIFIED FIXED

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

Description

[Rob MacDonald [:robmac]]

Steps to Reproduce
- Launch email
- Tap create a new message
- Tap on attachment icon
- Tap on "Gallery"
- Multi-task away then return to email. (Hit home then return, swipe left then swipe back, or use the task switcher)

Actual Behaviour
- Gallery picker is dismissed when emails go to the background
- Other pickers (like video) are retained.

Expected Behaviour
- The Gallery picker should be retained when the user returns to email, as per video.

Comment 1

[Rob MacDonald [:robmac]]

Needinfo'ing Etienne as he's familiar with the source of this issue and how to quickly fix it.

Comment 2

[Etienne Segonzac (:etienne)]

Same thing than the contacts app, old code that we probably want to revisit [1].

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/gallery/js/gallery.js#L1043-1046

Comment 3

[Jason Smith [:jsmith]]

Long standing issue, not a blocker.

Comment 4

[Punam Dahiya [:pdahiya]]

Attachment: PR with fix of Bug 1028800

Agree with Etienne (#comment 2), the code to hide picker activity on visibility change is safe to remove. Removed old code and tested attached patch to keep gallery pick activity alive when invoking app goes to background.

David, please review attached patch.Thanks

Comment 5

[Punam Dahiya [:pdahiya]]

NI Alive to keep him in loop as his fix discussed in  https://bugzilla.mozilla.org/show_bug.cgi?id=1028502#c2, for the issue we cannot have the two 'same' activity instances at the same time is going to help gallery pick activity as well, e.g. as of now if email is adding images, and the user swipes to the messages app to add another image, the second pick activity will get no system message and doesn't work.

Comment 6

[David Flanagan [:djf]]

Comment on attachment 8445791 [details] [review]
PR with fix of Bug 1028800

Looks good to me.

Comment 7

[David Flanagan [:djf]]

Punam: have you confirmed the scenario in comment #5? If that really happens, then maybe we should hold off on landing this patch until that bug is fixed. I'd argue that cancelling the activity when the app goes to the background is better than not being able to launch an activity in the foreground because there happens to be an activity in the background somewhere.  I can't tell from the discussion in bug 1028502 whether that is primarily about contacts cancelling a running activity (just like this gallery bug) or whether it is about that fundamental limitation of the system app.

If the issue you describe in comment #5 does actually reproduce, then I think we should make this bug depend on that one and nominate this as a 2.1?

If that issue does not actually reproduce, then I'd like to nominate this for uplift to 2.0 because the introduction of edge swiping will make this bug a lot more visible to users. The patch is so trivial that I'd like to uplift if we can do that safely.

Needinfo Punam to confirm whether the more serious issue in comment #5 is real.

Needinfo Rob: Rob since you filed this bug, would you support uplifting the fix to 2.0?  Also, would you agree that it is better to cancel the activity than to prevent foreground apps from launching an activity?

Comment 8

[Rob MacDonald [:robmac]]

(In reply to David Flanagan [:djf] from comment #7)

> Needinfo Rob: Rob since you filed this bug, would you support uplifting the
> fix to 2.0?  Also, would you agree that it is better to cancel the activity
> than to prevent foreground apps from launching an activity?

I'd like to uplift the fix to 2.0. My only concern is that, if comment 5 is still valid and we can't have two activities running at the same time, I'd prefer to cancel the first activity when the second activity is triggered as opposed to when the first activity goes into the background. Launching a second activity (from messaging or email for example) is more of an exception whereas tasking away while in an activity will become quite common. So we'd be compromising the more common use case. 

Does this make sense? And is it feasible for 2.0?

Comment 9

[Rob MacDonald [:robmac]]

Forgot to flag David.

Comment 10

[Punam Dahiya [:pdahiya]]

(In reply to David Flanagan [:djf] from comment #7)
> Punam: have you confirmed the scenario in comment #5? If that really
> happens, then maybe we should hold off on landing this patch until that bug
> is fixed. I'd argue that cancelling the activity when the app goes to the
> background is better than not being able to launch an activity in the
> foreground because there happens to be an activity in the background
> somewhere.  I can't tell from the discussion in bug 1028502 whether that is
> primarily about contacts cancelling a running activity (just like this
> gallery bug) or whether it is about that fundamental limitation of the
> system app.
> 
> If the issue you describe in comment #5 does actually reproduce, then I
> think we should make this bug depend on that one and nominate this as a 2.1?

Scenario explained in #comment 5 exists with the patch applied. With the patch applied, if email is adding images, and the user swipes to the messages app to add another image, the second pick activity doesn't work and gallery app loads in thumbnaillist view instead of pick view.

I am seeing this issue on master for Video, music and camera pick flow where we are not using visibility change to cancel first pick activity when it goes hidden.

I agree we should hold on landing this patch , if it lands we will not have a valid activity when we open it twice and we should file another bug for video, music and camera app to add similar code till Alive fix recommended in https://bugzilla.mozilla.org/show_bug.cgi?id=1028502#c2 is implemented.

Comment 11

[Punam Dahiya [:pdahiya]]

(In reply to David Flanagan [:djf] from comment #7)
> I can't tell from the discussion in bug 1028502 whether that is
> primarily about contacts cancelling a running activity (just like this
> gallery bug) or whether it is about that fundamental limitation of the
> system app.
> 

My understanding is Bug 1028502 is addressing similar issue for contacts as gallery 
with two part fix
1. Remove visibiitychange cancel pick activity code in https://bugzilla.mozilla.org/show_bug.cgi?id=1028502#c6
2. Address the system app limitation by providing support to kill first activity if there's a new activityrequest which is not related to the previous background activity. 

Alive, can you please confirm if Bug 1028502#c2 -> #2 is similar to what Rob has suggested in #comment 8 that is cancel the first activity when the second activity is triggered.

Comment 12

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

(In reply to Punam Dahiya from comment #11)
> (In reply to David Flanagan [:djf] from comment #7)
> > I can't tell from the discussion in bug 1028502 whether that is
> > primarily about contacts cancelling a running activity (just like this
> > gallery bug) or whether it is about that fundamental limitation of the
> > system app.
> > 
> 
> My understanding is Bug 1028502 is addressing similar issue for contacts as
> gallery 
> with two part fix
> 1. Remove visibiitychange cancel pick activity code in
> https://bugzilla.mozilla.org/show_bug.cgi?id=1028502#c6
> 2. Address the system app limitation by providing support to kill first
> activity if there's a new activityrequest which is not related to the
> previous background activity. 
> 
> Alive, can you please confirm if Bug 1028502#c2 -> #2 is similar to what Rob
> has suggested in #comment 8 that is cancel the first activity when the
> second activity is triggered.

We cannot cancel the first activity. The second one will fail "right away" once the second one is launched and we don't know they are the same activity or not.

For example,
(1) App A calls pick activity to launch gallery(instance#1)
(2) Swipe to App B
(3) App B calls pick activity to launch gallery(instance#2)

The instance#1 will get 'activity' system message "again" once the second request is observed in gecko,
and gaia doesn't know before the second gallery launch request (open-app) request is sent to system app from gecko.

I planned to work around this issue in gaia side in another bug, the idea is to build a chain for activity and once 
1. there's a new 'activity-choice' event from gecko
2. we are in an app which is not in the activity chain for the existing one, kill all the background activity.

Comment 13

[Punam Dahiya [:pdahiya]]

Thanks Alive for clarifying. I am marking this bug dependent on your fix (#comment 12) for bug 1028502.

Comment 14

[David Flanagan [:djf]]

In response to Rob's questions in comment #8: yes, it makes sense.  And, based on Alive's comments it doesn't sound feasible for 2.0 to me.

For 2.1, I suppose we could just land Punam's patch, making gallery behave like all the other apps, and then hope for Alive's patch to fix the other issue.

And if Alive's patch gets uplifted to 2.0 (which I doubt it will) then we'd uplift this patch, too?

Comment 15

[Punam Dahiya [:pdahiya]]

As discussed in #commemt 8 and #comment 14, landing the patch in master, so that gallery behave like the other media apps. I will leave the bug 1028502 dependency as Alive fix is needed for this bug.

https://github.com/mozilla-b2g/gaia/commit/6debced2e5dc3e8f85f6c45986207575a14486cb

Comment 16

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

Hi guys, the system app workaround to support activity chaining when going to background is merged in bug 1008928.
Lemme know if you have problems.

Comment 17

[Punam Dahiya [:pdahiya]]

(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #16)
> Hi guys, the system app workaround to support activity chaining when going
> to background is merged in bug 1008928.
> Lemme know if you have problems.

Thanks Alive, with bug 1008928 fix, the first activity is cancelled when the second activity is triggered. With the dependent bug fixed, marking the bug resolved.

Comment 18

[Lance（leave from Mozilla）]

This issue has been verified successfully on Flame 2.1.
See attachment: 1717.MP4
Reproducing rate: 0/5

Step:
1.Launch Email.
2.Create a new mail.
3.Tap the attachment icon -> "Gallery".
4.Swipe left then swipe back.
5.Press Home key, then launch Email again.
6.Long tap the Home key to enter recent app page.

Actual result:
4 & 5 & 6:The Gallery picker still retained when the user returns to email.

Flame 2.1 version:
Gaia-Rev        38e17b0219cbc50a4ad6f51101898f89e513a552
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-2g34_v2_1/rev/8b92c4b8f59a
Build-ID        20141205001201
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141205.035305
FW-Date         Fri Dec  5 03:53:16 EST 2014
Bootloader      L1TC00011880

Comment 19

[Lance（leave from Mozilla）]

Attachment: 1717.MP4