[epiphany/gnome-3-20] snapshot-service: Take new snapshots after a restart

[Michael Catanzaro <mcatanzaro src gnome org>]

commit 5300b066ce38fae58fb99cd3a5e21ba0c060b943
Author: Michael Catanzaro <mcatanzaro igalia com>
Date:   Tue Mar 29 20:59:31 2016 -0500

    snapshot-service: Take new snapshots after a restart
    
    The previous commit was correct, but the commit message was not. It
    stated that removing the erroneous check would cause snapshots to be
    taken if the stored snapshot is too old. In fact, removing the check did
    not affect behavior at all -- the check was useless -- and so that did
    nothing to fix our stale snapshot issue.
    
    If the snapshot path is not in memory cache, this is an indication that
    the snapshot is relatively stale (older than the current browsing
    session) and should be taken again. But we cannot use the existing
    snapshot path memory cache as-is, because it is populated not when
    snapshots are taken, but when snapshots are looked up. Augment the cache
    to remember which paths were snapshotted since Epiphany was last
    restarted, and check it to determine whether we're due for a snapshot of
    the current page. Also, remove use of the cache from EphyWebView, since
    that would prevent snapshots from ever being taken.
    
    This really fixes the issue.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=763184

 embed/ephy-web-view.c       |   19 +++----
 lib/ephy-snapshot-service.c |  113 ++++++++++++++++++++++++++++--------------
 2 files changed, 83 insertions(+), 49 deletions(-)
---
diff --git a/embed/ephy-web-view.c b/embed/ephy-web-view.c
index 6938865..73166bc 100644
--- a/embed/ephy-web-view.c
+++ b/embed/ephy-web-view.c
@@ -614,8 +614,7 @@ web_view_check_snapshot (WebKitWebView *web_view)
 
   view->snapshot_timeout_id = 0;
 
-  if (ephy_snapshot_service_lookup_snapshot_path (service, url) ||
-      view->error_page != EPHY_WEB_VIEW_ERROR_PAGE_NONE)
+  if (view->error_page != EPHY_WEB_VIEW_ERROR_PAGE_NONE)
     return FALSE;
 
   data = g_new (GetSnapshotPathAsyncData, 1);
@@ -1526,15 +1525,13 @@ load_changed_cb (WebKitWebView  *web_view,
 
       if (!ephy_web_view_is_history_frozen (view) &&
           ephy_embed_shell_get_mode (ephy_embed_shell_get_default ()) != EPHY_EMBED_SHELL_MODE_INCOGNITO) {
-        if (!ephy_snapshot_service_lookup_snapshot_path (ephy_snapshot_service_get_default (), 
webkit_web_view_get_uri (web_view))) {
-          /* FIXME: The 1s delay is a workaround to allow time to render the page and get a favicon.
-           * https://bugzilla.gnome.org/show_bug.cgi?id=761065
-           */
-          if (view->snapshot_timeout_id == 0) {
-            view->snapshot_timeout_id = g_timeout_add_seconds_full (G_PRIORITY_LOW, 1,
-                                                                    (GSourceFunc)web_view_check_snapshot,
-                                                                    web_view, NULL);
-          }
+        /* FIXME: The 1s delay is a workaround to allow time to render the page and get a favicon.
+         * https://bugzilla.gnome.org/show_bug.cgi?id=761065
+         */
+        if (view->snapshot_timeout_id == 0) {
+          view->snapshot_timeout_id = g_timeout_add_seconds_full (G_PRIORITY_LOW, 1,
+                                                                  (GSourceFunc)web_view_check_snapshot,
+                                                                  web_view, NULL);
         }
       }
 
diff --git a/lib/ephy-snapshot-service.c b/lib/ephy-snapshot-service.c
index 61ef261..c1ba49f 100644
--- a/lib/ephy-snapshot-service.c
+++ b/lib/ephy-snapshot-service.c
@@ -29,19 +29,37 @@
 
 struct _EphySnapshotService {
   GObject parent_instance;
+
+  /* Disk cache */
   GnomeDesktopThumbnailFactory *factory;
+
+  /* Memory cache */
   GHashTable *cache;
 };
 
 G_DEFINE_TYPE (EphySnapshotService, ephy_snapshot_service, G_TYPE_OBJECT)
 
-/* GObject boilerplate methods. */
+typedef enum {
+  SNAPSHOT_STALE,
+  SNAPSHOT_FRESH
+} EphySnapshotFreshness;
+
+typedef struct {
+  char *path;
+  EphySnapshotFreshness freshness;
+} SnapshotPathCachedData;
 
 static void
-ephy_snapshot_service_class_init (EphySnapshotServiceClass *klass)
+snapshot_path_cached_data_free (SnapshotPathCachedData *data)
 {
+  g_free (data->path);
+  g_free (data);
 }
 
+static void
+ephy_snapshot_service_class_init (EphySnapshotServiceClass *klass)
+{
+}
 
 static void
 ephy_snapshot_service_init (EphySnapshotService *self)
@@ -49,7 +67,7 @@ ephy_snapshot_service_init (EphySnapshotService *self)
   self->factory = gnome_desktop_thumbnail_factory_new (GNOME_DESKTOP_THUMBNAIL_SIZE_LARGE);
   self->cache = g_hash_table_new_full (g_str_hash, g_str_equal,
                                        (GDestroyNotify)g_free,
-                                       (GDestroyNotify)g_free);
+                                       (GDestroyNotify)snapshot_path_cached_data_free);
 }
 
 static GdkPixbuf *
@@ -343,6 +361,30 @@ ephy_snapshot_service_get_default (void)
   return service;
 }
 
+const char *
+ephy_snapshot_service_lookup_snapshot_path (EphySnapshotService *service,
+                                            const char          *url)
+{
+  SnapshotPathCachedData *data;
+
+  g_return_val_if_fail (EPHY_IS_SNAPSHOT_SERVICE (service), NULL);
+
+  data = g_hash_table_lookup (service->cache, url);
+
+  return data == NULL ? NULL : data->path;
+}
+
+static EphySnapshotFreshness
+ephy_snapshot_service_lookup_snapshot_freshness (EphySnapshotService *service,
+                                                 const char          *url)
+{
+  SnapshotPathCachedData *data;
+
+  data = g_hash_table_lookup (service->cache, url);
+
+  return data == NULL ? SNAPSHOT_STALE : data->freshness;
+}
+
 typedef struct {
   char *url;
   time_t mtime;
@@ -375,14 +417,14 @@ snapshot_for_url_async_data_free (SnapshotForURLAsyncData *data)
 typedef struct {
   GHashTable *cache;
   char *url;
-  char *path;
+  SnapshotPathCachedData *data;
 } CacheData;
 
 static gboolean
 idle_cache_snapshot_path (gpointer user_data)
 {
   CacheData *data = (CacheData *)user_data;
-  g_hash_table_insert (data->cache, data->url, data->path);
+  g_hash_table_insert (data->cache, data->url, data->data);
   g_hash_table_unref (data->cache);
   g_free (data);
 
@@ -390,6 +432,22 @@ idle_cache_snapshot_path (gpointer user_data)
 }
 
 static void
+cache_snapshot_data_in_idle (EphySnapshotService  *service,
+                             const char           *url,
+                             const char           *path,
+                             EphySnapshotFreshness freshness)
+{
+  CacheData *data;
+  data = g_new (CacheData, 1);
+  data->cache = g_hash_table_ref (service->cache);
+  data->url = g_strdup (url);
+  data->data = g_new (SnapshotPathCachedData, 1);
+  data->data->path = g_strdup (path);
+  data->data->freshness = freshness;
+  g_idle_add (idle_cache_snapshot_path, data);
+}
+
+static void
 get_snapshot_for_url_thread (GTask                   *task,
                              EphySnapshotService     *service,
                              SnapshotForURLAsyncData *data,
@@ -397,7 +455,6 @@ get_snapshot_for_url_thread (GTask                   *task,
 {
   GdkPixbuf *snapshot;
   GError *error = NULL;
-  CacheData *cache_data;
 
   data->path = gnome_desktop_thumbnail_factory_lookup (service->factory, data->url, data->mtime);
   if (data->path == NULL) {
@@ -408,11 +465,7 @@ get_snapshot_for_url_thread (GTask                   *task,
     return;
   }
 
-  cache_data = g_new (CacheData, 1);
-  cache_data->cache = g_hash_table_ref (service->cache);
-  cache_data->url = g_strdup (data->url);
-  cache_data->path = g_strdup (data->path);
-  g_idle_add (idle_cache_snapshot_path, cache_data);
+  cache_snapshot_data_in_idle (service, data->url, data->path, SNAPSHOT_STALE);
 
   snapshot = gdk_pixbuf_new_from_file (data->path, &error);
   if (snapshot == NULL) {
@@ -550,15 +603,18 @@ ephy_snapshot_service_get_snapshot_async (EphySnapshotService *service,
                         snapshot_async_data_new_for_snapshot (web_view, mtime),
                         (GDestroyNotify)snapshot_async_data_free);
 
-  /* Try to get the snapshot from the cache first if we have a URL */
+  /* Try to get the snapshot from the cache first if we have a URL, but only if
+   * the snapshot path is in memory cache; this is an indication that the
+   * snapshot is fresh. */
   uri = webkit_web_view_get_uri (web_view);
-  if (uri)
+  if (uri && ephy_snapshot_service_lookup_snapshot_freshness (service, uri) == SNAPSHOT_FRESH) {
     ephy_snapshot_service_get_snapshot_for_url_async (service,
                                                       uri, mtime, cancellable,
                                                       (GAsyncReadyCallback)got_snapshot_for_url,
                                                       task);
-  else
+  } else {
     g_idle_add ((GSourceFunc)ephy_snapshot_service_take_from_webview, task);
+  }
 }
 
 /**
@@ -636,7 +692,6 @@ save_snapshot_thread (GTask                 *task,
                       GCancellable          *cancellable)
 {
   char *path;
-  CacheData *cache_data;
 
   gnome_desktop_thumbnail_factory_save_thumbnail (service->factory,
                                                   data->snapshot,
@@ -645,11 +700,7 @@ save_snapshot_thread (GTask                 *task,
 
   path = gnome_desktop_thumbnail_path_for_uri (data->url, GNOME_DESKTOP_THUMBNAIL_SIZE_LARGE);
 
-  cache_data = g_new (CacheData, 1);
-  cache_data->cache = g_hash_table_ref (service->cache);
-  cache_data->url = g_strdup (data->url);
-  cache_data->path = g_strdup (path);
-  g_idle_add (idle_cache_snapshot_path, cache_data);
+  cache_snapshot_data_in_idle (service, data->url, path, SNAPSHOT_FRESH);
 
   g_task_return_pointer (task, path, g_free);
 }
@@ -688,15 +739,6 @@ ephy_snapshot_service_save_snapshot_finish (EphySnapshotService *service,
   return g_task_propagate_pointer (G_TASK (result), error);
 }
 
-const char *
-ephy_snapshot_service_lookup_snapshot_path (EphySnapshotService *service,
-                                            const char          *url)
-{
-  g_return_val_if_fail (EPHY_IS_SNAPSHOT_SERVICE (service), NULL);
-
-  return g_hash_table_lookup (service->cache, url);
-}
-
 static void
 get_snapshot_path_for_url_thread (GTask                   *task,
                                   EphySnapshotService     *service,
@@ -704,7 +746,6 @@ get_snapshot_path_for_url_thread (GTask                   *task,
                                   GCancellable            *cancellable)
 {
   char *path;
-  CacheData *cache_data;
 
   path = gnome_desktop_thumbnail_factory_lookup (service->factory, data->url, data->mtime);
   if (!path) {
@@ -715,11 +756,7 @@ get_snapshot_path_for_url_thread (GTask                   *task,
     return;
   }
 
-  cache_data = g_new (CacheData, 1);
-  cache_data->cache = g_hash_table_ref (service->cache);
-  cache_data->url = g_strdup (data->url);
-  cache_data->path = g_strdup (path);
-  g_idle_add (idle_cache_snapshot_path, cache_data);
+  cache_snapshot_data_in_idle (service, data->url, path, SNAPSHOT_STALE);
 
   g_task_return_pointer (task, path, g_free);
 }
@@ -740,7 +777,7 @@ ephy_snapshot_service_get_snapshot_path_for_url_async (EphySnapshotService *serv
 
   task = g_task_new (service, cancellable, callback, user_data);
 
-  path = g_hash_table_lookup (service->cache, url);
+  path = ephy_snapshot_service_lookup_snapshot_path (service, url);
   if (path) {
     g_task_return_pointer (task, g_strdup (path), g_free);
     g_object_unref (task);
@@ -798,8 +835,8 @@ ephy_snapshot_service_get_snapshot_path_async (EphySnapshotService *service,
   task = g_task_new (service, cancellable, callback, user_data);
 
   uri = webkit_web_view_get_uri (web_view);
-  if (uri) {
-    const char *path = g_hash_table_lookup (service->cache, uri);
+  if (uri && ephy_snapshot_service_lookup_snapshot_freshness (service, uri) == SNAPSHOT_FRESH) {
+    const char *path = ephy_snapshot_service_lookup_snapshot_path (service, uri);
 
     if (path) {
       g_task_return_pointer (task, g_strdup (path), g_free);