diff --git a/podcasts-gtk/src/utils.rs b/podcasts-gtk/src/utils.rs index 01173a7..95cf8dd 100644 --- a/podcasts-gtk/src/utils.rs +++ b/podcasts-gtk/src/utils.rs @@ -1,8 +1,8 @@ #![cfg_attr(feature = "cargo-clippy", allow(type_complexity))] use gdk::FrameClockExt; -use gdk_pixbuf::Pixbuf; -use glib; +use gdk_pixbuf::{Object, Pixbuf}; +use glib::{self, object::WeakRef}; use gtk; use gtk::prelude::*; use gtk::{IsA, Widget}; @@ -64,16 +64,26 @@ use i18n::i18n; /// let list = gtk::ListBox::new(); /// lazy_load(widgets, list, |w| w, || {}); /// ``` -pub(crate) fn lazy_load(data: T, container: C, mut contructor: F, callback: U) -where +pub(crate) fn lazy_load( + data: T, + container: WeakRef, + mut contructor: F, + callback: U, +) where T: IntoIterator + 'static, T::Item: 'static, - C: ContainerExt + 'static, + // FIXME: leaking a strong refference here + C: IsA + ContainerExt + 'static, F: FnMut(T::Item) -> W + 'static, W: IsA + WidgetExt, U: Fn() + 'static, { let func = move |x| { + let container = match container.upgrade() { + Some(c) => c, + None => return, + }; + let widget = contructor(x); container.add(&widget); widget.show(); diff --git a/podcasts-gtk/src/widgets/player.rs b/podcasts-gtk/src/widgets/player.rs index de11e33..b2687d1 100644 --- a/podcasts-gtk/src/widgets/player.rs +++ b/podcasts-gtk/src/widgets/player.rs @@ -6,7 +6,7 @@ use gtk; use gtk::prelude::*; use gio::{File, FileExt}; -use glib::SignalHandlerId; +use glib::{SignalHandlerId, WeakRef}; use chrono::NaiveTime; use crossbeam_channel::Sender; @@ -204,7 +204,8 @@ impl Default for PlayerWidget { let separator = builder.get_object("separator").unwrap(); let slider: gtk::Scale = builder.get_object("seek").unwrap(); slider.set_range(0.0, 1.0); - let slider_update = Rc::new(Self::connect_update_slider(&slider, &player)); + let player_weak = player.downgrade(); + let slider_update = Rc::new(Self::connect_update_slider(&slider, player_weak)); let timer = PlayerTimes { container: timer_container, progressed, @@ -385,11 +386,19 @@ impl PlayerWidget { Ok(()) } - fn connect_update_slider(slider: >k::Scale, player: &gst_player::Player) -> SignalHandlerId { - slider.connect_value_changed(clone!(player => move |slider| { + fn connect_update_slider( + slider: >k::Scale, + player: WeakRef, + ) -> SignalHandlerId { + slider.connect_value_changed(move |slider| { + let player = match player.upgrade() { + Some(p) => p, + None => return, + }; + let value = slider.get_value() as u64; player.seek(ClockTime::from_seconds(value)); - })) + }) } } diff --git a/podcasts-gtk/src/widgets/show.rs b/podcasts-gtk/src/widgets/show.rs index cf3440d..1be20a5 100644 --- a/podcasts-gtk/src/widgets/show.rs +++ b/podcasts-gtk/src/widgets/show.rs @@ -1,7 +1,7 @@ use glib; use gtk::{self, prelude::*, Adjustment}; -use crossbeam_channel::Sender; +use crossbeam_channel::{bounded, Sender}; use failure::Error; use fragile::Fragile; use html2text; @@ -103,14 +103,11 @@ impl ShowWidget { /// Populate the listbox with the shows episodes. fn populate_listbox( - // FIXME: we are leaking strong refs here show: &Rc, pd: Arc, sender: Sender, vadj: Option, ) -> Result<(), Error> { - use crossbeam_channel::bounded; - let count = dbqueries::get_pd_episodes_count(&pd)?; let (sender_, receiver) = bounded(1); @@ -128,7 +125,8 @@ fn populate_listbox( return Ok(()); } - let show_ = show.clone(); + let show_weak = Rc::downgrade(&show); + let list_weak = show.episodes.downgrade(); gtk::idle_add(move || { let episodes = match receiver.try_recv() { Some(e) => e, @@ -136,18 +134,18 @@ fn populate_listbox( }; debug_assert!(episodes.len() as i64 == count); - let list = show_.episodes.clone(); let constructor = clone!(sender => move |ep| { EpisodeWidget::new(ep, &sender).container.clone() }); - let callback = clone!(show_, vadj => move || { - if let Some(ref v) = vadj { - show_.view.set_adjutments(None, Some(v)) + let callback = clone!(show_weak, vadj => move || { + match (show_weak.upgrade(), &vadj) { + (Some(ref shows), Some(ref v)) => shows.view.set_adjutments(None, Some(v)), + _ => (), }; }); - lazy_load(episodes, list.clone(), constructor, callback); + lazy_load(episodes, list_weak.clone(), constructor, callback); glib::Continue(false) }); diff --git a/podcasts-gtk/src/widgets/show_menu.rs b/podcasts-gtk/src/widgets/show_menu.rs index e46a256..b67df6e 100644 --- a/podcasts-gtk/src/widgets/show_menu.rs +++ b/podcasts-gtk/src/widgets/show_menu.rs @@ -68,8 +68,13 @@ impl ShowMenu { } fn connect_played(&self, pd: &Arc, episodes: >k::ListBox, sender: &Sender) { - self.played - .connect_clicked(clone!(pd, episodes, sender => move |_| { + let episodes_weak = episodes.downgrade(); + self.played.connect_clicked(clone!(pd, sender => move |_| { + let episodes = match episodes_weak.upgrade() { + Some(e) => e, + None => return, + }; + let res = dim_titles(&episodes); debug_assert!(res.is_some()); @@ -121,6 +126,7 @@ fn dim_titles(episodes: >k::ListBox) -> Option<()> { } fn mark_all_watched(pd: &Show, sender: &Sender) -> Result<(), Error> { + // TODO: If this fails for whatever reason, it should be impossible, show an error dbqueries::update_none_to_played_now(pd)?; // Not all widgets might have been loaded when the mark_all is hit // So we will need to refresh again after it's done. diff --git a/podcasts-gtk/src/widgets/shows_view.rs b/podcasts-gtk/src/widgets/shows_view.rs index e4bace7..8fec376 100644 --- a/podcasts-gtk/src/widgets/shows_view.rs +++ b/podcasts-gtk/src/widgets/shows_view.rs @@ -64,17 +64,18 @@ impl ShowsView { fn populate_flowbox(shows: &Rc, vadj: Option) -> Result<(), Error> { let ignore = get_ignored_shows()?; let podcasts = dbqueries::get_podcasts_filter(&ignore)?; + let show_weak = Rc::downgrade(&shows); + let flowbox_weak = shows.flowbox.downgrade(); let constructor = move |parent| ShowsChild::new(&parent).child; - // FIXME: We are, possibly,leaking the strong ref here - let callback = clone!(shows => move || { - if let Some(ref v) = vadj { - shows.view.set_adjutments(None, Some(v)) + let callback = move || { + match (show_weak.upgrade(), &vadj) { + (Some(ref shows), Some(ref v)) => shows.view.set_adjutments(None, Some(v)), + _ => (), }; - }); + }; - let flowbox = shows.flowbox.clone(); - lazy_load(podcasts, flowbox, constructor, callback); + lazy_load(podcasts, flowbox_weak, constructor, callback); Ok(()) }