Merge lp:~jeremywootten/pantheon-files/further-armour-restore-tabs-function into lp:~elementary-apps/pantheon-files/trunk

[Jeremy Wootten]

Perform more checks when restoring tabs

This branch adds some further checks during restoring tabs.

[lp:~jeremywootten/pantheon-files/further-armour-restore-tabs-function]

1632. By Jeremy Wootten on 2014-11-09

Stop unnecessary rapid destruction and creation of slots on change tab; Slow down rate of creation of tabs during restore tabs - both likely to cause crashes due to race conditions

[Jeremy Wootten (jeremywootten)]

A further possible cause of the reported crashes on start up with restore tabs has been addressed.
Note that these issues should already be resolved in the the unmerged branch lp:~jeremywootten/pantheon-files/all-views-vala.

[Cody Garver (codygarver)]

Let's see if we can get more feedback on the bug report(s) once this update goes out

review: Approve

=== modified file 'src/View/ViewContainer.vala'
--- src/View/ViewContainer.vala	2014-10-29 13:59:32 +0000
+++ src/View/ViewContainer.vala	2014-11-09 18:03:53 +0000
@@ -32,7 +32,7 @@
         public GOF.Window.Slot? slot = null;
         public Marlin.Window.Columns? mwcol = null;
         Browser browser;
-        public int view_mode = 0;
+        public int view_mode = -1;
         public OverlayBar overlay_statusbar;
 
         //private ulong file_info_callback;
@@ -48,15 +48,13 @@
         public ViewContainer (Marlin.View.Window win, GLib.File location, int _view_mode = 0) {
             window = win;
             overlay_statusbar = new OverlayBar (win, this);
-            view_mode = _view_mode;
 
             /* set active tab */
             browser = new Browser ();
             label = _("Loading…");
             window.button_back.fetcher = get_back_menu;
             window.button_forward.fetcher = get_forward_menu;
-
-            change_view (view_mode, location);
+            change_view (_view_mode, location);
             update_location_state (true);
 
             this.show_all ();
@@ -203,14 +201,16 @@
             bool user_change_rq = location == null;
             select_childs = null;
 
+            GOF.Window.Slot? active_slot = get_active_slot ();
+
             if (location == null) {
                 /* we re just changing view keep the same location */
-                GOF.Window.Slot? active_slot = get_active_slot ();
                 if (active_slot == null) {
                     warning ("No active slot found - cannot change view");
                     return;
                 }
                 location = active_slot.location;
+
                 /* store the old selection to restore it */
                 if (slot != null && !content_shown) {
                     unowned List<GOF.File> list = ((FM.Directory.View) slot.view_box).get_selection ();
@@ -233,12 +233,17 @@
             Marlin.Window.Columns new_mwcol;
             GOF.Window.Slot new_slot;
 
-            if (nview == ViewMode.MILLER) {
-                new_mwcol = new Marlin.Window.Columns (location, this);
-                new_slot = new_mwcol.active_slot;
+            if (nview != view_mode || location != active_slot.location) {
+                if (nview == ViewMode.MILLER) {
+                    new_mwcol = new Marlin.Window.Columns (location, this);
+                    new_slot = new_mwcol.active_slot;
+                } else {
+                    new_mwcol = null;
+                    new_slot = new GOF.Window.Slot (location, this);
+                }
             } else {
-                new_mwcol = null;
-                new_slot = new GOF.Window.Slot (location, this);
+                new_mwcol = mwcol;
+                new_slot = slot;
             }
 
             /* automagicly enable icon view for icons keypath */
@@ -281,11 +286,6 @@
             slot = new_slot;
             mwcol = new_mwcol;
 
-            /* Setting up view_mode and its button */
-            view_mode = nview;
-            if (window.top_menu.view_switcher != null)
-                window.top_menu.view_switcher.mode = (ViewMode) view_mode;
-
             connect_available_info ();
             if (slot != null) {
                 slot.directory.done_loading.connect (directory_done_loading);
@@ -293,6 +293,9 @@
             }
             plugin_directory_loaded ();
 
+            /* Setting up view_mode and its button */
+            view_mode = nview;
+
             switch (nview) {
             case ViewMode.LIST:
                 slot.make_list_view ();
@@ -305,6 +308,9 @@
                 break;
             }
 
+            if (window.top_menu.view_switcher != null)
+                window.top_menu.view_switcher.mode = (ViewMode) view_mode;
+
             overlay_statusbar.showbar = nview != ViewMode.LIST;
         }
 
@@ -334,20 +340,22 @@
                 return slot;
         }
 
-        public string? get_root_uri () {
+        public string get_root_uri () {
+            string path = "";
             if (mwcol != null)
-                return mwcol.get_root_uri ();
+                path = mwcol.get_root_uri () ?? "";
             else if (slot != null)
-                return slot.location.get_uri ();
+                path = slot.location.get_uri () ?? "";
 
-            return null;
+            return path;
         }
 
-        public string? get_tip_uri () {
+        public string get_tip_uri () {
+            string path = "";
             if (mwcol != null)
-                return mwcol.get_tip_uri ();
-            else
-                return "";
+                path = mwcol.get_tip_uri () ?? "";
+
+            return path;
         }
 
         public void reload () {
@@ -355,7 +363,7 @@
             dir.cancel ();
             dir.need_reload.disconnect (reload);
             dir.remove_dir_from_cache ();
-            change_view (view_mode, null);
+            dir.load ();
         }
 
         public void update_location_state (bool save_history) {

=== modified file 'src/View/Window.vala'
--- src/View/Window.vala	2014-11-02 10:08:26 +0000
+++ src/View/Window.vala	2014-11-09 18:03:53 +0000
@@ -342,6 +342,7 @@
         public void change_tab (int offset) {
             ViewContainer? old_tab = current_tab;
             current_tab = (tabs.get_tab_by_index (offset)).page as ViewContainer;
+
             if (current_tab == null || old_tab == current_tab)
                 return;
 
@@ -523,8 +524,8 @@
             GLib.VariantIter iter = new GLib.VariantIter (tab_info_array);
             int tabs_added = 0;
             int viewmode = -1;
-            string root_uri = null;
-            string tip_uri = null;
+            string? root_uri = null;
+            string? tip_uri = null;
 
             /* inhibit unnecessary changes of view and rendering of location bar while restoring tabs
              * as this causes all sorts of problems */
@@ -552,20 +553,25 @@
                 viewmode = -1;
                 root_uri = null;
                 tip_uri = null;
+
+                /* Prevent too rapid loading of tabs which can cause crashes */
+                Thread.usleep (100000);
             }
 
             freeze_view_changes = false;
 
+            if (tabs_added < 1)
+                return 0;
+
             int active_tab_position = Preferences.settings.get_int ("active-tab-position");
-            if (active_tab_position >=0 && active_tab_position < tabs_added) {
-                tabs.current = tabs.get_tab_by_index (active_tab_position);
-                change_tab (active_tab_position);
-            }
-
-            string path;
-            if (current_tab == null)
-                path = "";
-            else {
+            if (active_tab_position < 0 || active_tab_position >= tabs_added)
+                active_tab_position = 0;
+
+            tabs.current = tabs.get_tab_by_index (active_tab_position);
+            change_tab (active_tab_position);
+
+            string path = "";
+            if (current_tab != null) {
                 path = current_tab.get_tip_uri ();
                 if (path == "")
                     path = current_tab.get_root_uri ();