Commit 55ecc77f authored by Jim Nelson's avatar Jim Nelson

#1067: Undo drag-and-drop move photos to event no longer asserts. Also...

#1067: Undo drag-and-drop move photos to event no longer asserts.  Also cleaned up some code that, in this 
particular code path, was preventing Pages from being fully unref'ed.
parent 211a38e7
......@@ -5,4 +5,5 @@ David Jeske <davidj@gmail.com>
Matthias Clasen <matthias.clasen@gmail.com>
Emanuele Grande <caccolangrifata@gmail.com>
Joeny Ang <ang.joeny@gmail.com>
Ingo Lütkebohle <iluetkeb@techfak.uni-bielefeld.de>
......@@ -344,7 +344,8 @@ public class CheckerboardLayout : Gtk.DrawingArea {
clear_drag_select();
reflow_scheduler = new OneShotScheduler(background_reflow);
reflow_scheduler = new OneShotScheduler("CheckerboardLayout for %s".printf(view.to_string()),
background_reflow);
// set existing items to be part of this layout
foreach (DataObject object in view.get_all())
......@@ -366,6 +367,10 @@ public class CheckerboardLayout : Gtk.DrawingArea {
}
~CheckerboardLayout() {
#if TRACE_DTORS
debug("DTOR: CheckerboardLayout for %s", view.to_string());
#endif
view.contents_altered -= on_contents_altered;
view.item_altered -= on_item_altered;
view.item_metadata_altered -= on_item_metadata_altered;
......@@ -385,6 +390,8 @@ public class CheckerboardLayout : Gtk.DrawingArea {
if (parent != null)
parent.size_allocate -= on_viewport_resized;
reflow_scheduler.cancel();
}
public void set_adjustments(Gtk.Adjustment hadjustment, Gtk.Adjustment vadjustment) {
......
......@@ -903,9 +903,7 @@ public class CollectionPage : CheckerboardPage {
}
private bool hidden_photo_filter(DataView view) {
Thumbnail thumbnail = (Thumbnail) view;
return !thumbnail.get_photo().is_hidden();
return !((Thumbnail) view).get_photo().is_hidden();
}
private static double scale_to_slider(int value) {
......
......@@ -36,7 +36,7 @@ public abstract class Command : Object, CommandDescription {
~Command() {
#if TRACE_DTORS
debug("Command dtor: %s/%s", name, explanation);
debug("DTOR: Command %s (%s)", name, explanation);
#endif
}
......
......@@ -40,7 +40,8 @@ public abstract class PageCommand : Command {
}
private void on_page_removed() {
get_command_manager().reset();
page.removed -= on_page_removed;
page = null;
}
}
......
......@@ -93,13 +93,13 @@ public class DataCollection {
}
}
public class OrderAddedComparator : Comparator<DataObject> {
public static class OrderAddedComparator : Comparator<DataObject> {
public override int64 compare(DataObject a, DataObject b) {
return a.internal_get_ordinal() - b.internal_get_ordinal();
}
}
protected class ComparatorWrapper : Comparator<DataObject> {
protected static class ComparatorWrapper : Comparator<DataObject> {
private Comparator<DataObject> comparator;
public ComparatorWrapper(Comparator<DataObject> comparator) {
......@@ -163,7 +163,7 @@ public class DataCollection {
~DataCollection() {
#if TRACE_DTORS
debug("DataCollection dtor: %s", name);
debug("DTOR: DataCollection %s", name);
#endif
}
......@@ -428,6 +428,16 @@ public class DataCollection {
assert(hash_set.size == 0);
}
// close() must be called before disposing of the DataCollection, so all signals may be
// disconnected and all internal references to the collection can be dropped. In the bare
// minimum, all items will be removed from the collection (and the appropriate signals and
// notify calls will be made). Subclasses may fire other signals while disposing of their
// references. However, if they are entirely synchronized on DataCollection's signals, that
// may be enough for them to clean up.
public virtual void close() {
clear();
}
// This method is only called by DataObject to report when it has been altered, so observers of
// this collection may be notified as well.
public void internal_notify_altered(DataObject object) {
......@@ -638,8 +648,11 @@ public class ViewCollection : DataCollection {
visible.resort(get_order_added_comparator());
}
~ViewCollection() {
public override void close() {
halt_monitoring();
filter = null;
base.close();
}
public void monitor_source_collection(SourceCollection sources, ViewManager manager) {
......@@ -672,6 +685,9 @@ public class ViewCollection : DataCollection {
}
public void install_view_filter(ViewFilter filter) {
if (this.filter == filter)
return;
// this currently replaces any existing ViewFilter
this.filter = filter;
......
......@@ -21,6 +21,12 @@ public abstract class DataObject {
private static int64 object_id_generator = 0;
#if TRACE_DTORS
// because calling to_string() in a destructor is dangerous, stash to_string()'s result in
// this variable for reporting
protected string dbg_to_string = null;
#endif
private int64 object_id = INVALID_OBJECT_ID;
private DataCollection member_of = null;
private int64 ordinal = DataCollection.INVALID_OBJECT_ORDINAL;
......@@ -86,6 +92,10 @@ public abstract class DataObject {
this.ordinal = ordinal;
notify_membership_changed(member_of);
#if TRACE_DTORS
dbg_to_string = to_string();
#endif
}
// This method is only called by DataCollection
......@@ -164,7 +174,7 @@ public abstract class DataSource : DataObject {
~DataSource() {
#if TRACE_DTORS
debug("DataSource destroyed");
debug("DTOR: DataSource %s", dbg_to_string);
#endif
}
......@@ -500,7 +510,7 @@ public class DataView : DataObject {
~DataView() {
#if TRACE_DTORS
debug("DataView destroyed");
debug("DTOR: DataView %s", dbg_to_string);
#endif
}
......
......@@ -127,7 +127,7 @@ public class Event : EventSource, Proxyable {
if (primary_photo != null)
primary_photo.thumbnail_altered += on_primary_thumbnail_altered;
view = new ViewCollection("ViewCollection for %lld".printf(event_id.id));
view = new ViewCollection("ViewCollection for Event %lld".printf(event_id.id));
view.monitor_source_collection(LibraryPhoto.global, new EventManager(event_id));
// watch for for removal and addition of photos
......
......@@ -291,6 +291,12 @@ public class EventPage : CollectionPage {
get_view().monitor_source_collection(LibraryPhoto.global, new EventViewManager(this));
init_page_context_menu("/EventContextMenu");
page_event.altered += on_event_altered;
}
~EventPage() {
page_event.altered -= on_event_altered;
}
private static Gtk.ActionEntry[] create_actions() {
......@@ -310,6 +316,10 @@ public class EventPage : CollectionPage {
return new_actions;
}
private void on_event_altered() {
set_page_name(page_event.get_name());
}
protected override void on_photos_menu() {
set_item_sensitive("/CollectionMenuBar/PhotosMenu/MakePrimary",
get_view().get_selected_count() == 1);
......
......@@ -314,7 +314,8 @@ public class LibraryWindow : AppWindow {
extended_properties.hide += hide_extended_properties;
extended_properties.show += show_extended_properties;
page_removal_scheduler = new OneShotScheduler(hide_remove_pages_internal);
page_removal_scheduler = new OneShotScheduler("Hide/Remove Pages Scheduler",
hide_remove_pages_internal);
}
~LibraryWindow() {
......@@ -331,6 +332,8 @@ public class LibraryWindow : AppWindow {
extended_properties.hide -= hide_extended_properties;
extended_properties.show -= show_extended_properties;
page_removal_scheduler.cancel();
}
private Gtk.ActionEntry[] create_actions() {
......
......@@ -69,23 +69,24 @@ public abstract class Page : Gtk.ScrolledWindow, SidebarPage {
~Page() {
#if TRACE_DTORS
debug("Page destroyed: %s", page_name);
debug("DTOR: Page %s", page_name);
#endif
}
// This is called by the page controller when it has removed this page ... pages should override
// this (or the signal) to clean up
public virtual void notify_removed() {
// signal prior to shutdown
removed();
// untie signals
detach_event_source();
view.halt_monitoring();
view.close();
// remove refs to external objects which may be pointing to the Page
layout = null;
clear_marker();
clear_container();
removed();
}
public string get_page_name() {
......
......@@ -25,7 +25,7 @@ public class Thumbnail : LayoutItem {
base(photo, photo.get_dimensions().get_scaled(scale, true));
this.scale = scale;
hq_scheduler = new OneShotScheduler(on_schedule_high_quality);
hq_scheduler = new OneShotScheduler("Thumbnail HQ scheduler", on_schedule_high_quality);
set_title(photo.get_name());
......
......@@ -128,7 +128,7 @@ public class ThumbnailCache : Object {
// Doing this because static construct {} not working nor new'ing in the above statement
public static void init() {
debug_scheduler = new OneShotScheduler(report_cycle);
debug_scheduler = new OneShotScheduler("ThumbnailCache cycle reporter", report_cycle);
fetch_workers = new Workers(Workers.THREAD_PER_CPU, false);
big = new ThumbnailCache(Size.BIG, MAX_BIG_CACHED_BYTES);
......
......@@ -379,15 +379,25 @@ public string format_local_date(Time date) {
public delegate void OneShotCallback();
public class OneShotScheduler {
private string name;
private OneShotCallback callback;
private bool scheduled = false;
private bool reschedule = false;
private bool cancelled = false;
public OneShotScheduler(OneShotCallback callback) {
public OneShotScheduler(string name, OneShotCallback callback) {
this.name = name;
this.callback = callback;
}
~OneShotScheduler() {
#if TRACE_DTORS
debug("DTOR: OneShotScheduler for %s", name);
#endif
cancelled = true;
}
public bool is_scheduled() {
return scheduled;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment