Last Comment Bug 544240 - Idle service does not work on Qt on mobile platforms
: Idle service does not work on Qt on mobile platforms
Status: RESOLVED FIXED
:
Product: Core Graveyard
Classification: Graveyard
Component: Widget: Qt (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Mike Kristoffersen (:MikeK)
:
:
Mentors:
Depends on: 555313 670551 556260 602085
Blocks:
  Show dependency treegraph
 
Reported: 2010-02-04 06:19 PST by Doug Turner (:dougt)
Modified: 2016-07-11 21:54 PDT (History)
6 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
This version only works on QT builds - will break other builds, compile time or runtime (31.80 KB, patch)
2010-02-04 06:58 PST, Mike Kristoffersen (:MikeK)
no flags Details | Diff | Splinter Review
Now with cross-platform support (108.60 KB, patch)
2010-02-11 16:03 PST, Mike Kristoffersen (:MikeK)
sdwilsh: review-
Details | Diff | Splinter Review
Patch updated to trunk - review comments not addressed (yet) (52.19 KB, patch)
2010-03-09 06:42 PST, Mike Kristoffersen (:MikeK)
no flags Details | Diff | Splinter Review
Review comments addressed, updated to trunk as of this morning, but failing mochitest (56.11 KB, patch)
2010-03-16 14:46 PDT, Mike Kristoffersen (:MikeK)
no flags Details | Diff | Splinter Review
No more known issues (61.59 KB, patch)
2010-03-18 14:34 PDT, Mike Kristoffersen (:MikeK)
sdwilsh: review+
Details | Diff | Splinter Review
Updated patch (62.20 KB, patch)
2010-03-26 14:36 PDT, Mike Kristoffersen (:MikeK)
no flags Details | Diff | Splinter Review
Updated to trunk (61.28 KB, patch)
2010-03-30 14:22 PDT, Mike Kristoffersen (:MikeK)
no flags Details | Diff | Splinter Review
Includes the patch from bug 556260 (61.29 KB, patch)
2010-03-31 12:04 PDT, Mike Kristoffersen (:MikeK)
no flags Details | Diff | Splinter Review
Updated to trunk (60.22 KB, patch)
2010-04-13 13:31 PDT, Mike Kristoffersen (:MikeK)
no flags Details | Diff | Splinter Review
Yet again updated to trunk (60.22 KB, patch)
2010-04-15 13:39 PDT, Mike Kristoffersen (:MikeK)
no flags Details | Diff | Splinter Review

Description Doug Turner (:dougt) 2010-02-04 06:19:01 PST
because some installations of Qt do not have screensaver support, we need to do something similar that we did for windows mobile and Gtk2.
Comment 1 Oleg Romashin (:romaxa) 2010-02-04 06:48:26 PST
Any patch? I know it exists ;)
Comment 2 Mike Kristoffersen (:MikeK) 2010-02-04 06:58:26 PST
Created attachment 425203 [details] [diff] [review]
This version only works on QT builds - will break other builds, compile time or runtime
Comment 3 Mike Kristoffersen (:MikeK) 2010-02-11 16:03:24 PST
Created attachment 426597 [details] [diff] [review]
Now with cross-platform support

Waiting for the try server to come up with some results, will post the result when I have it.
Comment 4 Mike Kristoffersen (:MikeK) 2010-02-11 16:14:04 PST
Oh, btw.  can someone help check/test this patch on WinMo?  (Especially that it doesn't introduce more checker boarding)  (OS X test will also be welcome, but might be harder)
Comment 5 Doug Turner (:dougt) 2010-02-25 11:16:52 PST
Comment on attachment 426597 [details] [diff] [review]
Now with cross-platform support

sdwilsh, can you take a first look?
Comment 6 Shawn Wilsher :sdwilsh 2010-03-08 13:37:21 PST
Comment on attachment 426597 [details] [diff] [review]
Now with cross-platform support

A bunch of this patch didn't apply cleanly, making it difficult to review.  Please address these comments, and then update it to tip of mozilla-central and I'll take a look again.  Comments with context can be found at http://reviews.visophyte.org/r/426597/

on file: widget/public/nsIIdleService.idl line 97
>     /**
>      * Resets the idle timeout, telling the system that activity has happend
>      * @note
>      * This won't tell the underlying OS that activity has happend, so it won't
>      * prevent screensavers etc. from starting
>      */
>     void resetIdleTimeOut();

This should probably go on a new interface that is only QIable from binary
code and not on this.  We don't want anyone to be able to do this really.


on file: widget/src/cocoa/nsIdleServiceX.h line 47
>   PRBool PollIdleTime(PRUint32* idleTime);

s/PRBool/bool/ please


on file: widget/src/gtk2/nsIdleServiceGTK.h line 25
>  *  Gijs Kruitbosch <gijskruitbosch@gmail.com>

er?


on file: widget/src/gtk2/nsIdleServiceGTK.cpp line 25
>  *  Gijs Kruitbosch <gijskruitbosch@gmail.com>

?


on file: widget/src/xpwidgets/nsIdleService.h line 97
>     // If you overwrite ResetIdleTimeout, you MUST call this implementation, or
>     // there is a risk that the system won't wake up again, after user activity
>     // resumes

I don't see why anyone would want to override this, and we could always add
hooks for stub classes.  We should use the NS_FINAL (or something like that)
attribute so static analysis can check this.


on file: widget/src/xpwidgets/nsIdleService.h line 106
>     // If there is a platform specific function to poll the system idel time
>     // then that must be returned in this function, and the function MUST return
>     // PR_TRUE, otherwise then the function should be left unimplemented or made
>     // to return PR_FALSE (this can also be used for systems where it depends on
>     // the configuration of the system if the idle time can be determined)
>     //
>     // NOTE: The time returned by this function can be different than the one
>     // returned by GetIdleTime, as that is corrected by any calls to
>     // ResetIdleTimeOut(), unless you overwrite that function too...
>     //
>     // The time returned in idleTime is meassured in ms

proper java-doc style please


on file: widget/src/xpwidgets/nsIdleService.h line None

s/PRBool/bool/


on file: widget/src/xpwidgets/nsIdleService.h line None

s/PRBool/bool/


on file: widget/src/xpwidgets/nsIdleService.h line None

Proper java-doc style please.  Also, indicating which timer would be useful
(we have daily, idle, etc)


on file: widget/src/xpwidgets/nsIdleService.h line None

Proper java-doc style please


on file: widget/src/xpwidgets/nsIdleService.h line 132
>     // Place to hold the timer

Timer for what?


on file: widget/src/xpwidgets/nsIdleService.h line 138
>     // Contains the time of the last idle reset or 0 there haven't been a reset
>     // Time is kept in seconds
>     PRUint32 mLastIdleReset;
> 
>     // The time since the last handled activity (which might be different than
>     // mLastIdleReset, since the activity that reset the idle timer could just
>     // have happend, and not handled yet)
>     // Time is kept in seconds since the epoch at midnight, January 1, 1970 UTC
>     PRUint32 mLastHandledActivity;

We usually use this style for multiline comments:
/**
 * comment
 */


on file: widget/src/xpwidgets/nsIdleService.cpp line 1
> /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */

You are making this file inconsistent by making this change.  Please keep it
at 4.


on file: widget/src/xpwidgets/nsIdleService.cpp line 81
>   nsCOMPtr<nsIObserverService> observerService =
>                                do_GetService("@mozilla.org/observer-service;1");

only indent one level.  this indentation looks odd


on file: widget/src/xpwidgets/nsIdleService.cpp line None

nit: closing parent on a new line


on file: widget/src/xpwidgets/nsIdleService.cpp line 144
> NS_IMETHODIMP nsIdleService::AddIdleObserver(nsIObserver* aObserver,
>                                              PRUint32 aIdleTime)

NS_IMETHODIMP should be on its own line (in general, commenting only once)
Comment 7 Mike Kristoffersen (:MikeK) 2010-03-09 06:37:10 PST
(In reply to comment #6)
> (From update of attachment 426597 [details] [diff] [review])
> A bunch of this patch didn't apply cleanly, making it difficult to review. 
> Please address these comments, and then update it to tip of mozilla-central and
> I'll take a look again.  Comments with context can be found at
> http://reviews.visophyte.org/r/426597/

Yeah, that tends to happen when patches are laying around for a month after the review has been requested :)  (not your fault, I blame the system) - I have merged it to trunk, will post in a minute.

> This should probably go on a new interface that is only QIable from binary
> code and not on this.  We don't want anyone to be able to do this really.

Nah, I guess it could be miss-used by bad guys, I'll investigate how to prevent the exposure of an interface.


> 
> on file: widget/src/cocoa/nsIdleServiceX.h line 47
> >   PRBool PollIdleTime(PRUint32* idleTime);
> 
> s/PRBool/bool/ please

We can use bool, I forgot that - great, this is one change I'll be more than happy to apply to the patch :)
 
> 
> on file: widget/src/gtk2/nsIdleServiceGTK.h line 25
> >  *  Gijs Kruitbosch <gijskruitbosch@gmail.com>
> 
> er?

That comment/question, didn't make much sense to me ?

> on file: widget/src/xpwidgets/nsIdleService.h line 97
> >     // If you overwrite ResetIdleTimeout, you MUST call this implementation, or
> >     // there is a risk that the system won't wake up again, after user activity
> >     // resumes
> 
> I don't see why anyone would want to override this, and we could always add
> hooks for stub classes.  We should use the NS_FINAL (or something like that)
> attribute so static analysis can check this.

It could be relevant to override if the underlying system/OS have some mechanism that should be triggered in the case of software generated user activity.

> 
> proper java-doc style please

np

> 
> on file: widget/src/xpwidgets/nsIdleService.h line None
> 
> Proper java-doc style please.  Also, indicating which timer would be useful
> (we have daily, idle, etc)

java-doc, np - I don't understand the second half of the comment?

> We usually use this style for multiline comments:
> /**
>  * comment
>  */

I'll change it, in this patch

> 
> on file: widget/src/xpwidgets/nsIdleService.cpp line 1
> > /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
> 
> You are making this file inconsistent by making this change.  Please keep it
> at 4.

As this file is basically being rewritten, I was under the impression that we should move it to a 2 space indentation, to comply with the style guide:

"The following norms should be followed for new code, and for Tower of Babel code that needs cleanup."

But I rather change it to 4 space indentation than getting into a discussion about it?

> on file: widget/src/xpwidgets/nsIdleService.cpp line 81
> >   nsCOMPtr<nsIObserverService> observerService =
> >                                do_GetService("@mozilla.org/observer-service;1");
> 
> only indent one level.  this indentation looks odd

ok, I suppose you mean, like:

  // Notify anyone who cares
  nsCOMPtr<nsIObserverService> observerService =
    do_GetService("@mozilla.org/observer-service;1");
Comment 8 Mike Kristoffersen (:MikeK) 2010-03-09 06:42:17 PST
Created attachment 431352 [details] [diff] [review]
Patch updated to trunk - review comments not addressed (yet)

Note that in order to build qt on the current trunk, you also need to apply the patch to bug 551149.

Patch with the review comments addressed will follow.
Comment 9 Shawn Wilsher :sdwilsh 2010-03-09 08:46:57 PST
(In reply to comment #7)
> Nah, I guess it could be miss-used by bad guys, I'll investigate how to prevent
> the exposure of an interface.
You'll want to create a UID for the native class that you can QI to (it's a safe way to cast).  It's pretty simple; here's an interface that I recently did this too:
http://mxr.mozilla.org/mozilla-central/source/docshell/base/IHistory.h

> We can use bool, I forgot that - great, this is one change I'll be more than
> happy to apply to the patch :)
Just don't use it in places where we can cross xpconnect.  Those places we can't do it with.

> That comment/question, didn't make much sense to me ?
Diff was showing me a change on that line, but I don't see one, so you changed some invisible character.

> It could be relevant to override if the underlying system/OS have some
> mechanism that should be triggered in the case of software generated user
> activity.
Sure, but we can always add hooks for it in the future.  For now, we should protect it (it also makes sure invariants are checked when/if someone does need it)

> java-doc, np - I don't understand the second half of the comment?
"timer" is vague; I'd like a slightly more detailed description (looking to help with other people being able to jump into this code)

> As this file is basically being rewritten, I was under the impression that we
> should move it to a 2 space indentation, to comply with the style guide:
I'm fine with it if you change the whole file.  I saw parts that had four spaces still.  You also forgot to update the vim modeline.  If you do decide to update the whole file, please do it in a separate patch (but in this bug is fine).  No reason to make the actual changes patch bigger with whitespace changes.

> ok, I suppose you mean, like:
>   // Notify anyone who cares
>   nsCOMPtr<nsIObserverService> observerService =
>     do_GetService("@mozilla.org/observer-service;1");
Bingo.
Comment 10 Mike Kristoffersen (:MikeK) 2010-03-16 14:46:15 PDT
Created attachment 432922 [details] [diff] [review]
Review comments addressed, updated to trunk as of this morning, but failing mochitest

See this as a progress report, I'm trying to figure out why its currently failing some of the test on the tryserver...

Fits:

changeset:   39479:050887c64183
tag:         tip
user:        Marco Bonardo <mbonardo@mozilla.com>
date:        Tue Mar 16 02:02:50 2010 +0100
summary:     Bug 552386 - test_history_sidebar.js fails between midnight and 2:00am with new DST settings, r=dietrich a=dholbert for CLOSED TREE
Comment 11 Mike Kristoffersen (:MikeK) 2010-03-18 14:34:01 PDT
Created attachment 433425 [details] [diff] [review]
No more known issues
Comment 12 Shawn Wilsher :sdwilsh 2010-03-25 12:00:24 PDT
Comment on attachment 433425 [details] [diff] [review]
No more known issues

http://reviews.visophyte.org/r/433425/
on file: widget/src/gtk2/nsWindow.cpp line 264
> UpdateLastInputEventTime() {

nit: brace on new line


on file: widget/src/qt/nsWindow.cpp line 1261
>     // The user has done something

nit: use punctuation too please! (in all places)


on file: widget/src/qt/nsWindow.cpp line 2526
>   nsCOMPtr<nsIdleService> idleService =
>    do_GetService("@mozilla.org/widget/idleservice;1");

you probably want to cache this and not do a do_GetService every time we call
this


on file: widget/src/xpwidgets/nsIdleService.h line 83
>    * @param aIdleService pointer to the idle service

nit: description goes on line after aIdleService, lined up with aIdleService


on file: widget/src/xpwidgets/nsIdleService.h line 95
>    * NOTE: This is a normal pointer, or the idle service could keep it self
>    * alive

nit: @note


on file: widget/src/xpwidgets/nsIdleService.h line 139
>    * @param idleTime The idle time in ms

nit: aIdleTime
nit: The on next line


on file: widget/src/xpwidgets/nsIdleService.h line 143
>    * @note
>    * The time returned by this function can be different than the one
>    * returned by GetIdleTime, as that is corrected by any calls to
>    * ResetIdleTimeOut(), unless you overwrite that function too...

usually we do it like this:
@note The time...
      ....
      ....


on file: widget/src/xpwidgets/nsIdleService.h line 169
>    * @param aDelay The time in seconds that should pass before the next
>    *               timeout

nit: new line for description


on file: widget/src/xpwidgets/nsIdleService.h line 208
>    * <p>

what is the <p>?


on file: widget/src/xpwidgets/nsIdleService.cpp line 132
>     MAX_IDLE_POLL_INTERVAL);

nit: closing paren on new line, no indent


on file: widget/src/xpwidgets/nsIdleService.cpp line 233
>   PRUint32 timeSinceReset = PR_IntervalToSeconds(PR_IntervalNow()) -
>                                                                 mLastIdleReset;

weird indent here
also, I wonder if we should be using TimeStamp now


on file: widget/src/xpwidgets/nsIdleService.cpp line 244
>   *idleTime = PR_MIN(timeSinceReset * 1000, polledIdleTimeMS);

use NS_MIN, not PR_MIN


r=sdwilsh
Comment 13 Mike Kristoffersen (:MikeK) 2010-03-26 06:08:49 PDT
(In reply to comment #12)
> (From update of attachment 433425 [details] [diff] [review])
> http://reviews.visophyte.org/r/433425/
<SNIP>
> on file: widget/src/xpwidgets/nsIdleService.h line 208
> >    * <p>
> 
> what is the <p>?

Paragraph break, according to http://java.sun.com/j2se/javadoc/writingdoccomments/ that is
refered to in our Coding Style ( https://developer.mozilla.org/En/Developer_Guide/Coding_Style )

> 
> 
> on file: widget/src/xpwidgets/nsIdleService.cpp line 132
> >     MAX_IDLE_POLL_INTERVAL);
> 
> nit: closing paren on new line, no indent

Like this?:

  static_cast<nsIdleServiceDaily*>(aClosure)->mIdleService->AddIdleObserver(
    static_cast<nsIdleServiceDaily*>(aClosure),
    MAX_IDLE_POLL_INTERVAL
  );

> on file: widget/src/xpwidgets/nsIdleService.cpp line 233
> >   PRUint32 timeSinceReset = PR_IntervalToSeconds(PR_IntervalNow()) -
> >                                                                 mLastIdleReset;
> 
> weird indent here
> also, I wonder if we should be using TimeStamp now

TimeStamp ?

> r=sdwilsh

Thanks for the review, updated patch pending...
Comment 14 Shawn Wilsher :sdwilsh 2010-03-26 06:55:26 PDT
(In reply to comment #13)
> Paragraph break, according to
> http://java.sun.com/j2se/javadoc/writingdoccomments/ that is
> refered to in our Coding Style (
> https://developer.mozilla.org/En/Developer_Guide/Coding_Style )
Neat.  I have never seen that before.

>   static_cast<nsIdleServiceDaily*>(aClosure)->mIdleService->AddIdleObserver(
>     static_cast<nsIdleServiceDaily*>(aClosure),
>     MAX_IDLE_POLL_INTERVAL
>   );
Yes!  Although it might also make sense to make a temporary here since you static_cast the same thing to the same type twice.

> TimeStamp ?
http://mxr.mozilla.org/mozilla-central/source/xpcom/ds/TimeStamp.h
the NSPR stuff has some gotchas so roc wrote a class that hides those gotchas.  We are supposed to try to use this when doing time difference calculations, so I think we should probably be using it here as well.
Comment 15 Mike Kristoffersen (:MikeK) 2010-03-26 13:18:28 PDT
> >   static_cast<nsIdleServiceDaily*>(aClosure)->mIdleService->AddIdleObserver(
> >     static_cast<nsIdleServiceDaily*>(aClosure),
> >     MAX_IDLE_POLL_INTERVAL
> >   );
> Yes!  Although it might also make sense to make a temporary here since you
> static_cast the same thing to the same type twice.

And presto, it fitted on one line.

> 
> > TimeStamp ?
> http://mxr.mozilla.org/mozilla-central/source/xpcom/ds/TimeStamp.h
> the NSPR stuff has some gotchas so roc wrote a class that hides those gotchas. 
> We are supposed to try to use this when doing time difference calculations, so
> I think we should probably be using it here as well.

I'll have a look at that interface
Comment 16 Mike Kristoffersen (:MikeK) 2010-03-26 14:36:13 PDT
Created attachment 435273 [details] [diff] [review]
Updated patch

The movement of the code to use the "TimeStamp" interface will be done seperately, other issues should be addressed.
Comment 17 Mike Kristoffersen (:MikeK) 2010-03-26 14:56:31 PDT
Created bug 555313 to deal with the TimeStamp update.
Comment 18 Mike Kristoffersen (:MikeK) 2010-03-30 14:22:52 PDT
Created attachment 436015 [details] [diff] [review]
Updated to trunk

Fits:

changeset:   40013:3d16328f87ed
tag:         tip
user:        Brad Lassey <blassey@mozilla.com>
date:        Tue Mar 30 15:23:44 2010 -0400
summary:     update nspr to NSPR_HEAD_20100330 to pick up support for android r=ted
Comment 19 Mike Kristoffersen (:MikeK) 2010-03-31 00:57:17 PDT
Pushed as: http://hg.mozilla.org/mozilla-central/rev/df9ac939b708
Comment 20 Boris Zbarsky [:bz] (still a bit busy) 2010-03-31 11:17:27 PDT
Backed out because it regressed Ts (all platforms) and Ts shutdown (all platforms),
Comment 21 Mike Kristoffersen (:MikeK) 2010-03-31 12:04:50 PDT
Created attachment 436241 [details] [diff] [review]
Includes the patch from bug 556260
Comment 22 Mike Kristoffersen (:MikeK) 2010-03-31 12:08:11 PDT
Try server builds at: /tryserver-builds/mkristoffersen@mozilla.com-IdlePatchTake2
Comment 23 Shawn Wilsher :sdwilsh 2010-03-31 12:21:28 PDT
How about I r+ the patch in bug 556260 since that has a smaller, and simpler patch.  Feel free to merge them when you land next.
Comment 24 Mike Kristoffersen (:MikeK) 2010-03-31 12:35:02 PDT
Thats much more easy for everyone :)
Comment 25 Mike Kristoffersen (:MikeK) 2010-03-31 12:40:42 PDT
Here are some of talos results from the try sever
The patch had two runs, one before the orginal patch was backed out, and one after (where it was merged with the original patch)
"Others" mean other try server runs that are unrelated to this patch

MacOSX Darwin 9.0.0 try talos dirty:
ts_cold_generated_min
---------------------
Patch:
12473.74 / 11223,16
Others:
12397.26 / 10706.05 / 11541.0 / 10370.47

ts_places_generated_max
-----------------------
Patch:
819.42 / 835.58

Others:
831.11 / 938.74 / 961.84 / 926.53

WINNT 5.1 try talos dirty
*************************
ts_cold_generated_min
---------------------
Patch: 
513.21 / 514.74
Others:
536.89 / 516.26 / 513.63 / 513.05 / 515.05

ts_places_generated_max
-----------------------
Patch:
515.47 / 517.84

Others:
515.68 / 560.0 / 515.37 / 513.42
Comment 26 Mike Kristoffersen (:MikeK) 2010-03-31 12:41:59 PDT
(In reply to comment #25)
> ts_cold_generated_min
should be read as ts_cold_generated_max
Comment 27 Mike Kristoffersen (:MikeK) 2010-03-31 14:00:16 PDT
Ok, might be a little complicated - the current patch is the updated r+ one patched with the r+ patch from bug 556260.

In other words, this is the patch as it should have been and sdwilsh (the reviewer) has OK'ed that the two patches was merged and submitted as one - any volunteers to push it are welcome :)
Comment 28 Mike Kristoffersen (:MikeK) 2010-04-13 13:31:11 PDT
Created attachment 438834 [details] [diff] [review]
Updated to trunk

Fits:

changeset:   40733:caa4eb12ad20
tag:         tip
user:        Robert Longson <longsonr@gmail.com>
date:        Tue Apr 13 09:58:09 2010 +0100
summary:     Bug 456286 - support altGlyph elements as tspans. r=roc
Comment 29 Mike Kristoffersen (:MikeK) 2010-04-13 13:33:40 PDT
Removed the chekin-needed flag as I haven't checked if the updating to trunk changed the performance of the previous patch - try server run seems ok.
Comment 30 Mike Kristoffersen (:MikeK) 2010-04-15 13:39:10 PDT
Created attachment 439330 [details] [diff] [review]
Yet again updated to trunk
Comment 31 Doug Turner (:dougt) 2010-04-16 14:36:33 PDT
pushed:
  http://hg.mozilla.org/mozilla-central/rev/57f0bc3d9edb
Comment 32 Mike Kristoffersen (:MikeK) 2010-04-21 16:48:59 PDT
No ill affects seems to have happened this time, closing it :)
Comment 33 Jon Hemming 2010-09-02 06:11:15 PDT
Comment on attachment 439330 [details] [diff] [review]
Yet again updated to trunk

>diff --git a/widget/src/qt/nsWindow.cpp b/widget/src/qt/nsWindow.cpp
>--- a/widget/src/qt/nsWindow.cpp
>+++ b/widget/src/qt/nsWindow.cpp
>@@ -1241,20 +1242,23 @@ nsWindow::InitButtonEvent(nsMouseEvent &
> nsEventStatus
> nsWindow::OnButtonPressEvent(QGraphicsSceneMouseEvent *aEvent)
> {
>+    // The user has done something.
>+    UserActivity();
>+
...
>@@ -1288,20 +1292,23 @@ nsWindow::OnButtonPressEvent(QGraphicsSc
> nsEventStatus
> nsWindow::OnButtonReleaseEvent(QGraphicsSceneMouseEvent *aEvent)
> {
>+    // The user has done something.
>+    UserActivity();
>+
...
> nsEventStatus
> nsWindow::OnKeyPressEvent(QKeyEvent *aEvent)
> {
>     LOGFOCUS(("OnKeyPressEvent [%p]\n", (void *)this));
> 
>+    // The user has done something.
>+    UserActivity();
>+
...
> nsEventStatus
> nsWindow::OnKeyReleaseEvent(QKeyEvent *aEvent)
> {
>     LOGFOCUS(("OnKeyReleaseEvent [%p]\n", (void *)this));
> 
>+    // The user has done something.
>+    UserActivity();
>+
...
>@@ -2483,10 +2496,22 @@ nsWindow::SetIMEEnabled(PRUint32 aState)
> NS_IMETHODIMP
> 
>+void
>+nsWindow::UserActivity()
>+{
>+  if (!mIdleService) {
>+    mIdleService = do_GetService("@mozilla.org/widget/idleservice;1");
>+  }
>+
>+  if (mIdleService) {
>+    mIdleService->ResetIdleTimeOut();
>+  }
>+}
>+

Was there a reason to just include button/key press/release as UserActivity? Now if browser is scrolled with button constantly pressed, observers are notified of idle state, since idle time just keeps going on.
Comment 34 Jon Hemming 2010-09-02 07:38:45 PDT
and could this idle service be implemented without polling? I have added already a bug 584660 about not using it for Maemo 6 platform.
Comment 35 Mike Kristoffersen (:MikeK) 2010-09-06 04:41:15 PDT
(In reply to comment #33)
> 
> Was there a reason to just include button/key press/release as UserActivity?
> Now if browser is scrolled with button constantly pressed, observers are
> notified of idle state, since idle time just keeps going on.

Not as far as I remember...


(In reply to comment #34)
> and could this idle service be implemented without polling? I have added
> already a bug 584660 about not using it for Maemo 6 platform.

It depends on the platform, in the platform specific code nsIdleService<PLATFORM> the return value of the PollIdleTime function determines if that platform needs to run in poll mode or not.

Some platforms might need to run in polling mode and others don't have to.
Comment 36 Jon Hemming 2010-09-08 22:49:54 PDT
(In reply to comment #35)
> (In reply to comment #33)
> > 
> > Was there a reason to just include button/key press/release as UserActivity?
> > Now if browser is scrolled with button constantly pressed, observers are
> > notified of idle state, since idle time just keeps going on.
> 
> Not as far as I remember...
> 
Ok added bug 594662 for counting scrolling and dragging also as interaction.

Note You need to log in before you can comment on or make changes to this bug.