<html>
    <head>
      <base href="https://bugzilla.gnome.org/" />
    </head>
    <body><span class="vcard"><a href="page.cgi?id=describeuser.html&login=bugzilla%40hadess.net" title="Bastien Nocera <bugzilla@hadess.net>"> <span class="fn">Bastien Nocera</span></a>
</span> changed
              <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - wayland: fix fontconfig monitoring"
   href="https://bugzilla.gnome.org/show_bug.cgi?id=786694">bug 786694</a>
          <br>
             <table border="1" cellspacing="0" cellpadding="8">
          <tr>
            <th>What</th>
            <th>Removed</th>
            <th>Added</th>
          </tr>

         <tr>
           <td style="text-align:right;">Attachment #368734 status</td>
           <td>none
           </td>
           <td>needs-work
           </td>
         </tr></table>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - wayland: fix fontconfig monitoring"
   href="https://bugzilla.gnome.org/show_bug.cgi?id=786694#c12">Comment # 12</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - wayland: fix fontconfig monitoring"
   href="https://bugzilla.gnome.org/show_bug.cgi?id=786694">bug 786694</a>
              from <span class="vcard"><a href="page.cgi?id=describeuser.html&login=bugzilla%40hadess.net" title="Bastien Nocera <bugzilla@hadess.net>"> <span class="fn">Bastien Nocera</span></a>
</span></b>
        <pre>Review of <span class=""><a href="attachment.cgi?id=368734" name="attach_368734" title="Introduce org.gnome.SettingsDaemon.FontConfig (timestamp as int64)">attachment 368734</a> <a href="attachment.cgi?id=368734&action=edit" title="Introduce org.gnome.SettingsDaemon.FontConfig (timestamp as int64)">[details]</a></span> <a href='review?bug=786694&attachment=368734'>[review]</a>:

1) Get rid of the array
2) Every time a timestamp change has happened, queue sending the new value for
half-a-second (or a second?), reschedule if a new timestamp arrives in that
time
3) Don't use g_get_real_time()

::: plugins/xsettings/gsd-xsettings-manager.c
@@ +80,3 @@
+static const gchar introspection_xml[] =
+"<node name='/org/gnome/SettingsDaemon/FontConfig'>"
+"  <interface name='org.gnome.SettingsDaemon.FontConfig'>"

Is this really the name of the interface we want to use? What about GTK+ apps
under other Wayland desktops?

@@ +284,3 @@
         GSettings         *plugin_settings;
         FcMonitor         *fontconfig_monitor;
+        GArray            *fontconfig_timestamps;

I've stared at the code for a long while, and can't figure out why you need to
send a "changed" signal for each and every fontconfig timestamp change. Why do
that?

You could just keep the last one, and send it out when it changes, or
aggregate/coalesce the changes into one event if the changes are too close
together. You need to remember that this would wake up every single GTK+
program, so less notifications is better.

@@ +1189,3 @@
         GList       *list, *l;
         const char  *session;
+        gint64       timestamp = g_get_real_time ();

You shouldn't user g_get_real_time(), this will break if the time on the
machine is changed. Any reason why g_get_monotonic_time() can't be used?

@@ +1453,3 @@
+        manager = GNOME_XSETTINGS_MANAGER (user_data);
+
+        if (manager->priv->dbus_connection == NULL)

This isn't necessary, you get a connection in the arguments of the function
above, and you don't use this variable anyway.</pre>
        </div>
      </p>
      <hr>
      <span>You are receiving this mail because:</span>
      
      <ul>
          <li>You are on the CC list for the bug.</li>
      </ul>
    </body>
</html>