[epiphany/wip/sync] sync: Fix issues pointed in the code review



commit 4949a3690067b0336b897c34c5c5bca3fcd961f0
Author: Gabriel Ivascu <ivascu gabriel59 gmail com>
Date:   Mon Jul 31 13:06:00 2017 +0300

    sync: Fix issues pointed in the code review

 lib/ephy-sync-utils.c                        |    7 +++++--
 lib/sync/ephy-history-manager.c              |    3 ++-
 lib/sync/ephy-history-record.c               |    4 +++-
 lib/sync/ephy-open-tabs-manager.c            |    5 ++++-
 lib/sync/ephy-sync-service.c                 |   17 +++++++++--------
 po/POTFILES.in                               |    1 +
 src/bookmarks/ephy-add-bookmark-popover.c    |    2 +-
 src/bookmarks/ephy-bookmark.h                |    7 -------
 src/passwords-dialog.c                       |    3 ++-
 src/prefs-dialog.c                           |    6 +++---
 src/profile-migrator/ephy-profile-migrator.c |    2 +-
 src/synced-tabs-dialog.c                     |    3 ++-
 12 files changed, 33 insertions(+), 27 deletions(-)
---
diff --git a/lib/ephy-sync-utils.c b/lib/ephy-sync-utils.c
index 894d4b8..f212971 100644
--- a/lib/ephy-sync-utils.c
+++ b/lib/ephy-sync-utils.c
@@ -84,7 +84,10 @@ base64_to_base64_urlsafe (char *text)
 {
   g_assert (text);
 
-  /* Replace '+' with '-' and '/' with '_' */
+  /* / and + are inappropriate for URLs and file systems paths, so they have to
+   * be omitted to make the base64 string safe. / is replaced with _ and + is
+   * replaced with -.
+   */
   g_strcanon (text, "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789=/", '-');
   g_strcanon (text, "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789=-", '_');
 }
@@ -265,7 +268,7 @@ ephy_sync_utils_get_device_name (void)
   name = g_settings_get_string (EPHY_SETTINGS_SYNC, EPHY_PREFS_SYNC_DEVICE_NAME);
   if (!g_strcmp0 (name, "")) {
     g_free (name);
-    name = g_strdup_printf ("%s's Epiphany on %s",
+    name = g_strdup_printf ("%s's GNOME Web on %s",
                             g_get_user_name (), g_get_host_name ());
   }
 
diff --git a/lib/sync/ephy-history-manager.c b/lib/sync/ephy-history-manager.c
index 1fb439d..b3a8da7 100644
--- a/lib/sync/ephy-history-manager.c
+++ b/lib/sync/ephy-history-manager.c
@@ -118,7 +118,8 @@ ephy_history_manager_set_property (GObject      *object,
 
   switch (prop_id) {
     case PROP_HISTORY_SERVICE:
-      g_clear_object (&self->service);
+      if (self->service)
+        g_object_unref (self->service);
       self->service = g_object_ref (g_value_get_object (value));
       break;
     default:
diff --git a/lib/sync/ephy-history-record.c b/lib/sync/ephy-history-record.c
index 93287f6..b0027d7 100644
--- a/lib/sync/ephy-history-record.c
+++ b/lib/sync/ephy-history-record.c
@@ -156,7 +156,9 @@ ephy_history_record_finalize (GObject *object)
   g_free (self->id);
   g_free (self->title);
   g_free (self->uri);
-  g_sequence_free (self->visits);
+
+  if (self->visits)
+    g_sequence_free (self->visits);
 
   G_OBJECT_CLASS (ephy_history_record_parent_class)->finalize (object);
 }
diff --git a/lib/sync/ephy-open-tabs-manager.c b/lib/sync/ephy-open-tabs-manager.c
index 882617d..cc03f52 100644
--- a/lib/sync/ephy-open-tabs-manager.c
+++ b/lib/sync/ephy-open-tabs-manager.c
@@ -27,6 +27,9 @@
 #include "ephy-sync-utils.h"
 #include "ephy-synchronizable-manager.h"
 
+#define BLANK_PAGE_TITLE N_("Blank page")
+#define OVERVIEW_PAGE_TITLE N_("Most Visited")
+
 struct _EphyOpenTabsManager {
   GObject parent_instance;
 
@@ -99,7 +102,7 @@ ephy_open_tabs_manager_get_local_tabs (EphyOpenTabsManager *self)
     for (GList *t = tabs; t && t->data; t = t->next) {
       title = ephy_embed_get_title (t->data);
 
-      if (!g_strcmp0 (title, "Blank page") || !g_strcmp0 (title, "Most Visited"))
+      if (!g_strcmp0 (title, _(BLANK_PAGE_TITLE)) || !g_strcmp0 (title, _(OVERVIEW_PAGE_TITLE)))
         continue;
 
       url = ephy_web_view_get_display_address (ephy_embed_get_web_view (t->data));
diff --git a/lib/sync/ephy-sync-service.c b/lib/sync/ephy-sync-service.c
index d9b88c5..0115132 100644
--- a/lib/sync/ephy-sync-service.c
+++ b/lib/sync/ephy-sync-service.c
@@ -32,7 +32,7 @@
 #include <json-glib/json-glib.h>
 #include <string.h>
 
-#define STORAGE_VERSION 5
+#define EPHY_STORAGE_VERSION 5
 
 struct _EphySyncService {
   GObject      parent_instance;
@@ -641,7 +641,8 @@ ephy_sync_service_forget_secrets (EphySyncService *self)
   g_assert (self->secrets);
 
   user = ephy_sync_utils_get_sync_user ();
-  g_assert (user);
+  g_return_if_fail (user);
+
   attributes = secret_attributes_build (EPHY_SYNC_SECRET_SCHEMA,
                                         ACCOUNT_KEY, user,
                                         NULL);
@@ -1900,7 +1901,7 @@ ephy_sync_service_upload_meta_global (EphySyncService *self)
   json_object_set_object_member (engines, "passwords", make_engine_object (1));
   json_object_set_object_member (engines, "forms", make_engine_object (1));
   json_object_set_object_member (payload, "engines", engines);
-  json_object_set_int_member (payload, "storageVersion", STORAGE_VERSION);
+  json_object_set_int_member (payload, "storageVersion", EPHY_STORAGE_VERSION);
   sync_id = ephy_sync_utils_get_random_sync_id ();
   json_object_set_string_member (payload, "syncID", sync_id);
   json_node_set_object (node, payload);
@@ -1978,11 +1979,11 @@ verify_storage_version_cb (SoupSession *session,
     goto out_error;
   }
   storage_version = json_object_get_int_member (json, "storageVersion");
-  if (storage_version != STORAGE_VERSION) {
+  if (storage_version != EPHY_STORAGE_VERSION) {
     /* Translators: the %d is the storage version, the \n is a newline character. */
-    message = g_strdup_printf (_("Your Firefox Account uses storage version %d "
-                                 "which Epiphany does not support.\n"
-                                 "Create a new account to use the latest storage version."),
+    message = g_strdup_printf (_("Your Firefox Account uses storage version %d. "
+                                 "Web only supports version %d."),
+                               EPHY_STORAGE_VERSION,
                                storage_version);
     goto out_error;
   }
@@ -2273,7 +2274,7 @@ ephy_sync_service_register_device (EphySyncService *self,
   g_return_if_fail (EPHY_IS_SYNC_SERVICE (self));
 
   /* Make protocol. */
-  protocol = g_strdup_printf ("1.%d", STORAGE_VERSION);
+  protocol = g_strdup_printf ("1.%d", EPHY_STORAGE_VERSION);
   array = json_array_new ();
   json_array_add_string_element (array, protocol);
 
diff --git a/po/POTFILES.in b/po/POTFILES.in
index f9308f0..609c263 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -20,6 +20,7 @@ lib/ephy-time-helpers.c
 lib/ephy-zoom.h
 lib/history/ephy-history-service-hosts-table.c
 lib/sync/ephy-sync-service.c
+lib/sync/ephy-open-tabs-manager.c
 lib/widgets/ephy-certificate-dialog.c
 lib/widgets/ephy-downloads-popover.c
 lib/widgets/ephy-download-widget.c
diff --git a/src/bookmarks/ephy-add-bookmark-popover.c b/src/bookmarks/ephy-add-bookmark-popover.c
index 2bef611..bfe041b 100644
--- a/src/bookmarks/ephy-add-bookmark-popover.c
+++ b/src/bookmarks/ephy-add-bookmark-popover.c
@@ -118,13 +118,13 @@ ephy_add_bookmark_popover_closed_cb (GtkPopover *popover,
 
   g_assert (EPHY_IS_ADD_BOOKMARK_POPOVER (popover));
 
+  self = EPHY_ADD_BOOKMARK_POPOVER (popover);
   manager = ephy_shell_get_bookmarks_manager (ephy_shell_get_default ());
 
   ephy_bookmarks_manager_save_to_file_async (manager, NULL,
                                              ephy_bookmarks_manager_save_to_file_warn_on_error_cb,
                                              NULL);
 
-  self = EPHY_ADD_BOOKMARK_POPOVER (popover);
   g_clear_pointer (&self->address, g_free);
   g_clear_pointer (&self->grid, gtk_widget_destroy);
 }
diff --git a/src/bookmarks/ephy-bookmark.h b/src/bookmarks/ephy-bookmark.h
index cb96a4d..f8504db 100644
--- a/src/bookmarks/ephy-bookmark.h
+++ b/src/bookmarks/ephy-bookmark.h
@@ -33,27 +33,21 @@ EphyBookmark        *ephy_bookmark_new                    (const char *url,
                                                            const char *title,
                                                            GSequence  *tags,
                                                            const char *id);
-
 void                 ephy_bookmark_set_time_added         (EphyBookmark *self,
                                                            gint64        time_added);
 gint64               ephy_bookmark_get_time_added         (EphyBookmark *self);
-
 void                 ephy_bookmark_set_url                (EphyBookmark *self,
                                                            const char   *url);
 const char          *ephy_bookmark_get_url                (EphyBookmark *self);
-
 void                 ephy_bookmark_set_title              (EphyBookmark *self,
                                                            const char   *title);
 const char          *ephy_bookmark_get_title              (EphyBookmark *self);
-
 void                 ephy_bookmark_set_id                 (EphyBookmark *self,
                                                            const char   *id);
 const char          *ephy_bookmark_get_id                 (EphyBookmark *self);
-
 void                 ephy_bookmark_set_is_uploaded        (EphyBookmark *self,
                                                            gboolean      uploaded);
 gboolean             ephy_bookmark_is_uploaded            (EphyBookmark *self);
-
 void                 ephy_bookmark_add_tag                (EphyBookmark *self,
                                                            const char   *tag);
 void                 ephy_bookmark_remove_tag             (EphyBookmark *self,
@@ -61,7 +55,6 @@ void                 ephy_bookmark_remove_tag             (EphyBookmark *self,
 gboolean             ephy_bookmark_has_tag                (EphyBookmark *self,
                                                            const char   *tag);
 GSequence           *ephy_bookmark_get_tags               (EphyBookmark *self);
-
 int                  ephy_bookmark_bookmarks_compare_func (EphyBookmark *bookmark1,
                                                            EphyBookmark *bookmark2);
 int                  ephy_bookmark_tags_compare           (const char *tag1,
diff --git a/src/passwords-dialog.c b/src/passwords-dialog.c
index 7bcfaae..c5b8fbd 100644
--- a/src/passwords-dialog.c
+++ b/src/passwords-dialog.c
@@ -80,7 +80,8 @@ ephy_passwords_dialog_set_property (GObject      *object,
 
   switch (prop_id) {
     case PROP_PASSWORD_MANAGER:
-      g_clear_object (&dialog->manager);
+      if (dialog->manager)
+        g_object_unref (dialog->manager);
       dialog->manager = g_object_ref (g_value_get_object (value));
       break;
     default:
diff --git a/src/prefs-dialog.c b/src/prefs-dialog.c
index 4b10511..3e4dea6 100644
--- a/src/prefs-dialog.c
+++ b/src/prefs-dialog.c
@@ -1962,10 +1962,10 @@ prefs_dialog_init (PrefsDialog *dialog)
   setup_stored_data_page (dialog);
   setup_language_page (dialog);
 
-  if (mode == EPHY_EMBED_SHELL_MODE_APPLICATION || mode == EPHY_EMBED_SHELL_MODE_INCOGNITO)
-    gtk_notebook_remove_page (GTK_NOTEBOOK (dialog->notebook), -1);
-  else
+  if (mode == EPHY_EMBED_SHELL_MODE_BROWSER)
     setup_sync_page (dialog);
+  else
+    gtk_notebook_remove_page (GTK_NOTEBOOK (dialog->notebook), -1);
 
   ephy_gui_ensure_window_group (GTK_WINDOW (dialog));
   g_signal_connect (dialog, "response",
diff --git a/src/profile-migrator/ephy-profile-migrator.c b/src/profile-migrator/ephy-profile-migrator.c
index adb8d26..5af795b 100644
--- a/src/profile-migrator/ephy-profile-migrator.c
+++ b/src/profile-migrator/ephy-profile-migrator.c
@@ -987,7 +987,7 @@ migrate_passwords_to_firefox_sync_passwords (void)
   GError *error = NULL;
   int default_profile_migration_version;
 
-  /* Similar to the insecure passowrds migration, we want to migrate passwords
+  /* Similar to the insecure passwords migration, we want to migrate passwords
    * to Firefox Sync passwords only once since saved passwords are stored
    * globally and not per profile. This won't affect password lookup for web
    * apps because this migration only adds a couple of new fields to the
diff --git a/src/synced-tabs-dialog.c b/src/synced-tabs-dialog.c
index d6cd8eb..3176553 100644
--- a/src/synced-tabs-dialog.c
+++ b/src/synced-tabs-dialog.c
@@ -235,7 +235,8 @@ synced_tabs_dialog_set_property (GObject      *object,
 
   switch (prop_id) {
     case PROP_OPEN_TABS_MANAGER:
-      g_clear_object (&dialog->manager);
+      if (dialog->manager)
+        g_object_unref (dialog->manager);
       dialog->manager = g_object_ref (g_value_get_object (value));
       break;
     default:


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]