GNOME.org
 

[epiphany] bookmarks: Split list model functionality into separate class

commit feacce23897127e0eb13bf93b75e753d8e18767b
Author: Michael Catanzaro <mcatanzaro gnome org>
Date:   Tue Nov 1 18:00:36 2016 -0500

    bookmarks: Split list model functionality into separate class
    
    This is much less efficient, but it also makes the code much simpler and
    more robust. Using EphyBookmarksManager as the list model was a good
    choice at the time, when we were not thinking about smart bookmarks, but
    now that we need to handle both it doesn't work so well. My initial
    implementation was complex and buggy. Simplify this.

 src/Makefile.am                           |    2 +
 src/bookmarks/ephy-bookmarks-list-model.c |  193 +++++++++++++++++++++++++++++
 src/bookmarks/ephy-bookmarks-list-model.h |   36 ++++++
 src/bookmarks/ephy-bookmarks-manager.c    |   54 +--------
 src/bookmarks/ephy-bookmarks-popover.c    |   20 +++-
 src/ephy-location-controller.c            |    2 +
 6 files changed, 251 insertions(+), 56 deletions(-)
---
diff --git a/src/Makefile.am b/src/Makefile.am
index ec8a1db..6310e62 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -30,6 +30,8 @@ libephymain_la_SOURCES = \
        bookmarks/ephy-bookmark-properties-grid.h       \
        bookmarks/ephy-bookmark-row.c           \
        bookmarks/ephy-bookmark-row.h           \
+       bookmarks/ephy-bookmarks-list-model.c   \
+       bookmarks/ephy-bookmarks-list-model.h   \
        bookmarks/ephy-bookmarks-manager.c      \
        bookmarks/ephy-bookmarks-manager.h      \
        bookmarks/ephy-bookmarks-popover.c      \
diff --git a/src/bookmarks/ephy-bookmarks-list-model.c b/src/bookmarks/ephy-bookmarks-list-model.c
new file mode 100644
index 0000000..d3145dc
--- /dev/null
+++ b/src/bookmarks/ephy-bookmarks-list-model.c
@@ -0,0 +1,193 @@
+/* -*- Mode: C; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/*
+ *  Copyright © 2016 Igalia S.L.
+ *
+ *  This file is part of Epiphany.
+ *
+ *  Epiphany is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  Epiphany is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with Epiphany.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "config.h"
+
+#include "ephy-bookmarks-list-model.h"
+
+struct _EphyBookmarksListModel {
+  GObject     parent_instance;
+
+  EphyBookmarksManager *bookmarks_manager;
+  GList *dumb_bookmarks;
+};
+
+static void list_model_iface_init (GListModelInterface *iface);
+
+G_DEFINE_TYPE_WITH_CODE (EphyBookmarksListModel, ephy_bookmarks_list_model, G_TYPE_OBJECT,
+                         G_IMPLEMENT_INTERFACE (G_TYPE_LIST_MODEL, list_model_iface_init))
+
+enum {
+  PROP_0,
+  PROP_BOOKMARKS_MANAGER,
+  LAST_PROP
+};
+
+static GParamSpec *obj_properties[LAST_PROP];
+
+static void
+maybe_add_bookmark (EphyBookmark           *bookmark,
+                    EphyBookmarksListModel *self)
+{
+  if (!ephy_bookmark_is_smart (bookmark))
+    self->dumb_bookmarks = g_list_append (self->dumb_bookmarks, g_object_ref (bookmark));
+}
+
+static void
+refresh_bookmarks_list (EphyBookmarksListModel *self)
+{
+  GSequence *bookmarks;
+  guint previous_length = 0;
+
+  if (self->dumb_bookmarks != NULL) {
+    previous_length = g_list_length (self->dumb_bookmarks);
+    g_list_free_full (self->dumb_bookmarks, g_object_unref);
+    self->dumb_bookmarks = NULL;
+  }
+
+  bookmarks = ephy_bookmarks_manager_get_bookmarks (self->bookmarks_manager);
+  g_sequence_foreach (bookmarks, (GFunc)maybe_add_bookmark, self);
+
+  g_list_model_items_changed (G_LIST_MODEL (self),
+                              0,
+                              previous_length,
+                              g_list_length (self->dumb_bookmarks));
+}
+
+static void
+bookmarks_modified_cb (EphyBookmarksManager   *manager,
+                       EphyBookmark           *bookmark,
+                       EphyBookmarksListModel *self)
+{
+  if (!ephy_bookmark_is_smart (bookmark))
+    refresh_bookmarks_list (self);
+}
+
+static GType
+ephy_bookmarks_list_model_list_model_get_item_type (GListModel *model)
+{
+  return EPHY_TYPE_BOOKMARK;
+}
+
+static guint
+ephy_bookmarks_list_model_list_model_get_n_items (GListModel *model)
+{
+  EphyBookmarksListModel *self = EPHY_BOOKMARKS_LIST_MODEL (model);
+
+  return g_list_length (self->dumb_bookmarks);
+}
+
+static gpointer
+ephy_bookmarks_list_model_list_model_get_item (GListModel *model,
+                                               guint       position)
+{
+  EphyBookmarksListModel *self = EPHY_BOOKMARKS_LIST_MODEL (model);
+
+  return g_object_ref (g_list_nth_data (self->dumb_bookmarks, position));
+}
+
+static void
+list_model_iface_init (GListModelInterface *iface)
+{
+  iface->get_item_type = ephy_bookmarks_list_model_list_model_get_item_type;
+  iface->get_n_items = ephy_bookmarks_list_model_list_model_get_n_items;
+  iface->get_item = ephy_bookmarks_list_model_list_model_get_item;
+}
+
+static void
+ephy_bookmarks_list_model_dispose (GObject *object)
+{
+  EphyBookmarksListModel *self = EPHY_BOOKMARKS_LIST_MODEL (object);
+
+  if (self->dumb_bookmarks != NULL) {
+    g_list_free_full (self->dumb_bookmarks, g_object_unref);
+    self->dumb_bookmarks = NULL;
+  }
+
+  G_OBJECT_CLASS (ephy_bookmarks_list_model_parent_class)->dispose (object);
+}
+
+static void
+ephy_bookmarks_list_model_set_property (GObject      *object,
+                                        guint         prop_id,
+                                        const GValue *value,
+                                        GParamSpec   *pspec)
+{
+  EphyBookmarksListModel *self = EPHY_BOOKMARKS_LIST_MODEL (object);
+
+  switch (prop_id) {
+    case PROP_BOOKMARKS_MANAGER:
+      self->bookmarks_manager = g_value_get_object (value);
+      break;
+    default:
+      G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
+  }
+}
+
+static void
+ephy_bookmarks_list_model_constructed (GObject *object)
+{
+  EphyBookmarksListModel *self = EPHY_BOOKMARKS_LIST_MODEL (object);
+
+  G_OBJECT_CLASS (ephy_bookmarks_list_model_parent_class)->constructed (object);
+
+  refresh_bookmarks_list (self);
+
+  g_signal_connect_object (self->bookmarks_manager, "bookmark-added",
+                           G_CALLBACK (bookmarks_modified_cb), self, 0);
+  g_signal_connect_object (self->bookmarks_manager, "bookmark-removed",
+                           G_CALLBACK (bookmarks_modified_cb), self, 0);
+  g_signal_connect_object (self->bookmarks_manager, "bookmark-title-changed",
+                           G_CALLBACK (bookmarks_modified_cb), self, 0);
+  g_signal_connect_object (self->bookmarks_manager, "bookmark-url-changed",
+                           G_CALLBACK (bookmarks_modified_cb), self, 0);
+}
+
+static void
+ephy_bookmarks_list_model_class_init (EphyBookmarksListModelClass *klass)
+{
+  GObjectClass *object_class = G_OBJECT_CLASS (klass);
+
+  object_class->constructed = ephy_bookmarks_list_model_constructed;
+  object_class->dispose = ephy_bookmarks_list_model_dispose;
+  object_class->set_property = ephy_bookmarks_list_model_set_property;
+
+  obj_properties[PROP_BOOKMARKS_MANAGER] =
+    g_param_spec_object ("bookmarks-manager",
+                         "The bookmarks manager",
+                         "The bookmarks manager",
+                         EPHY_TYPE_BOOKMARKS_MANAGER,
+                         G_PARAM_WRITABLE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS);
+
+  g_object_class_install_properties (object_class, LAST_PROP, obj_properties);
+}
+
+static void
+ephy_bookmarks_list_model_init (EphyBookmarksListModel *self)
+{
+}
+
+EphyBookmarksListModel *
+ephy_bookmarks_list_model_new (EphyBookmarksManager *manager)
+{
+  return EPHY_BOOKMARKS_LIST_MODEL (g_object_new (EPHY_TYPE_BOOKMARKS_LIST_MODEL,
+                                                  "bookmarks-manager", manager,
+                                                  NULL));
+}
diff --git a/src/bookmarks/ephy-bookmarks-list-model.h b/src/bookmarks/ephy-bookmarks-list-model.h
new file mode 100644
index 0000000..adee3cc
--- /dev/null
+++ b/src/bookmarks/ephy-bookmarks-list-model.h
@@ -0,0 +1,36 @@
+/* -*- Mode: C; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/*
+ *  Copyright © 2016 Igalia S.L.
+ *
+ *  This file is part of Epiphany.
+ *
+ *  Epiphany is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  Epiphany is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with Epiphany.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include "ephy-bookmark.h"
+#include "ephy-bookmarks-manager.h"
+
+#include <glib-object.h>
+
+G_BEGIN_DECLS
+
+#define EPHY_TYPE_BOOKMARKS_LIST_MODEL (ephy_bookmarks_list_model_get_type ())
+
+G_DECLARE_FINAL_TYPE (EphyBookmarksListModel, ephy_bookmarks_list_model, EPHY, BOOKMARKS_LIST_MODEL, GObject)
+
+EphyBookmarksListModel *ephy_bookmarks_list_model_new (EphyBookmarksManager *manager);
+
+G_END_DECLS
diff --git a/src/bookmarks/ephy-bookmarks-manager.c b/src/bookmarks/ephy-bookmarks-manager.c
index f768b83..a630189 100644
--- a/src/bookmarks/ephy-bookmarks-manager.c
+++ b/src/bookmarks/ephy-bookmarks-manager.c
@@ -38,10 +38,7 @@ struct _EphyBookmarksManager {
   gchar      *gvdb_filename;
 };
 
-static void list_model_iface_init     (GListModelInterface *iface);
-
-G_DEFINE_TYPE_EXTENDED (EphyBookmarksManager, ephy_bookmarks_manager, G_TYPE_OBJECT, 0,
-                        G_IMPLEMENT_INTERFACE (G_TYPE_LIST_MODEL, list_model_iface_init))
+G_DEFINE_TYPE (EphyBookmarksManager, ephy_bookmarks_manager, G_TYPE_OBJECT)
 
 enum {
   BOOKMARK_ADDED,
@@ -245,48 +242,6 @@ count_smart_bookmarks_prior_to (EphyBookmarksManager *self,
   return count;
 }
 
-static GType
-ephy_bookmarks_manager_list_model_get_item_type (GListModel *model)
-{
-  return EPHY_TYPE_BOOKMARK;
-}
-
-static guint
-ephy_bookmarks_manager_list_model_get_n_items (GListModel *model)
-{
-  EphyBookmarksManager *self = EPHY_BOOKMARKS_MANAGER (model);
-  guint length = g_sequence_get_length (self->bookmarks);
-
-  return length - count_smart_bookmarks_prior_to (self, length);
-}
-
-static gpointer
-ephy_bookmarks_manager_list_model_get_item (GListModel *model,
-                                            guint       position)
-{
-  EphyBookmarksManager *self = EPHY_BOOKMARKS_MANAGER (model);
-  GSequenceIter *iter = g_sequence_get_begin_iter (self->bookmarks);
-  EphyBookmark *bookmark = NULL;
-  guint i = 0;
-
-  do {
-    bookmark = g_sequence_get (iter);
-    if (!ephy_bookmark_is_smart (bookmark))
-      i++;
-    iter = g_sequence_iter_next (iter);
-  } while (i < position); //FIXME: should be <= right?
-
-  return bookmark ? g_object_ref (bookmark) : NULL;
-}
-
-static void
-list_model_iface_init (GListModelInterface *iface)
-{
-  iface->get_item_type = ephy_bookmarks_manager_list_model_get_item_type;
-  iface->get_n_items = ephy_bookmarks_manager_list_model_get_n_items;
-  iface->get_item = ephy_bookmarks_manager_list_model_get_item;
-}
-
 static void
 bookmark_title_changed_cb (EphyBookmark         *bookmark,
                            GParamSpec           *pspec,
@@ -327,9 +282,6 @@ ephy_bookmarks_manager_add_bookmark (EphyBookmarksManager *self,
 
       /* Update list */
       position = g_sequence_iter_get_position (iter);
-      g_list_model_items_changed (G_LIST_MODEL (self),
-                                  position - count_smart_bookmarks_prior_to (self, position) - 1,
-                                  0, 1);
 
       g_signal_emit (self, signals[BOOKMARK_ADDED], 0, bookmark);
 
@@ -338,7 +290,6 @@ ephy_bookmarks_manager_add_bookmark (EphyBookmarksManager *self,
                                                  NULL);
   }
 
-  // FIXME: Need g_list_model_items_changed if a bookmark's smartness changes
   g_signal_connect_object (bookmark, "notify::title",
                            G_CALLBACK (bookmark_title_changed_cb), self, 0);
   g_signal_connect_object (bookmark, "notify::url",
@@ -397,9 +348,6 @@ ephy_bookmarks_manager_remove_bookmark (EphyBookmarksManager *self,
 
   position = g_sequence_iter_get_position (iter);
   g_sequence_remove (iter);
-  g_list_model_items_changed (G_LIST_MODEL (self),
-                              position - count_smart_bookmarks_prior_to (self, position),
-                              1, 0);
 
   ephy_bookmarks_manager_save_to_file_async (self, NULL,
                                              
(GAsyncReadyCallback)ephy_bookmarks_manager_save_to_file_warn_on_error_cb,
diff --git a/src/bookmarks/ephy-bookmarks-popover.c b/src/bookmarks/ephy-bookmarks-popover.c
index 0b89d28..5f61e0c 100644
--- a/src/bookmarks/ephy-bookmarks-popover.c
+++ b/src/bookmarks/ephy-bookmarks-popover.c
@@ -24,6 +24,7 @@
 
 #include "ephy-bookmark.h"
 #include "ephy-bookmark-row.h"
+#include "ephy-bookmarks-list-model.h"
 #include "ephy-bookmarks-manager.h"
 #include "ephy-debug.h"
 #include "ephy-shell.h"
@@ -40,8 +41,9 @@ struct _EphyBookmarksPopover {
   GtkWidget             *tag_detail_back_button;
   GtkWidget             *tag_detail_label;
 
-  EphyBookmarksManager  *manager;
-  EphyWindow            *window;
+  EphyBookmarksManager   *manager;
+  EphyBookmarksListModel *list_model;
+  EphyWindow             *window;
 };
 
 G_DEFINE_TYPE (EphyBookmarksPopover, ephy_bookmarks_popover, GTK_TYPE_POPOVER)
@@ -413,6 +415,16 @@ ephy_bookmarks_popover_list_box_row_activated_cb (EphyBookmarksPopover   *self,
 }
 
 static void
+ephy_bookmarks_popover_dispose (GObject *object)
+{
+  EphyBookmarksPopover *self = EPHY_BOOKMARKS_POPOVER (object);
+
+  g_clear_object (&self->list_model);
+
+  G_OBJECT_CLASS (ephy_bookmarks_popover_parent_class)->dispose (object);
+}
+
+static void
 ephy_bookmarks_popover_set_property (GObject      *object,
                                      guint         prop_id,
                                      const GValue *value,
@@ -435,6 +447,7 @@ ephy_bookmarks_popover_class_init (EphyBookmarksPopoverClass *klass)
   GObjectClass *object_class = G_OBJECT_CLASS (klass);
   GtkWidgetClass *widget_class = GTK_WIDGET_CLASS (klass);
 
+  object_class->dispose = ephy_bookmarks_popover_dispose;
   object_class->set_property = ephy_bookmarks_popover_set_property;
 
   obj_properties[PROP_WINDOW] =
@@ -470,6 +483,7 @@ ephy_bookmarks_popover_init (EphyBookmarksPopover *self)
   gtk_widget_init_template (GTK_WIDGET (self));
 
   self->manager = ephy_shell_get_bookmarks_manager (ephy_shell_get_default ());
+  self->list_model = ephy_bookmarks_list_model_new (self->manager);
 
   group = g_simple_action_group_new ();
   g_action_map_add_action_entries (G_ACTION_MAP (group), entries,
@@ -479,7 +493,7 @@ ephy_bookmarks_popover_init (EphyBookmarksPopover *self)
   g_object_unref (group);
 
   gtk_list_box_bind_model (GTK_LIST_BOX (self->bookmarks_list_box),
-                           G_LIST_MODEL (self->manager),
+                           G_LIST_MODEL (self->list_model),
                            create_bookmark_row,
                            self, NULL);
 
diff --git a/src/ephy-location-controller.c b/src/ephy-location-controller.c
index 1670e55..958906f 100644
--- a/src/ephy-location-controller.c
+++ b/src/ephy-location-controller.c
@@ -23,6 +23,7 @@
 #include "ephy-location-controller.h"
 
 #include "ephy-bookmark.h"
+#include "ephy-bookmarks-list-model.h"
 #include "ephy-bookmarks-manager.h"
 #include "ephy-completion-model.h"
 #include "ephy-debug.h"
@@ -52,6 +53,7 @@ struct _EphyLocationController {
   EphyWindow *window;
   EphyTitleWidget *title_widget;
   EphyBookmarksManager *bookmarks_manager;
+  EphyBookmarksListModel *list_model;
   GSequence *smart_bookmarks;
   char *address;
   guint editable : 1;
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]