Closed Bug 880588 Opened 10 years ago Closed 10 years ago

[System] inline activities are removed when locked.

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:leo+, b2g18 fixed, b2g-v1.1hd fixed)

RESOLVED FIXED
1.1 QE3 (26jun)
blocking-b2g leo+
Tracking Status
b2g18 --- fixed
b2g-v1.1hd --- fixed

People

(Reporter: leo.bugzilla.gaia, Assigned: alive)

References

Details

(Whiteboard: TaipeiWW)

Attachments

(1 file)

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:  hanj.kim25@gmail.com
This causes Attachment downloads get aborted from Browser app. 
Can we change their visibility, instead of calling stopInlineActivity?
Flags: needinfo?(alive)
If you want to fix this, let's fix 853759 first.
Flags: needinfo?(alive)
Depends on: 846850
Assignee: nobody → alive
blocking-b2g: --- → leo?
blocking-b2g: leo? → leo+
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?
Flags: needinfo?(justin.lebar+bug)
See previous comment.
Flags: needinfo?(justin.lebar+bug)
Blocks: 845251
Whiteboard: TaipeiWW
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?
Flags: needinfo?(justin.lebar+bug)
(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?)
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?
Flags: needinfo?(fabrice)
(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.
Thanks.

Mounir, do you know anything about this?
Flags: needinfo?(mounir)
Target Milestone: --- → 1.1 QE4
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.
(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?
Flags: needinfo?(mounir) → needinfo?(gene.lian)
Flags: needinfo?(21)
Simple workaround w/o gecko fix.
Attachment #767562 - Flags: review?(timdream)
Target Milestone: 1.1 QE4 → 1.1 QE3 (26jun)
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.
Attachment #767562 - Flags: review?(timdream) → review+
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.
No longer blocks: 845251
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.
master
https://github.com/mozilla-b2g/gaia/commit/4cead537e5cf3d772a182cabe3ce8211838c371d
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Blocks: 887650
Blocks: 887683
v1.1.0hd: 6770b0519596f6e10f891d53c606f6f1b652b455
v1.1.0hd: 92af9b501ab4d1d68e127a12dd10c0b6fd87ff62
Flags: needinfo?(gene.lian)
Blocks: 898307
Flags: needinfo?(fabrice)
You need to log in before you can comment on or make changes to this bug.