Closed Bug 475361 Opened 11 years ago Closed 11 years ago
Need idle service on windows mobile
We have disabled it: http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/Makefile.in#109 and we see lots of assertions: WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80040154: file c:/builds/mobile/mozilla-central/toolkit/components/places/src/nsNavHistory.cpp, line 5196
The GetIdleTime() API measures the amount of time the processor has been idle, not the amount of time since user input. Since WinMobile devices have all sorts of timer routines going off every second, the GetIdleTime() return value is never very big at all. No, we need a different approach for setting up an idle timer. I have been awaiting our shipment of WinMobile alpha in order to debug my code for this bug.
thanks for the clarification.
Adds a new nsIIdleServiceWinCE service, and sub-classes the existing nsIdleTimeWin class. Adds a single function into nsWindow.cpp to reset "idleTime" when user interaction happens to a top-level window (mouse move, key press). Currently also resets idleTime for Input Method Editor (soft keypad) events.
Attachment #362007 - Flags: review?(pavlov)
wolfe, did you investigate using: http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsWindow.cpp#7990 might be a challenge to access it from the idle service implementation, but it is cleaner than having to have a separate interface and all of the gunk required to support this.
Comment on attachment 362007 [details] [diff] [review] v1.0 patch get rid of your printfs I don't really see any reason for ModifyLastInputTime() and #defining it -- the function isn't very big safe_compare should have a better name, and macros should be all caps. you should have firstname.lastname@example.org review the rest of the patch.
Attachment #362007 - Flags: review?(pavlov) → review-
Greatly simplified patch that should have no effect outside of WINCE builds, and only adds one global variable to WINCE builds. The global variable was already around as a static global variable inside nsWindow.cpp No need for a new service. No need for subclassing nsIdleServiceWin.
Attachment #366497 - Flags: review?(pavlov) → review?(emaijala)
Status: NEW → ASSIGNED
OS: Mac OS X → Windows CE
Hardware: x86 → ARM
Comment on attachment 366497 [details] [diff] [review] v2.0 patch - greatly simplified The non-WinCE part is broken. There's no lastInputTime. Did you mean to make it: *aTimeDiff = SAFE_COMPARE_EVEN_WITH_WRAPPING(GetTickCount(), inputInfo.dwTime); If so, r=me with that fixed.
Attachment #366497 - Flags: review?(emaijala) → review-
Fixes typo to nsIdleServiceWin.cpp source file patch - Ere was right. This patch is only a small, requested delta from the patch that makes this patch r=Ere
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Is there any reason not to build nsIdleServiceWin.cpp?
wolfe, how could this have been tested? ere, should we back this out?
I must have gotten the Makefile change imbedded into another patch, and not pulled a completely new tree to test the patch. This was in the middle of trying to get the testbed working, and I got lost walking through the architecture problems. This code was a small delta off what I had originally tested - but I will run through the testing again today (with nsIdleServiceWin.cpp building for WinCE as per Hiroyuki's patch) to double-check. I will post another comment when I have finished testing.
Please back out. And please don't check in without super review.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
This new patch superceeds the older patches. I have also run through a day of testing of this patch, and see where the code depending upon the idleService properly calls into the idleService. The idleTime returned uses the (formerly) static nsWindow.cpp variable gLastInputEventTime since the APO GetLastInputInfo() is not available on WinCE. Some things added to the v2.1 patch: * Always compile the nsIdleServiceWin.cpp source file when building for WinCE * Always generate the nsIdleServiceWin Generic Factory Constructor when building for WinCE * Always add the NS_IDLE_SERVICE_CID and service information to the listing of services when building for WinCE * Initialize the gLastInputEventTime global variable when a window is created - just in case the user starts XULRunner but does not produce and mouse or character events, an unlikely scenario. In this case we simply say that Window creation marks the last input event time. Apologies for the poorly tested v2.1 patch.
Comment on attachment 371717 [details] [diff] [review] v3.0 patch Just one nit: + // NOTE: nowTime is not necessarily equivalent to GetTickCount() return value + // we need to compare apples to apples - hence the nowTime variable + PRUint32 nowTime = PR_IntervalToMicroseconds(PR_IntervalNow()); I'd indent the comments four characters too. Please request super-review now.
This patch fixes the indentation spacing as requested by Ere Maijala. The previous attachment was r+ere
Comment on attachment 372348 [details] [diff] [review] v3.1 patch - fixes comment indentation I would leave gLastInputEventTime as static and use nsWindow::GetLastInputEventTime in the idle service.
Attachment #372348 - Flags: superreview?(pavlov) → superreview-
Stuart changed his sr- to sr+ after some discussion. changeset: 27326:ea400876d06c user: John Wolfe <email@example.com> date: Tue Apr 14 14:23:01 2009 -0400 summary: Bug 475361 - Need idle service on windows mobile - r+emaijala, sr+pavlov
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
clearing wanted1.9.1 flag. we need this for fennec.
Flags: wanted1.9.1? → wanted-fennec1.0?
clearing wanted flag;
You need to log in before you can comment on or make changes to this bug.