Closed Bug 472548 Opened 15 years ago Closed 15 years ago

nsIdleService tests fail on maemo

Categories

(Core :: Widget: Gtk, defect)

ARM
Maemo
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jmaher, Assigned: dougt)

References

()

Details

(Keywords: fixed1.9.1)

Attachments

(1 file, 1 obsolete file)

in the chrome test suite there is chrome://mochikit/content/chrome/widget/test/test_bug343416.xul.  This test tests the nsIdleService by adding listeners to idle events and measuring the time it took to fire the events and receive the event.

Most likely this test should be disabled for fennec on devices.  Is this service useful on a mobile device?
Blocks: 473562
how does this test fail?

what is on the console?
(In reply to comment #2)
> how does this test fail?
> 
> what is on the console?

Had to use these steps to get chrome tests running.
https://bugzilla.mozilla.org/show_bug.cgi?id=421611#c40

file content_chrome.log :
http://mozilla.pastebin.com/fd7ec2f

console output:
http://mozilla.pastebin.com/f2d1a569d
Put in some printfs nsIdleServiceOSSO.cpp
 
http://mxr.mozilla.org/mozilla-central/source/widget/src/gtk2/nsIdleServiceOSSO.cpp

and some dump() in the test and wondering why
observe is not being called?

this is the output from running the test with the printf and dump
http://mozilla.pastebin.com/f5825f5ee

also trying to get gdb to work on the device
http://maemo.org/development/tools/#apt-example
got it installed but the symbols get stripped from the package
Zad, can you debug this on fennec desktop linux version.  If this is only failing on the maemo device, we can at least see what we expect on the desktop.
I attempted to run the test on linux based on these instructions
https://wiki.mozilla.org/Mobile/Fennec_Chrome

ran into this error:
error while loading shared libraries: libxpcom.so: cannot open shared object file: No such file or directory
Server pid: 27206
Timed out while waiting for server startup.

Is it possible that the observe is not getting registered on start-up?
on OSSO, we calculate the total time between "now" and when the device goes idle.

on Gtk-desktop, we calculate the total time between "now", and when we received the last mouse event (via the screensaver).

I think this difference is enough to cause some problems like you will see.  I think we can tweak the OSSO implementation to respond to both "screen dimming" and the system idle time event.
Attached patch patch v.1 (obsolete) — Splinter Review
this send the idle message when the screen dims and the active event when the screen gets powered on.

I posted a build here:
http://www.meer.net/users/dougt/random/fennec-1.0a3pre.en-US.linux-arm.tar.bz2

Please give it a try.
Assignee: nobody → doug.turner
IMHO, this is not the right thing to do. For one thing, this will duplicate the notifications, but without any context indicating whether the system-idle/active notification was caused by the display or the actual "idle" functionality of the device, possibly leading to interleaving notifications which would confuse apps.

For another, the test will still fail in this case if the display is not idle. What the test tries to ascertain is that, assuming that nobody touches the keyboard/mouse for the duration of the test, the idle time increases the "right" amount. You may well argue that the test is not very nice; I would welcome recommendations on how to improve the testing of this particular interface.

As for this particular problem, I believe it would be best solved my making the test send its own system-idle/active notifications in order to 'fool' the OSSO component into being idle, and then checking the idle time increases the right amount. This should be fairly trivial to implement in the test file as it stands, assuming there's an easy way to check whether or not we're on OSSO.
(In reply to comment #8)
> Created an attachment (id=363423) [details]
> patch v.1
> 
> this send the idle message when the screen dims and the active event when the
> screen gets powered on.
> 
> I posted a build here:
> http://www.meer.net/users/dougt/random/fennec-1.0a3pre.en-US.linux-arm.tar.bz2
> 
> Please give it a try.

update on the patch:
Attempted to run the chrome test on the build.
Ran into an issue with nsXPConnect 

http://mozilla.pastebin.com/fb16b08d

Thought it was my build to updated the tree and ran
into a build issue 
from /home/seneca/mozilla/mozilla-central/dom/src/geolocation/MaemoLocationProvider.cpp

http://mozilla.pastebin.com/f50c7c938
(In reply to comment #9)
> As for this particular problem, I believe it would be best solved my making the
> test send its own system-idle/active notifications in order to 'fool' the OSSO
> component into being idle, and then checking the idle time increases the right
> amount. This should be fairly trivial to implement in the test file as it
> stands, assuming there's an easy way to check whether or not we're on OSSO.

May I have more information on how fool the OSSO component?
> Thought it was my build to updated the tree and ran
> into a build issue 
> from
> /home/seneca/mozilla/mozilla-central/dom/src/geolocation/MaemoLocationProvider.cpp
> 
> http://mozilla.pastebin.com/f50c7c938

see patches in bug 478923
(In reply to comment #11)
> (In reply to comment #9)
> > As for this particular problem, I believe it would be best solved my making the
> > test send its own system-idle/active notifications in order to 'fool' the OSSO
> > component into being idle, and then checking the idle time increases the right
> > amount. This should be fairly trivial to implement in the test file as it
> > stands, assuming there's an easy way to check whether or not we're on OSSO.
> 
> May I have more information on how fool the OSSO component?

As I wrote in the email conversation we all had:
> 3) Make the test send a system-idle notification to enforce the
> correct state of mIdle. (hack! :-) ) It should then send another
> notification with the correct state (idle or active, depending on what
> the platform claims) after the test is done.

So presumably you run:

observerService.notifyObservers(null, "system-idle", null);
// See: http://mxr.mozilla.org/mozilla-central/source/xpcom/ds/nsIObserverService.idl#86

and after running the test you run the same thing with "system-active" as the topic to restore the original state - if necessary (ie if the system was not idle before).

This will still break the test if the system really returns from idle to active state while the test runs, and sends an system-active notification itself, but that is currently true for all other platforms that run the test, too (id est, it requires people to leave their machine untouched while the test runs).
Gij -- Why do you think the duplication of the event is a problem.  Did you look at the listeners and see what they are doing?

you said that my build that I mentioned in comment #8 worked.  Please confirm?
(In reply to comment #14)
> Gij -- Why do you think the duplication of the event is a problem.  Did you
> look at the listeners and see what they are doing?

I think there are two problems, actually, after more careful inspection:

a) Suppose:
   1. Display turns off. "system-idle" gets sent. mIdleTime == Now()
   2. I check the idle time using nsIIdleService.getIdleTime(); Suppose I get 2 minutes.
   3. System goes idle completely. "system-idle" gets sent again. mIdleTime == Now()
   4. I check the idle time again, this time I could get some number which is <= 2 minutes, even though the system has remained idle and the idle time should therefore have increased monotonically. In this case, if I have a listener on nsIIdleService that says I care about an idle time >= 2 minutes, I might mistakenly get a notification saying the user is no longer idle, while in fact the user *is* still idle. This is wrong.

b) Suppose:
   1. Display turns off. "system-idle" gets sent. mIdleTime == Now()
   2. System goes idle completely. "system-idle" gets sent again. mIdleTime == Now()
   3. System stops being idle, but doesn't turn on display (background process or something -- though I admit I can't tell if that is possible, never having used such a device nor knowing when exactly OSSO does or doesn't send these notifications). "system-active" gets sent.

Now a Mozilla app thinks the system is not idle. getIdleTime() will return 0. This is wrong, too.


You could potentially solve these problems by keeping two booleans (mIdleScreen and mIdleSystem or whatever) and two sets of notifications ("system-idle/active" and "screen-idle/active"), setting mIdleTime when either of the "idle" notifications arrives, not changing it if/when the second one does, and only resetting mIdleTime (claiming we're not idle) if *both* mIdleScreen and mIdleSystem are set to false.

Anwyay, I'm not sure what we would gain in that particular situation, presumably because I am not familiar enough with the OSSO platform.


> you said that my build that I mentioned in comment #8 worked.  Please confirm?

I'm assuming this was intended for someone else? I don't have a maemo device, so did not test anything, sorry.
Gij we probably can just use the screen to indicate system idle time (but should probably rename the notification to something else as you suggest).
Attached patch patch v.2Splinter Review
just listens to the display callback to indicate "idle" time.  this means, that on maemo/osso, a mozilla application is not idle until the screen is powered down or off.  I think that this is closer to what gtk "standard" does.
Attachment #363423 - Attachment is obsolete: true
Attachment #363693 - Flags: review?
Attachment #363693 - Flags: review? → review?(gijskruitbosch+bugs)
Attachment #363693 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 363693 [details] [diff] [review]
patch v.2

This looks good, but I'm not a widget peer/owner, so I believe you need r/sr from someone else as well. Your pick! :-)
Attachment #363693 - Flags: superreview?(pavlov)
Attachment #363693 - Flags: superreview?(pavlov) → superreview+
this gets the tests passing, i assume?
I went ahead and tested this (per the build from #8), and the test still fails.  I tried toying with the screen idle and device sleep and nothing seems to work.  In the end, I wonder if we are really understanding the problem.

Oddly enough, some of the console messages that I see are:

WARNING: SQL statement 'DELETE FROM moz_historyvisits_temp WHERE visit_type <> 4' was not finalized: file /home/dougt/builds/mobile/mozilla-central/storage/src/mozStorageConnection.cpp, line 254
WARNING: SQL statement 'DELETE FROM moz_places_temp WHERE id IN (SELECT id FROM moz_places_temp h WHERE h.hidden <> 1 OR NOT EXISTS ( SELECT id FROM moz_historyvisits_temp WHERE place_id = h.id AND visit_type = 4 LIMIT 1) )' was not finalized: file /home/dougt/builds/mobile/mozilla-central/storage/src/mozStorageConnection.cpp, line 254
WARNING: sqlite3_close failed. There are probably outstanding statements that are listed above!: file /home/dougt/builds/mobile/mozilla-central/storage/src/mozStorageConnection.cpp, line 267
************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "Component returned failure code: 0x80630001 [mozIStorageConnection.close]"  nsresult: "0x80630001 (<unknown>)"  location: "JS frame :: file:///media/mmc1/dougt/fennec/xulrunner/components/nsPlacesDBFlush.js :: anonymous :: line 135"  data: no]
************************************************************


It was mentioned in the fennec meeting today that bug 472597 might be related to the idleService.  

I guess I still don't understand the point of nsIdleService.  Who depends on it?  Wouldn't we want it to mirror that of firefox?  It seems like it isn't working on Maemo and we are conceding that the service will act much different than all other implementations of it.  This leads me to the point of making sure we fix the problem in the right way (which could be removing the service if it won't work).
I talked to dietrich and shawn and found out that the places temp tables are not created during idle time. So this bug should not be related to bug 472597.
Joel,
which test are you talking about?

Does the build in comment 8 fix:
chrome://mochikit/content/chrome/widget/test/test_bug343416.xul

Also, make sure that you have not disabled your screen dimming or turning off.  This was part of the instructions on setting up talos.
Sorry for not being specific, the test I am trying to see fixed is:
chrome://mochikit/content/chrome/widget/test/test_bug343416.xul

I ran this test in various way with the screen on/no dim, screen dim, device asleep, and adjusted the setTimeout to toggle between before/after the dim/asleep settings when used.  

In the end, no combination would result in returning something other than 0 for the idleTime.  As far as I can tell, this is the only test for idleTime that we have and it is failing.  

I don't mind adjusting the test to fix this problem if we understand why we are adjusting it.  There is also a chance that we have to ignore certain tests on the Fennec project as they don't apply.  If we have the idleService available, I would expect that we keep this test for Fennec.
joel, using the observer service, could you register for and see if you get the "system-display-dimmed-or-off" event?
(you will have to use the latest patch in this bug, not the build that I produced in comment #8)
Assignee: doug.turner → nobody
Component: General → Widget: Gtk
Flags: wanted-fennec1.0?
Product: Fennec → Core
QA Contact: general → gtk
Comment on attachment 363693 [details] [diff] [review]
patch v.2

I pushed patch v.2.  It improves Maemo's idle service: 

http://hg.mozilla.org/mozilla-central/rev/99a2c8d31818
Attachment #363693 - Flags: approval1.9.1?

the failure in the idle test is caused by the fact that the idle service on maemo returns 0 until the device actually is in idle mode.  Idle mode is indicated by the screen dimming or going off.  I also believe that CE may behave similarly (i objected to the current patches in bug 475361 where we have to create this hack to support such a feature).

This is unlike GTK w/ screensaver where input events are monitor and idle time is based on those inputs.  It isn't clear that we need such granularity of idle time and believe that the test case should be fixed until there is a real customer that needs to know fine grain idle time differences.
Thanks Doug!  This makes sense and I will work on disabling this test case for the fennec platform.  This is tracked in bug 464081
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Assignee: nobody → doug.turner
Attachment #363693 - Flags: approval1.9.1? → approval1.9.1+
clearing wanted flag; this landed on 1.9.1
Flags: wanted-fennec1.0?
You need to log in before you can comment on or make changes to this bug.