Comment on attachment 8404230 Implement WakeLockListener on Linux to disable screensaver while video is playing
The main thing missing here is appropriate handling of this situation: After a InhibitScreenSaver() call, WakeLockListener::Callback() is called with a non-"locked-foreground" state before WakeLockListenerStatusNotify() receives the cookie for mInhibitRequest. The Uninhibit message can't be sent until the cookie is received in the reply from Inhibit. The other things below should be easier to resolve. >+CFLAGS += $(MOZ_DBUS_GLIB_CFLAGS) This line can be dropped because only cpp files are involved here. >+using namespace mozilla; > > using mozilla::unused; I assume the first line here makes the other unnecessary. >+ char const *const sReason = "Media Playback"; This is generic code inhibiting the screen saver on any "topic" in locked-foreground state, so mention of "Media Playback" seems strange. It seems that either Callback() should listen only for the "Playing_media" topic or should pass on each topic to the screen saver control (the cookie means that multiple inhibit/unhibit pairs will be handled appropriately). >+ mDesktopEnvironment++; Re a postfix ++ expression, N3242 says "The type of the operand shall be an arithmetic type or a pointer to a complete object type." gcc 4.7.3 says: > /home/karl/moz/dev/widget/gtk/nsAppShell.cpp: In member function 'void > WakeLockListener::InhibitFailed()': > /home/karl/moz/dev/widget/gtk/nsAppShell.cpp:156:32: error: no > 'operator++(int)' declared for postfix '++' [-fpermissive] >+ void Init(void) >+ { >+ if (mConnection) { >+ dbus_connection_setup_with_g_main(mConnection, nullptr); >+ } >+ } The dbus_connection_set_exit_on_disconnect() call seems to have been lost. If there is no lazy initialization, then there is no need to have a separate Init() function and moving the code into the constructor would make it clearer that this can only happen once. >+ GNOME, >+ FreeDesktop, >+ Unsupported, This is now a freedesktop spec and implemented on GNOME (comment 9) so try the FreeDesktop interface first and fall back to GNOME if not available. I assume the GNOME interface will be deprecated at some point. http://specs.freedesktop.org/idle-inhibit-spec/latest/re01.html >+ DBusConnection* mConnection; This needs a dbus_connection_unref(mConnection). >+ // Keep track of all the topics that have requested a wake lock. When the >+ // number of topics in the hashtable reaches zero, we can uninhibit the >+ // screensaver again. >+ nsTHashtable<nsStringHashKey> mLockedTopics; If only one topic is handled, then this hash table should no longer be necessary. If multiple topics are handled then this could hold the cookies. >+ nsCOMPtr<nsIPowerManagerService> powerManagerService; >+ > // make the pipe nonblocking >+ powerManagerService = do_GetService(POWERMANAGERSERVICE_CONTRACTID); Please declare at the initialization. -- You received this bug notification because you are a member of Desktop Packages, which is subscribed to firefox in Ubuntu. https://bugs.launchpad.net/bugs/434476 Title: screensaver starts while playing HTML5 videos Status in The Mozilla Firefox Browser: Confirmed Status in “firefox” package in Ubuntu: Confirmed Status in “firefox-3.5” package in Ubuntu: Triaged Bug description: Binary package hint: firefox-3.5 karmic alpha6 + updates - click on a .ogv video link - after watching some time, screensaver starts. ProblemType: Bug Architecture: amd64 Date: Tue Sep 22 08:45:53 2009 DistroRelease: Ubuntu 9.10 NonfreeKernelModules: nvidia Package: firefox 3.5.3+build1+nobinonly-0ubuntu2 PackageArchitecture: all ProcEnviron: LANG=de_AT.UTF-8 SHELL=/bin/bash ProcVersionSignature: Ubuntu 2.6.31-10.34-generic SourcePackage: firefox-3.5 Uname: Linux 2.6.31-10-generic x86_64 To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/434476/+subscriptions -- Mailing list: https://launchpad.net/~desktop-packages Post to : desktop-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~desktop-packages More help : https://help.launchpad.net/ListHelp