581314 - Process-level suspending of the content

Status: RESOLVED WONTFIX

(Core :: IPC, defect)

Description

[Jon Hemming]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.11) Gecko/2009060309 Ubuntu/8.04 (hardy) Firefox/3.0.11
Build Identifier: 

Process-level suspending of the content could be used when browser has been idle longer than value set for suspending by user. This way user can self decide when to use suspending.

Reproducible: Always

Comment 1

[Jon Hemming]

Attachment: Makes suspending when receives "browser-suspend" and continues with "browser-continue"

This also adds display state listening for Maemo 6 which is used with fennec additions to decide when to notify with "browser-suspend" and "browser-continue"

Comment 2

[Jon Hemming]

Attachment: Fennec patch for making the "browser-suspend" and "browser-continue" notifies

Created against revision: 52f539338319
This patch adds "browser.idle.suspend.after" pref and setting for it that can be used to set time limit for how long is waited in "display-off" before "browser-suspend" is notified to observers.

Comment 3

[Jon Hemming]

Comment on attachment 459767 [details] [diff] [review]
Makes suspending when receives "browser-suspend" and continues with "browser-continue"

Created against Mozilla Central revision: 58101a16aff7

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

just wanted to point out bug 579982 which is related. It adds PauseChild() and ResumeChild() to ContentParent which gets called from Android's onPause() and onResume() notification callbacks .

Comment 5

[Alon Zakai (:azakai)]

Jon, if I understand correctly, then after the screen goes off, the child process will be frozen? What if it needs to continue working, though - like if it is doing some calculation, or playing music, etc.?

Or maybe I don't understand what Maemo::QmDisplayState means (is it something more severe than just turning off the display)?

Comment 6

[Jon Hemming]

(In reply to comment #5)
> Jon, if I understand correctly, then after the screen goes off, the child
> process will be frozen? What if it needs to continue working, though - like if
> it is doing some calculation, or playing music, etc.?
> 

Patches that I put work together, so first "system-display-off" is notified on xulrunner side when we detect that "display-off" is coming from Maemo::QmDisplayState. This is just a notification to which different parts can react. We will react to this "system-display-off" on fennec side and start a single shot timer function call to notify about "browser-suspend" which will happen after time interval set by user. This "browser-suspend" is then observed in ContentParent which will suspend the content process. So as an end result we have mechanism that will suspend the content process after specified time set by user in settings. As a default this is now set to 30min, since it is something that should not happen straight when display goes off. This now ensures that browser wont continue endlessly as a default, but user may set this to whatever value he/she likes. There could be of course a notification to user also that he would now what has happened and change setting to preferred value.

> Or maybe I don't understand what Maemo::QmDisplayState means (is it something
> more severe than just turning off the display)?

No it isn't more server so that is why we have this separate mechanism to decide when to suspend content.

Comment 7

[Alon Zakai (:azakai)]

Ok, I see. I think with a proper notification to the user this would indeed be very useful.

Meanwhile I have put up a wiki page to summarize the various OS options:

https://wiki.mozilla.org/Mobile/PowerManagement

so we should update that and when that is done, we can define nice interfaces for all this stuff.

Comment 8

[Miika Jarvinen]

I added information about maemo specific things to that wikipage. There might be slight problem with usage of QmDisplayState, as it's not available for N900. If support for maemo5 qt is wanted, it would maybe better to listen for D-Bus messages sent by bme/mce or use bme/mce libraries.

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 459769 [details] [diff] [review]
Fennec patch for making the "browser-suspend" and "browser-continue" notifies


>diff -r 52f539338319 chrome/content/browser.js

>+var DisplayStateObserver = {

>+      // Just to be on safe side, let's initialize suspending to happen after 30min
>+      var in_mseconds = 1800000;

30 minutes is fairly large, but maybe it needs to be large

>+      var prefs = Components.classes["@mozilla.org/preferences-service;1"]
>+                            .getService(Components.interfaces.nsIPrefBranch);
>+      if(prefs) {
>+        var minutes = prefs.getIntPref("browser.idle.suspend.after");

Use | Services.prefs |

>+        if(typeof minutes == "number") {
>+          in_mseconds = minutes*60*1000;
>+        }

nits:
* Space after "if"
* No {} for a single line "if/"else" block
* Spaces between operators

          if (typeof minutes == "number")
            in_mseconds = minutes * 60 * 1000;

>+        var os = Cc["@mozilla.org/observer-service;1"].getService(Ci.nsIObserverService);
>+        if(os) {
>+          os.notifyObservers(null, "browser-continue", null);

Use | Services.obs |

>+  },
>+  sendSuspendNotification: function dso_sendIdleNotification() {

nit: Add a line break between methods

>+    var os = Cc["@mozilla.org/observer-service;1"].getService(Ci.nsIObserverService);
>+    os.notifyObservers(null, "browser-suspend", null);

Same

>+    _idle = true;

I think you want | this._idle = true; |

>diff -r 52f539338319 chrome/content/browser.xul

>+                <setting pref="browser.idle.suspend.after" type="integer" title="&suspendAfter.title;"/>

I am not sure we want to expose as a visible pref. Let's get some UX feedback.

r- for the small issues, but it looks OK. I'd like to establish:
* We want to take this kind of feature.
* How/if to expose in the UI

Comment 10

[Alon Zakai (:azakai)]

Comment on attachment 459767 [details] [diff] [review]
Makes suspending when receives "browser-suspend" and continues with "browser-continue"

I think this is very good. We do need to decide about the specific names and definitions for the notifications (browser-suspend, etc.), though. I think we should do that on the wiki page linked to before.

Code looks good otherwise, r+ once we decide on the names and definitions.

Comment 11

[Alon Zakai (:azakai)]

In https://bugzilla.mozilla.org/show_bug.cgi?id=568054#c6 there is a patch for a nice scriptable interface for freezing processes (also throttling).

If feedback for that is ok, then the next step would be to change the patches in this bug to use that framework (which would simplify them).

Comment 12

[Alon Zakai (:azakai)]

The approach of suspending the content process has not been accepted (see bug 568054 for details). So this particular issue should be handled in an OS-specific way as best we can.

Maybe for Meego it would be simpler to suspend all of Firefox when the OS tells us we can do that, and not just the content process? In other words all or most of this could be done from the outside (see bug 572329 for similar stuff).

Comment 13

[Jon Hemming]

(In reply to comment #12)
> Maybe for Meego it would be simpler to suspend all of Firefox when the OS tells
> us we can do that, and not just the content process? In other words all or most
> of this could be done from the outside (see bug 572329 for similar stuff).
In MeeGo the OS expects us to go idle after some time, when device is not used, but doesn't actually tell us to do so. We are still missing definition for "browser-idle" that could be used for putting browser to idle. Should we create a bug for creating a setting for minutes after browser will be set to completely idle. Could we also create "soft idle" which would be used before that. It would at least suspend all the timers that are not needed when user is not interacting and wouldn't hurt user experience to do so.

Comment 14

[Jon Hemming]

Of course this "soft idle" could be "system-display-dimmed-or-off" is used for unix or either "system-display-off" or "system-display-dimmed" which are separated in bug 580588. At least display going off would always mean that user is not going to be interacting with the browser.

Comment 15

[Alon Zakai (:azakai)]

I think the display dimming is all we need in the way of notifications. But I am unsure how much we can do at that point. Suspending all the timers would impact stuff that expects to continue running (downloads, web pages that the user wants to continue running - calculations, music, etc.). The display going off does mean the user isn't looking at the browser, but not necessarily more than that I think.

One approach for what to do when the display goes off is in bug 592751.

Comment 16

[Jon Hemming]

I suggest that we open new bug for handling browser going to full idle and continue conversation over there and maybe make new bug for actions when "system-display-dimmed" meaning "soft idle". There are things like animation suspending, idle connection handling, timer in nsPlacesDBFlusher, nsUITimerCallback bug 508518 , which some of them can be improved by better design and some can be suspended when "soft idle" and some need total stop when full idle. Still I would prefer some other notification of this "soft idle" instead of using "system-display-dimmed". Then it would be more clear why some action are done. Using dimmed can raise question marks of why some timer is stopped because of it. This would also give better way for different platforms to define this "soft idle". For instance MeeGo has signal of user not active that comes before this "system-display-dimmed". Also what would happen if some setting would enable not to dim the display? My suggestions would be to use something like "browser-inactive" and "browser-idle", former being the "soft idle" and latter being the full idle. Full idle should be something that could be set by user, since it would affect his/her user experience. For desktops we could either not implement full idle at all or put default setting to never suspend and for Mobile platforms we could set default value to some suitable value that would not be annoying to start with. Also there could be notification for first times about what happened when browser goes to full idle so that user would know how to tune this setting.

Comment 17

[Alon Zakai (:azakai)]

I prefer system-display-dimmed. It doesn't mean 'soft idle', but rather that the display is actually off - so all rendering to the screen (but nothing else, necessarily) can be suppressed.

A 'full-idle' state is not something all mobile OSes even have. It seems that it should be handled by those OSes themselves anyhow - if the device truly wants Firefox to be idle, it should pause or close Firefox (Android does basically that). Alternatively, we can handle this from outside of Firefox using something that freezes and unfreezes Firefox (doing the OS's work for it, in effect).

Comment 18

[Jon Hemming]

(In reply to comment #17)
> I prefer system-display-dimmed. It doesn't mean 'soft idle', but rather that
> the display is actually off - so all rendering to the screen (but nothing else,
> necessarily) can be suppressed.
> 

On MeeGo side(and I guess on mobile platforms in general) after "display-dimmed" you can still read from display quite well so I guess we would need to use this "system-display-off". I don't know if we could change "system-display-dimmed-or-off" used on unix side to be only "system-display-off", if that is basically meaning the same thing or shall we use ("system-display-dimmed-or-off" || "system-display-off") in every check?

Comment 19

[Alon Zakai (:azakai)]

Yeah, I meant 'off' and not 'dimmed' before. Not sure if anything can be done if the display is just dimmed and not off.