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

Reply via email to