880588 - [System] inline activities are removed when locked.

Status: RESOLVED FIXED

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

Description

[Leo]

Currently the inline activities are removed when the device is locked. (Refer to the following code.)
 	
apps/system/js/window_manager.js
 	
window.addEventListener('lock', function onScreenLocked() {
    if (inlineActivityFrames.length) {
      stopInlineActivity(true);
    }
  });


This causes Attachment downloads get aborted from Browser app. Therefore, instead of stopInlineActivity, can we change the visibility such as the following?

window.addEventListener('lock', function onScreenLocked() {
    if (inlineActivityFrames.length) {
      //stopInlineActivity(true);
      for (var inlineActivity of inlineActivityFrames) {
        inlineActivity.firstChild.setVisible(false);
      }
    }
  });


Mozilla build ID: 20130518070206
Gaia Revision : 9380ceb81b3eac45861b8d1be07ab7f748ed52a3

Personal email id:  [e-mail]

Comment 1

[Leo]

This causes Attachment downloads get aborted from Browser app. 
Can we change their visibility, instead of calling stopInlineActivity?

Comment 2

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

No because https://bugzilla.mozilla.org/show_bug.cgi?id=853759, https://bugzilla.mozilla.org/show_bug.cgi?id=846850

Comment 3

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

If you want to fix this, let's fix 853759 first.

Comment 4

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

WIP based on bug 853759.
https://github.com/alivedise/gaia/tree/inline-activity-visibility

Comment 5

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

I found that the visibility of inline activity opener is never turned into false.
And if I correct the activity opener and the intermediate activity frame to false,
it's easy to hit OOM and to be killed...once that happens, the top activity frame would be removed too.
That's bad.

Justin, how do you think? Shall we turn every frame of inline activity to avoid that? Though I don't think so...
Is there some way to let activity opener less to be killed like what we do to homescreen app?

Comment 6

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

See previous comment.

Comment 7

[Justin Lebar (not reading bugmail)]

So we want to make it so that when we show an inline activity, the opener is setVisible(false)'d, but has...what OOM priority?

If we kill the opener, we kill the activity, so it seems to me that the opener should have at least the same priority as the activity.  Maybe the opener should have even higher priority than the activity!

Is that right?

Comment 8

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

(In reply to Justin Lebar [:jlebar] from comment #7)
> So we want to make it so that when we show an inline activity, the opener is
> setVisible(false)'d, but has...what OOM priority?
> 
> If we kill the opener, we kill the activity, so it seems to me that the
> opener should have at least the same priority as the activity.  Maybe the
> opener should have even higher priority than the activity!
> 
> Is that right?

Should be! Is this a current policy or we need to implement it in gecko?
(Does mozapptype help?)

Comment 9

[Justin Lebar (not reading bugmail)]

This is not the current policy, but we can implement it without too much trouble.

Fabrice, do you know how inline activities work in Gecko?  Are they actually in a different process?  Does Gecko know when a process has an inline activity "over" it, and does Gecko know when a frame houses an inline activity?

Comment 10

[Jason Smith [:jsmith]]

(In reply to Justin Lebar [:jlebar] from comment #9)
> This is not the current policy, but we can implement it without too much
> trouble.
> 
> Fabrice, do you know how inline activities work in Gecko?  Are they actually
> in a different process?  Does Gecko know when a process has an inline
> activity "over" it, and does Gecko know when a frame houses an inline
> activity?

FYI - Fabrice is out away from bugmail until June 29th.

Comment 11

[Justin Lebar (not reading bugmail)]

Thanks.

Mounir, do you know anything about this?

Comment 12

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

After discussion w/ leo, he hopes this to be resolved before 6/26.
So I think we could just revise comment 0's code to be workaround. if we don't want to be blocked by gecko fix.

Comment 13

[Mounir Lamouri (:mounir)]

(In reply to Justin Lebar [:jlebar] from comment #11)
> Thanks.
> 
> Mounir, do you know anything about this?

I don't know much about what is asked in comment 9. Maybe Vivien or Gene can help?

Comment 14

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

Attachment: https://github.com/mozilla-b2g/gaia/pull/10623

Simple workaround w/o gecko fix.

Comment 15

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 767562 [details]
https://github.com/mozilla-b2g/gaia/pull/10623

What's our proper plan to do memory management? The plan should address this use case, please make sure proper persons are aware of it.

Comment 16

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

Leo.gecko tells me that patch in this bug would introduce bug 881584.

I think we have two options to do here, if we don't want to cause regression:
1) Make pdfjs activity to be not inline.
2) Make video/camera/youtube app listen to visibilitychange event to close themselves.

I personally vote for (1), but I am worried about that pdfjs which is a hidden app would be shown in cardview if we do (1).
(2) sounds a big work to do. But I think this is a longterm way even we don't change it now. Every app/activity should use visibilitychange event to do something dependently.

Comment 17

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

Lan(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #15)
> Comment on attachment 767562 [details]
> https://github.com/mozilla-b2g/gaia/pull/10623
> 
> What's our proper plan to do memory management? The plan should address this
> use case, please make sure proper persons are aware of it.

Let's track in another bug.
Merging.

Comment 18

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

master
https://github.com/mozilla-b2g/gaia/commit/4cead537e5cf3d772a182cabe3ce8211838c371d

Comment 19

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

v1-train
https://github.com/mozilla-b2g/gaia/commit/6770b0519596f6e10f891d53c606f6f1b652b455

Comment 20

[John Ford [:jhford] CET/CEST Berlin Time]

v1.1.0hd: 6770b0519596f6e10f891d53c606f6f1b652b455
v1.1.0hd: 92af9b501ab4d1d68e127a12dd10c0b6fd87ff62