[gnome-software/wip/jrocha/fix-screenshots] Fix stale screenshots when they are cached
- From: Joaquim Manuel Pereira Rocha <jrocha src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gnome-software/wip/jrocha/fix-screenshots] Fix stale screenshots when they are cached
- Date: Wed, 9 Nov 2016 13:52:00 +0000 (UTC)
commit 5c4cc476ad0143996f02abc011e7b83c87edd5d0
Author: Joaquim Rocha <jrocha endlessm com>
Date: Tue Nov 8 17:00:04 2016 +0100
Fix stale screenshots when they are cached
This patch solves a couple of issues in the way we use the screenshots'
cache:
1) Once a screenshot was downloaded, it was never updated again until
there was a new version of GNOME Software that would initialize a
new cache. With these changes we always check the screenshots
remotely so we update the files as soon as there are newer ones on
the server. However, we use the images' file modification when
checking the remote files so we only effectively download those if
they are newer.
2) If screenshots were present in the system's cache, then they would
not be updated even on a new version of GNOME Software and they
would be used even if the user cache's version was newer.
3) It was downloading an image for every screenshot shown; this would
end up in unnecessary downloads when the there are not different
images for thumbnails/large screenshots. This patch saves a the
counterpart sizes when downloading a thumbnail/large image if their
kind is unknown (which means that when the image have an assigned
'thumbnail' or 'source' kind, they still get downloaded
individually).
src/gs-screenshot-image.c | 184 +++++++++++++++++++++++++++++++++++++++------
src/gs-utils.c | 27 +++++++
src/gs-utils.h | 3 +
3 files changed, 191 insertions(+), 23 deletions(-)
---
diff --git a/src/gs-screenshot-image.c b/src/gs-screenshot-image.c
index 01fbcf4..9f11fa5 100644
--- a/src/gs-screenshot-image.c
+++ b/src/gs-screenshot-image.c
@@ -35,6 +35,13 @@
#include "gs-common.h"
#include "gs-utils.h"
+typedef enum {
+ GS_SCREENSHOT_IMAGE_STATUS_UNKNOWN,
+ GS_SCREENSHOT_IMAGE_STATUS_IMAGE,
+ GS_SCREENSHOT_IMAGE_STATUS_BLURRED,
+ GS_SCREENSHOT_IMAGE_STATUS_ERROR
+} GsScreenshotImageStatus;
+
struct _GsScreenshotImage
{
GtkBin parent_instance;
@@ -53,10 +60,13 @@ struct _GsScreenshotImage
guint width;
guint height;
guint scale;
+ GsScreenshotImageStatus status;
};
G_DEFINE_TYPE (GsScreenshotImage, gs_screenshot_image, GTK_TYPE_BIN)
+static gchar *gs_screenshot_get_cachefn_for_url (const gchar *url);
+
AsScreenshot *
gs_screenshot_image_get_screenshot (GsScreenshotImage *ssimg)
{
@@ -76,6 +86,8 @@ gs_screenshot_image_set_error (GsScreenshotImage *ssimg, const gchar *message)
gtk_widget_hide (ssimg->label_error);
else
gtk_widget_show (ssimg->label_error);
+
+ ssimg->status = GS_SCREENSHOT_IMAGE_STATUS_ERROR;
}
static GdkPixbuf *
@@ -168,6 +180,7 @@ as_screenshot_show_image (GsScreenshotImage *ssimg)
ssimg->current_image = "image1";
}
+ ssimg->status = GS_SCREENSHOT_IMAGE_STATUS_IMAGE;
gtk_widget_show (GTK_WIDGET (ssimg));
}
@@ -196,6 +209,58 @@ gs_screenshot_image_show_blurred (GsScreenshotImage *ssimg,
gs_image_set_from_pixbuf_with_scale (GTK_IMAGE (ssimg->image2),
pb, (gint) ssimg->scale);
}
+
+ ssimg->status = GS_SCREENSHOT_IMAGE_STATUS_BLURRED;
+}
+
+static void
+gs_screenshot_image_save_counterpart (GsScreenshotImage *ssimg, AsImage *im)
+{
+ guint width = ssimg->width;
+ guint height = ssimg->height;
+ gboolean ret;
+ g_autoptr(GError) error = NULL;
+ const char *url = as_image_get_url (im);
+ g_autofree char *filename = NULL;
+ g_autofree char *size_dir = NULL;
+ g_autofree char *cache_kind = NULL;
+ g_autofree char *basename = gs_screenshot_get_cachefn_for_url (url);
+
+ /* if the image is a thumbnail we save the normal size and
+ * vice-versa */
+ if (width == AS_IMAGE_THUMBNAIL_WIDTH &&
+ height == AS_IMAGE_THUMBNAIL_HEIGHT) {
+ width = AS_IMAGE_NORMAL_WIDTH;
+ height = AS_IMAGE_NORMAL_HEIGHT;
+ } else {
+ width = AS_IMAGE_THUMBNAIL_WIDTH;
+ height = AS_IMAGE_THUMBNAIL_HEIGHT;
+ }
+
+ width *= ssimg->scale;
+ height *= ssimg->scale;
+
+ size_dir = g_strdup_printf ("%ux%u", width, height);
+ cache_kind = g_build_filename ("screenshots", size_dir, NULL);
+ filename = gs_utils_get_cache_filename (cache_kind,
+ basename,
+ GS_UTILS_CACHE_FLAG_WRITEABLE,
+ NULL);
+
+ if (!filename) {
+ g_warning ("Failed to get cache filename for counterpart "
+ "screenshot for URL '%s' in folder '%s'", url,
+ cache_kind);
+ return;
+ }
+
+ ret = as_image_save_filename (im, filename, width, height,
+ AS_IMAGE_SAVE_FLAG_PAD_16_9, &error);
+
+ if (!ret)
+ g_warning ("Failed to save counterpart screenshot for URL '%s' "
+ "in folder '%s': %s", url, cache_kind,
+ error->message);
}
static void
@@ -214,7 +279,25 @@ gs_screenshot_image_complete_cb (SoupSession *session,
if (msg->status_code == SOUP_STATUS_CANCELLED || ssimg->session == NULL)
return;
+ if (msg->status_code == SOUP_STATUS_NOT_MODIFIED) {
+ SoupURI *uri = soup_message_get_uri (msg);
+ g_autofree char *url = soup_uri_to_string (uri, FALSE);
+
+ g_debug ("Screenshot for '%s' has not been modified; not "
+ "downloading again", url);
+ return;
+ }
if (msg->status_code != SOUP_STATUS_OK) {
+ SoupURI *uri = soup_message_get_uri (msg);
+ g_autofree char *url = soup_uri_to_string (uri, FALSE);
+ g_warning ("Result of downloading screenshot from '%s': %s",
+ url, msg->reason_phrase);
+
+ /* if we are already showing a screenshot, do not show and
+ * error */
+ if (ssimg->status == GS_SCREENSHOT_IMAGE_STATUS_IMAGE)
+ return;
+
/* TRANSLATORS: this is when we try to download a screenshot and
* we get back 404 */
gs_screenshot_image_set_error (ssimg, _("Screenshot not found"));
@@ -232,6 +315,8 @@ gs_screenshot_image_complete_cb (SoupSession *session,
/* load the image */
pixbuf = gdk_pixbuf_new_from_stream (stream, NULL, NULL);
if (pixbuf == NULL) {
+ if (ssimg->status == GS_SCREENSHOT_IMAGE_STATUS_IMAGE)
+ return;
/* TRANSLATORS: possibly image file corrupt or not an image */
gs_screenshot_image_set_error (ssimg, _("Failed to load image"));
return;
@@ -250,10 +335,13 @@ gs_screenshot_image_complete_cb (SoupSession *session,
return;
}
} else {
+ AsImageKind kind;
+
/* save to file, using the same code as the AppStream builder
* so the preview looks the same */
im = as_image_new ();
as_image_set_pixbuf (im, pixbuf);
+
ret = as_image_save_filename (im, ssimg->filename,
ssimg->width * ssimg->scale,
ssimg->height * ssimg->scale,
@@ -262,6 +350,13 @@ gs_screenshot_image_complete_cb (SoupSession *session,
gs_screenshot_image_set_error (ssimg, error->message);
return;
}
+
+ /* if the kind is unknown then we save also the counterpart size
+ * image in order to save an extra download when the user
+ * requests that size later */
+ kind = as_image_get_kind (im);
+ if (kind == AS_IMAGE_KIND_UNKNOWN)
+ gs_screenshot_image_save_counterpart (ssimg, im);
}
/* got image, so show */
@@ -324,6 +419,10 @@ gs_screenshot_image_load_async (GsScreenshotImage *ssimg,
g_autofree gchar *cachefn_thumb = NULL;
g_autofree gchar *sizedir = NULL;
g_autoptr(SoupURI) base_uri = NULL;
+ char *loaded_filename = NULL;
+ g_autofree char *writable_filename = NULL;
+ gboolean showing_image = FALSE;
+ gboolean fetch_new_image = TRUE;
g_return_if_fail (GS_IS_SCREENSHOT_IMAGE (ssimg));
@@ -359,6 +458,7 @@ gs_screenshot_image_load_async (GsScreenshotImage *ssimg,
as_screenshot_show_image (ssimg);
return;
}
+ g_clear_pointer (&ssimg->filename, g_free);
}
basename = gs_screenshot_get_cachefn_for_url (url);
@@ -368,26 +468,49 @@ gs_screenshot_image_load_async (GsScreenshotImage *ssimg,
sizedir = g_strdup_printf ("%ux%u", ssimg->width * ssimg->scale, ssimg->height *
ssimg->scale);
}
cache_kind = g_build_filename ("screenshots", sizedir, NULL);
- ssimg->filename = gs_utils_get_cache_filename (cache_kind,
- basename,
- GS_UTILS_CACHE_FLAG_NONE,
- NULL);
+
+ /* request the writable cache filename before trying the system's one
+ * because the former should be the more recent */
+ writable_filename = gs_utils_get_cache_filename (cache_kind,
+ basename,
+ GS_UTILS_CACHE_FLAG_WRITEABLE,
+ NULL);
+
+ g_clear_pointer (&ssimg->filename, g_free);
+
+ if (writable_filename == NULL)
+ fetch_new_image = FALSE;
+
+ if (fetch_new_image &&
+ g_file_test (writable_filename, G_FILE_TEST_EXISTS)) {
+ ssimg->filename = g_steal_pointer (&writable_filename);
+ } else {
+ ssimg->filename = gs_utils_get_cache_filename (cache_kind,
+ basename,
+ GS_UTILS_CACHE_FLAG_NONE,
+ NULL);
+ }
+
if (ssimg->filename == NULL) {
- /* TRANSLATORS: this is when we try create the cache directory
- * but we were out of space or permission was denied */
- gs_screenshot_image_set_error (ssimg, _("Could not create cache"));
+ /* TRANSLATORS: this is when we try to get the image path in
+ * the system's cache but it doesn't exist, and it also failed
+ * to get the user's cache directory but we were out of space
+ * or permission was denied */
+ gs_screenshot_image_set_error (ssimg,
+ _("Could not create cache"));
return;
}
- /* does local file already exist */
+ /* show the screenshot if its cached version exists, otherwise try to
+ * load a blurred smaller version while we fetch the new image */
if (g_file_test (ssimg->filename, G_FILE_TEST_EXISTS)) {
+ showing_image = TRUE;
as_screenshot_show_image (ssimg);
- return;
- }
- /* can we load a blurred smaller version of this straight away */
- if (ssimg->width > AS_IMAGE_THUMBNAIL_WIDTH &&
- ssimg->height > AS_IMAGE_THUMBNAIL_HEIGHT) {
+ if (!fetch_new_image)
+ return;
+ } else if (ssimg->width > AS_IMAGE_THUMBNAIL_WIDTH &&
+ ssimg->height > AS_IMAGE_THUMBNAIL_HEIGHT) {
const gchar *url_thumb;
g_autofree gchar *basename_thumb = NULL;
g_autofree gchar *cache_kind_thumb = NULL;
@@ -407,18 +530,16 @@ gs_screenshot_image_load_async (GsScreenshotImage *ssimg,
gs_screenshot_image_show_blurred (ssimg, cachefn_thumb);
}
- /* re-request the cache filename, which might be different as it needs
- * to be writable this time */
- g_free (ssimg->filename);
- ssimg->filename = gs_utils_get_cache_filename (cache_kind,
- basename,
- GS_UTILS_CACHE_FLAG_WRITEABLE,
- NULL);
-
- /* download file */
- g_debug ("downloading %s to %s", url, ssimg->filename);
+ /* download file (if there is a new version available) */
base_uri = soup_uri_new (url);
if (base_uri == NULL || !SOUP_URI_VALID_FOR_HTTP (base_uri)) {
+ if (showing_image) {
+ g_warning ("Could not try to download a new screenshot "
+ "because the URL is invalid '%s'. Showing "
+ "cached image '%s'.", url, ssimg->filename);
+ return;
+ }
+
/* TRANSLATORS: this is when we try to download a screenshot
* that was not a valid URL */
gs_screenshot_image_set_error (ssimg, _("Screenshot not valid"));
@@ -435,12 +556,28 @@ gs_screenshot_image_load_async (GsScreenshotImage *ssimg,
ssimg->message = soup_message_new_from_uri (SOUP_METHOD_GET, base_uri);
if (ssimg->message == NULL) {
+ if (showing_image) {
+ g_warning ("Could not setup a new soup_message for the "
+ "URL '%s'. Showing cached image '%s'.", url,
+ ssimg->filename);
+ return;
+ }
+
/* TRANSLATORS: this is when networking is not available */
gs_screenshot_image_set_error (ssimg, _("Screenshot not available"));
return;
}
+ if (showing_image)
+ gs_utils_set_modified_request (ssimg->message, ssimg->filename);
+
+ if (writable_filename != NULL) {
+ g_free (ssimg->filename);
+ ssimg->filename = g_steal_pointer (&writable_filename);
+ }
+
/* send async */
+ g_debug ("Downloading '%s' if its newer than our cached file.", url);
soup_session_queue_message (ssimg->session,
g_object_ref (ssimg->message) /* transfer full */,
gs_screenshot_image_complete_cb,
@@ -481,6 +618,7 @@ gs_screenshot_image_init (GsScreenshotImage *ssimg)
atk_object_set_role (accessible, ATK_ROLE_IMAGE);
atk_object_set_name (accessible, _("Screenshot"));
}
+ ssimg->status = GS_SCREENSHOT_IMAGE_STATUS_UNKNOWN;
}
static gboolean
diff --git a/src/gs-utils.c b/src/gs-utils.c
index ac9bbd2..b989a8e 100644
--- a/src/gs-utils.c
+++ b/src/gs-utils.c
@@ -796,4 +796,31 @@ gs_utils_error_convert_appstream (GError **perror)
return TRUE;
}
+void
+gs_utils_set_modified_request (SoupMessage *msg,
+ const char *file_path)
+{
+ GTimeVal time_val;
+ g_autoptr(GDateTime) date_time = NULL;
+ g_autoptr(GFile) file = NULL;
+ g_autoptr(GFileInfo) info = NULL;
+ g_autofree char *mod_date = NULL;
+
+ file = g_file_new_for_path (file_path);
+ info = g_file_query_info (file,
+ G_FILE_ATTRIBUTE_TIME_MODIFIED,
+ G_FILE_QUERY_INFO_NONE,
+ NULL,
+ NULL);
+ if (info == NULL)
+ return;
+ g_file_info_get_modification_time (info, &time_val);
+ date_time = g_date_time_new_from_timeval_local (&time_val);
+ mod_date = g_date_time_format (date_time, "%a, %d %b %Y %H:%M:%S %Z");
+
+ soup_message_headers_append (msg->request_headers, "If-Modified-Since",
+ mod_date);
+ return;
+}
+
/* vim: set noexpandtab: */
diff --git a/src/gs-utils.h b/src/gs-utils.h
index 3a55cd3..d6bc618 100644
--- a/src/gs-utils.h
+++ b/src/gs-utils.h
@@ -24,6 +24,7 @@
#include <gio/gdesktopappinfo.h>
#include <gtk/gtk.h>
+#include <libsoup/soup.h>
#include "gs-app.h"
@@ -78,6 +79,8 @@ gboolean gs_utils_error_convert_gdbus (GError **perror);
gboolean gs_utils_error_convert_gdk_pixbuf(GError **perror);
gboolean gs_utils_error_convert_json_glib (GError **perror);
gboolean gs_utils_error_convert_appstream (GError **perror);
+void gs_utils_set_modified_request (SoupMessage *message,
+ const char *file_path);
G_END_DECLS
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
