Comment 68 for bug 434476

Revision history for this message
In , Karlt (karlt) wrote :

Comment on attachment 693331
v2

How about disabling the screensaver only when "DOM_Fullscreen" and
"Playing_media" are both in "locked-foreground" state?
That would still lock the screen while a fullscreen window is playing audio,
but at least that reduces the number of situations considerably.

I don't want any DBus calls blocking on reply from the service, especially
during nsAppShell::Init(), which would be called during start-up.

I think it would be simplest, instead of testing different methods during
initialization, to wait until after calling "Inhibit" on the freedesktop
interface first. If an error is received from that call, then set some state to indicate that the freedesktop interface is not available and try again. When the freedesktop interface is not available, the gnome interface is tried. If that also fails, the state could even be updated to so that no interfaces are tried next time.

These situations need to be handled: After a screenSaverInhibit() call,
WakeLockListener::Callback() is called with a "locked-foreground" or other
state before WakeLockListenerStatusNotify() receives the cookie for
mInhibitRequest.

>+LOCAL_INCLUDES += $(MOZ_DBUS_CFLAGS)
>+CFLAGS += $(MOZ_DBUS_GLIB_CFLAGS)
>+CXXFLAGS += $(MOZ_DBUS_GLIB_CFLAGS) -DHAVE_PTHREADS

Do you know why -DHAVE_PTHREADS is here? I see one other instance of it
beside MOZ_DBUS_GLIB_CFLAGS in Gecko, but other places don't have it.
Please remove if it is not required.

I expect MOZ_DBUS_CFLAGS is unnecessary with MOZ_DBUS_GLIB_CFLAGS.

>+#include <dbus/dbus-glib.h>

Looks like this may be unnecessary.

>+ if(!topic.Equals(NS_LITERAL_STRING("DOM_Fullscreen")))

EqualsLiteral

Remember Gecko style is "if (".

>+nsCOMPtr<nsIPowerManagerService> sPowerManagerService = nullptr;
>+nsCOMPtr<nsIDOMMozWakeLockListener> sWakeLockListener = nullptr;

nsCOMPtr should not be used for global variables. (It runs constructors
at startup.) I don't know whether or not StaticRefPtr would work here.