Comment 59 for bug 434476

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

Comment on attachment 681480
v1

IIUC this will inhibit the screensaver, even if there is an advert or audio
playing out of view on any foreground tab of an unminimized window.

I'm not sure it is reasonable to require users to minimize their windows to
allow the screensaver to activate. Perhaps there is some other mechanism to
prevent media playing until the user interacts. Better get feedback from the
media team on this. Does Gnome Shell let you minimize windows? Does it unmap
windows on background virtual desktops?

  Don't know whether only considering the active window would be better. On
  Android, I assume it doesn't make much difference because only one window
  will be visible at a time. At least considering only active windows would
  filter out windows on other virtual desktops. But the same question still
  remains about whether leaving an active window visible should allow the site
  to disable the screensaver.

  Seems we should only be disabling the screensaver on some user action,
  such as asking for (or allowing fullscreen).

Does the screensaver that you work with support org.freedesktop.ScreenSaver?
That sounds like the preferred API to use, but I haven't found official
documentation. It seems very similar to org.gnome.ScreenSaver.
http://lists.freedesktop.org/archives/xdg/2007-March/009187.html

Using a GNOME interface is OK if the freedesktop interface is not supported
and using the GNOME interface doesn't cause any new processes to start (in
sessions with different managers). What is the reason to prefer
gnome.SessionManager over gnome.ScreenSaver?

reason = "Fullscreen mode" isn't matching up with the conditions of calling
Inhibit. I agree fullscreen mode seems the more appropriate condition under
which to disable the screensaver.

We don't want to wait for the DBus server when starting to play, so the calls
should be asynchronous. Blocking on the inhibit reply before uninhibit should
be OK.

  When not using dbus_connection_send_with_reply_and_block, something will be
  necessary to actually flush the dbus send queue. dbus_connection_flush()
  doesn't seem quite ideal because it "blocks until it can write out the
  entire outgoing queue." I'm guessing dbus-glib's
  dbus_connection_setup_with_g_main will be the easiest way to look after
  that. We perhaps could get lucky that dbus_connection_setup_with_g_main has
  already been called elsewhere, but better to ensure it is called by adding
  another call here. Feel free to use other dbus-glib API if that makes
  things simpler. I don't really mind whether libdbus or dbus-glib functions
  are used because both are superseded or deprecated by GIO's GDBus, and GDBus
  is too new to use.

The concept of activating a WakeLock on notification that the foreground has
already been locked seems quite strange to me, but it does follow the Android
implementation. Best to get feedback from the author or reviewer of those
changes because there are some things that I don't understand:

  There doesn't seem to be any documentation on what a Wakelock is for, what
  the topic is used for, or the difference between hidden and active locks.

  The android-specific listener provides for concurrent locks of different
  topic/tag.

  Why is ModifyWakeLock implemented in the hardware abstraction layer when it
  doesn't interact with the hardware?