diff --git a/hammond-gtk/src/app.rs b/hammond-gtk/src/app.rs index 80e1ae9..3c51f1b 100644 --- a/hammond-gtk/src/app.rs +++ b/hammond-gtk/src/app.rs @@ -156,14 +156,8 @@ impl App { let sender = self.sender.clone(); if self.settings.get_boolean("refresh-on-startup") { info!("Refresh on startup."); - // The ui loads async, after initialization - // so we need to delay this a bit so it won't block - // requests that will come from loading the gui on startup. - gtk::timeout_add(1500, move || { - let s: Option> = None; - utils::refresh(s, sender.clone()); - glib::Continue(false) - }); + let s: Option> = None; + utils::refresh(s, sender.clone()); } } diff --git a/hammond-gtk/src/widgets/episode.rs b/hammond-gtk/src/widgets/episode.rs index 720c244..754ad3e 100644 --- a/hammond-gtk/src/widgets/episode.rs +++ b/hammond-gtk/src/widgets/episode.rs @@ -17,7 +17,8 @@ use hammond_data::EpisodeWidgetModel; use app::Action; use manager; -use std::rc::Rc; +use std::cell::RefCell; +use std::rc::{Rc, Weak}; use std::sync::{Arc, Mutex, TryLockError}; lazy_static! { @@ -206,12 +207,37 @@ impl Default for EpisodeWidget { } impl EpisodeWidget { - pub fn new(episode: &EpisodeWidgetModel, sender: &Sender) -> Rc { + pub fn new(episode: EpisodeWidgetModel, sender: &Sender) -> Rc { let widget = Rc::new(Self::default()); - widget.info.init(episode); - Self::determine_buttons_state(&widget, episode, sender) - .map_err(|err| error!("Error: {}", err)) - .ok(); + let episode = RefCell::new(Some(episode)); + let weak = Rc::downgrade(&widget); + widget.container.connect_draw(clone!(sender => move |_, _| { + episode.borrow_mut().take().map(|ep| { + weak.upgrade().map(|w| w.info.init(&ep)); + Self::determine_buttons_state(&weak, &ep, &sender) + .map_err(|err| error!("Error: {}", err)) + .ok(); + }); + + gtk::Inhibit(false) + })); + + // When the widget is attached to a parent, + // since it's a rust struct and not a widget the + // compiler drops the refference to it at the end of + // scope. That's cause we only attach the `self.container` + // to the parent. + // + // So this callback keeps a refference to the Rust Struct + // so the compiler won't drop it. + // + // When the widget is detached from it's parent view this + // callback runs freeing the last refference we were holding. + let foo = RefCell::new(Some(widget.clone())); + widget.container.connect_remove(move |_, _| { + foo.borrow_mut().take(); + }); + widget } @@ -296,10 +322,13 @@ impl EpisodeWidget { /// | ToDownload | /// ------------------- fn determine_buttons_state( - widget: &Rc, + weak: &Weak, episode: &EpisodeWidgetModel, sender: &Sender, ) -> Result<(), Error> { + let widget = weak + .upgrade() + .ok_or_else(|| format_err!("Widget is already dropped"))?; // Reset the buttons state no matter the glade file. // This is just to make it easier to port to relm in the future. widget.buttons.cancel.hide(); @@ -319,11 +348,11 @@ impl EpisodeWidget { // State: InProggress if let Some(prog) = active_dl()? { // set a callback that will update the state when the download finishes - let callback = clone!(widget, sender => move || { + let callback = clone!(weak, sender => move || { if let Ok(guard) = manager::ACTIVE_DOWNLOADS.read() { if !guard.contains_key(&id) { if let Ok(ep) = dbqueries::get_episode_widget_from_rowid(id) { - Self::determine_buttons_state(&widget, &ep, &sender) + Self::determine_buttons_state(&weak, &ep, &sender) .map_err(|err| error!("Error: {}", err)) .ok(); @@ -340,20 +369,20 @@ impl EpisodeWidget { widget .buttons .cancel - .connect_clicked(clone!(prog, widget, sender => move |_| { + .connect_clicked(clone!(prog, weak, sender => move |_| { // Cancel the download if let Ok(mut m) = prog.lock() { m.cancel(); } // Cancel is not instant so we have to wait a bit - timeout_add(50, clone!(widget, sender => move || { + timeout_add(50, clone!(weak, sender => move || { if let Ok(thing) = active_dl() { if thing.is_none() { // Recalculate the widget state dbqueries::get_episode_widget_from_rowid(id) .map_err(From::from) - .and_then(|ep| Self::determine_buttons_state(&widget, &ep, &sender)) + .and_then(|ep| Self::determine_buttons_state(&weak, &ep, &sender)) .map_err(|err| error!("Error: {}", err)) .ok(); @@ -368,10 +397,10 @@ impl EpisodeWidget { // Setup a callback that will update the total_size label // with the http ContentLength header number rather than // relying to the RSS feed. - update_total_size_callback(&widget, &prog); + update_total_size_callback(&weak, &prog); // Setup a callback that will update the progress bar. - update_progressbar_callback(&widget, &prog, id); + update_progressbar_callback(&weak, &prog, id); // Change the widget layout/state widget.state_prog(); @@ -388,9 +417,9 @@ impl EpisodeWidget { widget .buttons .play - .connect_clicked(clone!(widget, sender => move |_| { + .connect_clicked(clone!(weak, sender => move |_| { if let Ok(mut ep) = dbqueries::get_episode_widget_from_rowid(id) { - on_play_bttn_clicked(&widget, &mut ep, &sender) + on_play_bttn_clicked(&weak, &mut ep, &sender) .map_err(|err| error!("Error: {}", err)) .ok(); } @@ -404,14 +433,14 @@ impl EpisodeWidget { widget .buttons .download - .connect_clicked(clone!(widget, sender => move |dl| { + .connect_clicked(clone!(weak, sender => move |dl| { // Make the button insensitive so it won't be pressed twice dl.set_sensitive(false); if let Ok(ep) = dbqueries::get_episode_widget_from_rowid(id) { on_download_clicked(&ep, &sender) .and_then(|_| { info!("Donwload started succesfully."); - Self::determine_buttons_state(&widget, &ep, &sender) + Self::determine_buttons_state(&weak, &ep, &sender) }) .map_err(|err| error!("Error: {}", err)) .ok(); @@ -441,10 +470,14 @@ fn on_download_clicked(ep: &EpisodeWidgetModel, sender: &Sender) -> Resu } fn on_play_bttn_clicked( - widget: &Rc, + widget: &Weak, episode: &mut EpisodeWidgetModel, sender: &Sender, ) -> Result<(), Error> { + let widget = widget + .upgrade() + .ok_or_else(|| format_err!("Widget is already dropped"))?; + // Mark played episode.set_played_now()?; // Grey out the title @@ -461,7 +494,7 @@ fn on_play_bttn_clicked( #[inline] #[cfg_attr(feature = "cargo-clippy", allow(if_same_then_else))] fn update_progressbar_callback( - widget: &Rc, + widget: &Weak, prog: &Arc>, episode_rowid: i32, ) { @@ -474,10 +507,15 @@ fn update_progressbar_callback( #[allow(if_same_then_else)] fn progress_bar_helper( - widget: &Rc, + widget: &Weak, prog: &Arc>, episode_rowid: i32, ) -> Result { + let widget = match widget.upgrade() { + Some(w) => w, + None => return Ok(glib::Continue(false)), + }; + let (fraction, downloaded) = match prog.try_lock() { Ok(guard) => (guard.get_fraction(), guard.get_downloaded()), Err(TryLockError::WouldBlock) => return Ok(glib::Continue(true)), @@ -517,7 +555,7 @@ fn progress_bar_helper( // with the http ContentLength header number rather than // relying to the RSS feed. #[inline] -fn update_total_size_callback(widget: &Rc, prog: &Arc>) { +fn update_total_size_callback(widget: &Weak, prog: &Arc>) { let callback = clone!(prog, widget => move || { total_size_helper(&widget, &prog).unwrap_or(glib::Continue(true)) }); @@ -525,9 +563,14 @@ fn update_total_size_callback(widget: &Rc, prog: &Arc, + widget: &Weak, prog: &Arc>, ) -> Result { + let widget = match widget.upgrade() { + Some(w) => w, + None => return Ok(glib::Continue(false)), + }; + // Get the total_bytes. let total_bytes = match prog.try_lock() { Ok(guard) => guard.get_total_size(), diff --git a/hammond-gtk/src/widgets/home_view.rs b/hammond-gtk/src/widgets/home_view.rs index 584491a..a9bd175 100644 --- a/hammond-gtk/src/widgets/home_view.rs +++ b/hammond-gtk/src/widgets/home_view.rs @@ -13,6 +13,7 @@ use app::Action; use utils::{self, lazy_load_full}; use widgets::EpisodeWidget; +use std::cell::Cell; use std::rc::Rc; use std::sync::Mutex; @@ -95,7 +96,7 @@ impl HomeView { let view_ = view.clone(); let func = move |ep: EpisodeWidgetModel| { let epoch = ep.epoch(); - let widget = HomeEpisode::new(&ep, &sender); + let widget = HomeEpisode::new(ep, &sender); match split(&now_utc, i64::from(epoch)) { Today => add_to_box(&widget, &view_.today_list, &view_.today_box), @@ -197,7 +198,7 @@ impl Default for HomeEpisode { } impl HomeEpisode { - fn new(episode: &EpisodeWidgetModel, sender: &Sender) -> HomeEpisode { + fn new(episode: EpisodeWidgetModel, sender: &Sender) -> HomeEpisode { let builder = gtk::Builder::new_from_resource("/org/gnome/Hammond/gtk/episodes_view_widget.ui"); let container: gtk::Box = builder.get_object("container").unwrap(); @@ -216,14 +217,27 @@ impl HomeEpisode { } fn init(&self, show_id: i32) { - self.set_cover(show_id) - .map_err(|err| error!("Failed to set a cover: {}", err)) - .ok(); - + self.set_cover(show_id); self.container.pack_start(&self.episode, true, true, 6); } - fn set_cover(&self, show_id: i32) -> Result<(), Error> { - utils::set_image_from_path(&self.image, show_id, 64) + fn set_cover(&self, show_id: i32) { + // The closure above is a regular `Fn` closure. + // which means we can't mutate stuff inside it easily, + // so Cell is used. + // + // `Option` along with the `.take()` method ensure + // that the function will only be run once, during the first execution. + let show_id = Cell::new(Some(show_id)); + + self.image.connect_draw(move |image, _| { + show_id.take().map(|id| { + utils::set_image_from_path(image, id, 64) + .map_err(|err| error!("Failed to set a cover: {}", err)) + .ok(); + }); + + gtk::Inhibit(false) + }); } } diff --git a/hammond-gtk/src/widgets/show.rs b/hammond-gtk/src/widgets/show.rs index e62249e..5159d2f 100644 --- a/hammond-gtk/src/widgets/show.rs +++ b/hammond-gtk/src/widgets/show.rs @@ -206,7 +206,7 @@ fn populate_listbox( let list = show_.episodes.clone(); let constructor = clone!(sender => move |ep| { - EpisodeWidget::new(&ep, &sender).container.clone() + EpisodeWidget::new(ep, &sender).container.clone() }); let callback = clone!(pd, show_ => move || { diff --git a/hammond-gtk/src/widgets/shows_view.rs b/hammond-gtk/src/widgets/shows_view.rs index 5504a73..59cfef8 100644 --- a/hammond-gtk/src/widgets/shows_view.rs +++ b/hammond-gtk/src/widgets/shows_view.rs @@ -11,6 +11,7 @@ use hammond_data::Show; use app::Action; use utils::{self, get_ignored_shows, lazy_load, set_image_from_path}; +use std::cell::Cell; use std::rc::Rc; use std::sync::Arc; use std::sync::Mutex; @@ -158,11 +159,25 @@ impl ShowsChild { WidgetExt::set_name(&self.child, &pd.id().to_string()); self.set_cover(pd.id()) - .map_err(|err| error!("Failed to set a cover: {}", err)) - .ok(); } - fn set_cover(&self, show_id: i32) -> Result<(), Error> { - set_image_from_path(&self.cover, show_id, 256) + fn set_cover(&self, show_id: i32) { + // The closure above is a regular `Fn` closure. + // which means we can't mutate stuff inside it easily, + // so Cell is used. + // + // `Option` along with the `.take()` method ensure + // that the function will only be run once, during the first execution. + let show_id = Cell::new(Some(show_id)); + + self.cover.connect_draw(move |cover, _| { + show_id.take().map(|id| { + set_image_from_path(cover, id, 256) + .map_err(|err| error!("Failed to set a cover: {}", err)) + .ok(); + }); + + gtk::Inhibit(false) + }); } } diff --git a/scripts/test.sh b/scripts/test.sh index 25d1a40..83e3bb8 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -1,4 +1,4 @@ -#!/usr/bin/sh +#! /usr/bin/sh file="/.flatpak-info" if [ -f "$file" ]