[fractal] Drop Data struct when exiting media viewer

[Daniel Garcia Moreno <danigm src gnome org>]

commit 4dfa5989f20b18a29054e0125246e7155dee95d9
Author: sonjita <sonjaleaheinze gmail com>
Date:   Wed Jan 22 16:13:24 2020 +0100

    Drop Data struct when exiting media viewer
    
    Before, when entering the media viewer, the MediaViewer struct was only
    temporary, whereas the Data struct was kept alive. But the Data struct
    was kept forever, even when exiting the media viewer. As a result,
    the last media widget, i.e. Image or VideoPlayerWidget, seen in the
    media viewer wasn't dropped either.
    
    With this commit, the MediaViewer struct is kept as long as the
    media viewer is open; the Data struct is kept with it. When the
    user navigates out of the media viewer, both of them get dropped.
    Therefore, the corresponding widget gets dropped, too.

 fractal-gtk/src/appop/media_viewer.rs    |  13 ++++
 fractal-gtk/src/appop/mod.rs             |   5 ++
 fractal-gtk/src/widgets/inline_player.rs |  20 ------
 fractal-gtk/src/widgets/media_viewer.rs  | 120 ++++++++++++++++++-------------
 4 files changed, 90 insertions(+), 68 deletions(-)
---
diff --git a/fractal-gtk/src/appop/media_viewer.rs b/fractal-gtk/src/appop/media_viewer.rs
index 69d03ac0..b92ebbee 100644
--- a/fractal-gtk/src/appop/media_viewer.rs
+++ b/fractal-gtk/src/appop/media_viewer.rs
@@ -1,6 +1,9 @@
 use gtk;
 use gtk::prelude::*;
 
+use std::cell::RefCell;
+use std::rc::Rc;
+
 use crate::appop::AppOp;
 use crate::appop::AppState;
 
@@ -53,6 +56,16 @@ impl AppOp {
 
             stack.add_named(&body, "media-viewer");
             stack_header.add_named(&header, "media-viewer");
+
+            let media_viewer_back_button = panel
+                .builder
+                .get_object::<gtk::Button>("media_viewer_back_button")
+                .expect("Can't find media_viewer_back_button in ui file.");
+            self.media_viewer = Rc::new(RefCell::new(Some(panel)));
+            let mv = self.media_viewer.clone();
+            media_viewer_back_button.connect_clicked(move |_| {
+                mv.borrow_mut().take();
+            });
         }
 
         self.set_state(AppState::MediaViewer);
diff --git a/fractal-gtk/src/appop/mod.rs b/fractal-gtk/src/appop/mod.rs
index 76f20f5a..6d6ec410 100644
--- a/fractal-gtk/src/appop/mod.rs
+++ b/fractal-gtk/src/appop/mod.rs
@@ -1,4 +1,6 @@
+use std::cell::RefCell;
 use std::collections::HashMap;
+use std::rc::Rc;
 use std::sync::mpsc::Sender;
 
 use fractal_api::r0::AccessToken;
@@ -72,6 +74,8 @@ pub struct AppOp {
     pub unsent_messages: HashMap<String, (String, i32)>,
     pub typing: HashMap<String, std::time::Instant>,
 
+    pub media_viewer: Rc<RefCell<Option<widgets::MediaViewer>>>,
+
     pub state: AppState,
     pub since: Option<String>,
 
@@ -110,6 +114,7 @@ impl AppOp {
             since: None,
             unsent_messages: HashMap::new(),
             typing: HashMap::new(),
+            media_viewer: Rc::new(RefCell::new(None)),
 
             md_enabled: false,
             invitation_roomid: None,
diff --git a/fractal-gtk/src/widgets/inline_player.rs b/fractal-gtk/src/widgets/inline_player.rs
index 9f4c9018..d8c379b9 100644
--- a/fractal-gtk/src/widgets/inline_player.rs
+++ b/fractal-gtk/src/widgets/inline_player.rs
@@ -283,26 +283,6 @@ impl VideoPlayerWidget {
         }
 
         let w = Rc::new(player_widget);
-        if with_controls {
-            // 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.controls.container`
-            // to the parent.
-            //
-            // So this callback keeps a refference to the Rust Struct
-            // so the compiler won't drop it which would cause to also drop
-            // the `gst_player`.
-            //
-            // When the widget is detached from it's parent which happens
-            // when we drop the room widget, this callback runs freeing
-            // the last refference we were holding.
-            let container = w.controls.clone().unwrap().container;
-            let foo = RefCell::new(Some(w.clone()));
-            container.connect_remove(move |_, _| {
-                foo.borrow_mut().take();
-            });
-        }
 
         /* The followign callback requires `Send` but is handled by the gtk main loop */
         let player_weak = Fragile::new(Rc::downgrade(&w));
diff --git a/fractal-gtk/src/widgets/media_viewer.rs b/fractal-gtk/src/widgets/media_viewer.rs
index 5cfde95b..6ecb2ea1 100644
--- a/fractal-gtk/src/widgets/media_viewer.rs
+++ b/fractal-gtk/src/widgets/media_viewer.rs
@@ -36,7 +36,7 @@ use std::sync::Mutex;
 pub struct MediaViewer {
     data: Rc<RefCell<Data>>,
     /* gtk widgets we need to have a reference to */
-    builder: gtk::Builder,
+    pub builder: gtk::Builder,
     backend: Sender<BKCommand>,
 }
 
@@ -412,6 +412,17 @@ impl Data {
     }
 }
 
+impl Drop for Data {
+    fn drop(&mut self) {
+        match &self.widget {
+            Widget::Video(player) => {
+                player.stop();
+            }
+            _ => {}
+        }
+    }
+}
+
 impl MediaViewer {
     pub fn new(
         backend: Sender<BKCommand>,
@@ -518,29 +529,33 @@ impl MediaViewer {
 
     /* connect media viewer headerbar */
     pub fn connect_media_viewer_headerbar(&self) {
-        let own = self.data.clone();
+        let own_weak = Rc::downgrade(&self.data);
         let full_screen_button = self
             .builder
             .get_object::<gtk::Button>("full_screen_button")
             .expect("Cant find full_screen_button in ui file.");
         full_screen_button.connect_clicked(move |_| {
-            let main_window = own.borrow().main_window.clone();
-            if let Some(win) = main_window.get_window() {
-                if !win.get_state().contains(gdk::WindowState::FULLSCREEN) {
-                    own.borrow_mut().enter_full_screen();
-                } else {
-                    own.borrow_mut().leave_full_screen()
+            own_weak.upgrade().map(|own| {
+                let main_window = own.borrow().main_window.clone();
+                if let Some(win) = main_window.get_window() {
+                    if !win.get_state().contains(gdk::WindowState::FULLSCREEN) {
+                        own.borrow_mut().enter_full_screen();
+                    } else {
+                        own.borrow_mut().leave_full_screen()
+                    }
                 }
-            }
+            });
         });
 
         let save_as_button = self
             .builder
             .get_object::<gtk::ModelButton>("save_as_button")
             .expect("Cant find save_as_button in ui file.");
-        let data = self.data.clone();
+        let data_weak = Rc::downgrade(&self.data);
         save_as_button.connect_clicked(move |_| {
-            data.borrow().save_media();
+            data_weak.upgrade().map(|data| {
+                data.borrow().save_media();
+            });
         });
     }
 
@@ -662,7 +677,7 @@ impl MediaViewer {
             Inhibit(false)
         });
 
-        let own = self.data.clone();
+        let own_weak = Rc::downgrade(&self.data);
         let builder = self.builder.clone();
         let backend = self.backend.clone();
         let previous_media_button = self
@@ -670,18 +685,22 @@ impl MediaViewer {
             .get_object::<gtk::Button>("previous_media_button")
             .expect("Cant find previous_media_button in ui file.");
         previous_media_button.connect_clicked(move |_| {
-            if !own.borrow_mut().previous_media() {
-                load_more_media(own.clone(), builder.clone(), backend.clone());
-            }
+            own_weak.upgrade().map(|own| {
+                if !own.borrow_mut().previous_media() {
+                    load_more_media(own.clone(), builder.clone(), backend.clone());
+                }
+            });
         });
 
-        let own = self.data.clone();
+        let own_weak = Rc::downgrade(&self.data);
         let next_media_button = self
             .builder
             .get_object::<gtk::Button>("next_media_button")
             .expect("Cant find next_media_button in ui file.");
         next_media_button.connect_clicked(move |_| {
-            own.borrow_mut().next_media();
+            own_weak.upgrade().map(|own| {
+                own.borrow_mut().next_media();
+            });
         });
         let full_screen_button = self
             .builder
@@ -719,13 +738,15 @@ impl MediaViewer {
         self.data.borrow_mut().signal_id = Some(id);
 
         // Remove the keyboard signal management on hide
-        let data = self.data.clone();
+        let data_weak = Rc::downgrade(&self.data);
         media_viewer_box.connect_unmap(move |_| {
-            let id = data.borrow_mut().signal_id.take();
-            let main_window = &data.borrow().main_window;
-            if let Some(id) = id {
-                signal::signal_handler_disconnect(main_window, id);
-            }
+            data_weak.upgrade().map(|data| {
+                let id = data.borrow_mut().signal_id.take();
+                let main_window = &data.borrow().main_window;
+                if let Some(id) = id {
+                    signal::signal_handler_disconnect(main_window, id);
+                }
+            });
         });
     }
 
@@ -799,41 +820,44 @@ fn load_more_media(data: Rc<RefCell<Data>>, builder: gtk::Builder, backend: Send
         .unwrap();
 
     let ui = builder.clone();
-    let data = data.clone();
+    let data_weak = Rc::downgrade(&data);
     gtk::timeout_add(50, move || match rx.try_recv() {
         Err(TryRecvError::Empty) => gtk::Continue(true),
         Err(TryRecvError::Disconnected) => {
-            data.borrow_mut().loading_error = true;
-            let err = i18n("Error while loading previous media");
-            ErrorDialog::new(false, &err);
+            data_weak.clone().upgrade().map(|data| {
+                data.borrow_mut().loading_error = true;
+                let err = i18n("Error while loading previous media");
+                ErrorDialog::new(false, &err);
+            });
 
             gtk::Continue(false)
         }
         Ok((msgs, prev_batch)) => {
             if msgs.len() == 0 {
-                data.borrow_mut().no_more_media = true;
+                data_weak.clone().upgrade().map(|data| {
+                    data.borrow_mut().no_more_media = true;
+                });
                 return gtk::Continue(false);
             }
-
-            let media_list = data.borrow().media_list.clone();
-            let img_msgs: Vec<Message> = msgs
-                .into_iter()
-                .filter(|msg| msg.mtype == "m.image" || msg.mtype == "m.video")
-                .collect();
-            let img_msgs_count = img_msgs.len();
-            let new_media_list: Vec<Message> =
-                img_msgs.into_iter().chain(media_list.into_iter()).collect();
-
-            data.borrow_mut().media_list = new_media_list;
-            data.borrow_mut().prev_batch = Some(prev_batch);
-
-            if img_msgs_count == 0 {
-                load_more_media(data.clone(), builder.clone(), backend.clone());
-            } else {
-                data.borrow_mut().current_media_index += img_msgs_count;
-                data.borrow_mut().previous_media();
-                data.borrow_mut().loading_more_media = loading_state(&ui, false);
-            }
+            data_weak.upgrade().map(|data| {
+                let media_list = data.borrow().media_list.clone();
+                let img_msgs: Vec<Message> = msgs
+                    .into_iter()
+                    .filter(|msg| msg.mtype == "m.image" || msg.mtype == "m.video")
+                    .collect();
+                let img_msgs_count = img_msgs.len();
+                let new_media_list: Vec<Message> =
+                    img_msgs.into_iter().chain(media_list.into_iter()).collect();
+                data.borrow_mut().media_list = new_media_list;
+                data.borrow_mut().prev_batch = Some(prev_batch);
+                if img_msgs_count == 0 {
+                    load_more_media(data.clone(), builder.clone(), backend.clone());
+                } else {
+                    data.borrow_mut().current_media_index += img_msgs_count;
+                    data.borrow_mut().previous_media();
+                    data.borrow_mut().loading_more_media = loading_state(&ui, false);
+                }
+            });
 
             gtk::Continue(false)
         }