Blob Blame History Raw
From e7f496a155da98fe6321b157b83c878001ea8fb8 Mon Sep 17 00:00:00 2001
From: Debarshi Ray <debarshir@gnome.org>
Date: Thu, 7 Dec 2017 20:11:39 +0100
Subject: [PATCH 1/7] application: Insert the getting started PDF only when
 showing a window

The startup is shared between the application and the search provider.
There is no need to initialize the getting started PDF for the search
provider, since it is only useful when the user runs the application.

https://bugzilla.gnome.org/show_bug.cgi?id=791518
---
 src/application.js | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/application.js b/src/application.js
index 40506d48627b..73cb01767240 100644
--- a/src/application.js
+++ b/src/application.js
@@ -488,9 +488,6 @@ var Application = new Lang.Class({
               accels: ['<Primary>f'] }
         ];
 
-        if (!this.isBooks)
-            this._initGettingStarted();
-
         Utils.populateActionGroup(this, this._actionEntries, 'app');
     },
 
@@ -498,6 +495,9 @@ var Application = new Lang.Class({
         if (this._mainWindow)
             return;
 
+        if (!this.isBooks)
+            this._initGettingStarted();
+
         notificationManager = new Notifications.NotificationManager();
         this._connectActionsToMode();
         this._mainWindow = new MainWindow.MainWindow(this);
-- 
2.14.3


From c98d19eaf7aeb20eea90d93ba1a0c9c112a2a3e7 Mon Sep 17 00:00:00 2001
From: Debarshi Ray <debarshir@gnome.org>
Date: Thu, 7 Dec 2017 20:53:10 +0100
Subject: [PATCH 2/7] application: Make window creation complete asynchronously

Creating MainWindow starts the TrackerControllers. The getting started
PDF needs to be located before that happens, to let the
TrackerControllers include it in their queries. Since, locating the PDF
is an asynchronous operation, the overall _createWindow method also
needs to complete asynchronously.

This doesn't make any user-visible changes, but sets the stage for the
subsequent patches.

https://bugzilla.gnome.org/show_bug.cgi?id=791518
---
 src/application.js | 81 ++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 54 insertions(+), 27 deletions(-)

diff --git a/src/application.js b/src/application.js
index 73cb01767240..b80f22ddd03a 100644
--- a/src/application.js
+++ b/src/application.js
@@ -491,9 +491,16 @@ var Application = new Lang.Class({
         Utils.populateActionGroup(this, this._actionEntries, 'app');
     },
 
-    _createWindow: function() {
-        if (this._mainWindow)
+    _createWindow: function(callback) {
+        if (this._mainWindow) {
+            Mainloop.idle_add(Lang.bind(this,
+                function() {
+                    callback();
+                    return GLib.SOURCE_REMOVE;
+                }));
+
             return;
+        }
 
         if (!this.isBooks)
             this._initGettingStarted();
@@ -512,6 +519,20 @@ var Application = new Lang.Class({
 
         // start miners
         this._startMiners();
+
+        Mainloop.idle_add(Lang.bind(this,
+            function() {
+                callback();
+                return GLib.SOURCE_REMOVE;
+            }));
+    },
+
+    _presentWindow: function() {
+        if (!this._mainWindow)
+            throw(new Error('this._mainWindow == null'));
+
+        this._mainWindow.present_with_time(this._activationTimestamp);
+        this._activationTimestamp = Gdk.CURRENT_TIME;
     },
 
     vfunc_dbus_register: function(connection, path) {
@@ -554,12 +575,14 @@ var Application = new Lang.Class({
 
     vfunc_activate: function() {
         if (!this._mainWindow) {
-            this._createWindow();
-            modeController.setWindowMode(WindowMode.WindowMode.DOCUMENTS);
+            this._createWindow(Lang.bind(this,
+                function() {
+                    modeController.setWindowMode(WindowMode.WindowMode.DOCUMENTS);
+                    this._presentWindow();
+                }));
+        } else {
+            this._presentWindow();
         }
-
-        this._mainWindow.present_with_time(this._activationTimestamp);
-        this._activationTimestamp = Gdk.CURRENT_TIME;
     },
 
     _clearState: function() {
@@ -598,22 +621,24 @@ var Application = new Lang.Class({
     },
 
     _onActivateResult: function(provider, urn, terms, timestamp) {
-        this._createWindow();
-        modeController.setWindowMode(WindowMode.WindowMode.PREVIEW_EV);
-
-        let doc = documentManager.getItemById(urn);
-        if (doc) {
-            doActivate.apply(this, [doc]);
-        } else {
-            let job = new TrackerUtils.SingleItemJob(urn, queryBuilder);
-            job.run(Query.QueryFlags.UNFILTERED, Lang.bind(this,
-                function(cursor) {
-                    if (cursor)
-                        doc = documentManager.addDocumentFromCursor(cursor);
+        this._createWindow(Lang.bind(this,
+            function() {
+                modeController.setWindowMode(WindowMode.WindowMode.PREVIEW_EV);
 
+                let doc = documentManager.getItemById(urn);
+                if (doc) {
                     doActivate.apply(this, [doc]);
-                }));
-        }
+                } else {
+                    let job = new TrackerUtils.SingleItemJob(urn, queryBuilder);
+                    job.run(Query.QueryFlags.UNFILTERED, Lang.bind(this,
+                        function(cursor) {
+                            if (cursor)
+                                doc = documentManager.addDocumentFromCursor(cursor);
+
+                            doActivate.apply(this, [doc]);
+                        }));
+                }
+            }));
 
         function doActivate(doc) {
             documentManager.setActiveItem(doc);
@@ -637,13 +662,15 @@ var Application = new Lang.Class({
     },
 
     _onLaunchSearch: function(provider, terms, timestamp) {
-        this._createWindow();
-        modeController.setWindowMode(WindowMode.WindowMode.DOCUMENTS);
-        searchController.setString(terms.join(' '));
-        this.change_action_state('search', GLib.Variant.new('b', true));
+        this._createWindow(Lang.bind(this,
+            function() {
+                modeController.setWindowMode(WindowMode.WindowMode.DOCUMENTS);
+                searchController.setString(terms.join(' '));
+                this.change_action_state('search', GLib.Variant.new('b', true));
 
-        this._activationTimestamp = timestamp;
-        this.activate();
+                this._activationTimestamp = timestamp;
+                this.activate();
+            }));
     },
 
     getScaleFactor: function() {
-- 
2.14.3


From 935d00626f3a3481e9ae13c0eadb146c10d9625a Mon Sep 17 00:00:00 2001
From: Debarshi Ray <debarshir@gnome.org>
Date: Fri, 8 Dec 2017 17:13:52 +0100
Subject: [PATCH 3/7] application: Tie the catch block more tightly to the
 failure site

Having a lot of code inside a try statement makes it harder to follow
the error handling logic. The catch statements are far away from the
code that can throw an exception, so it is difficult to match them
together. Let's reduce the size of the try blocks by only using them
for fallible operations.

https://bugzilla.gnome.org/show_bug.cgi?id=791518
---
 src/application.js | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/src/application.js b/src/application.js
index b80f22ddd03a..8f9b6276ac0a 100644
--- a/src/application.js
+++ b/src/application.js
@@ -149,19 +149,21 @@ var Application = new Lang.Class({
                 function(object, res) {
                     try {
                         let info = object.query_info_finish(res);
-                        this.gettingStartedLocation = file.get_parent();
-
-                        manager.index_file_async(file, null,
-                            function(object, res) {
-                                try {
-                                    manager.index_file_finish(res);
-                                } catch (e) {
-                                    log('Error indexing the getting started PDF: ' + e.message);
-                                }
-                            });
                     } catch (e) {
                         checkNextFile.apply(this);
+                        return;
                     }
+
+                    this.gettingStartedLocation = file.get_parent();
+
+                    manager.index_file_async(file, null,
+                        function(object, res) {
+                            try {
+                                manager.index_file_finish(res);
+                            } catch (e) {
+                                log('Error indexing the getting started PDF: ' + e.message);
+                            }
+                        });
                 }));
         }
 
-- 
2.14.3


From 2362a9b4b6e518ccc04d6d6c6d16f837e3df038d Mon Sep 17 00:00:00 2001
From: Debarshi Ray <debarshir@gnome.org>
Date: Fri, 8 Dec 2017 17:29:56 +0100
Subject: [PATCH 4/7] application: Shuffle some code around

Having the _initGettingStarted method take care of the isBooks
condition will make it easier to chain it with the window creation.

https://bugzilla.gnome.org/show_bug.cgi?id=791518
---
 src/application.js | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/application.js b/src/application.js
index 8f9b6276ac0a..81ae382770df 100644
--- a/src/application.js
+++ b/src/application.js
@@ -127,6 +127,9 @@ var Application = new Lang.Class({
     },
 
     _initGettingStarted: function() {
+        if (this.isBooks)
+            return;
+
         let manager = TrackerControl.MinerManager.new_full(false);
 
         let languages = GLib.get_language_names();
@@ -504,8 +507,7 @@ var Application = new Lang.Class({
             return;
         }
 
-        if (!this.isBooks)
-            this._initGettingStarted();
+        this._initGettingStarted();
 
         notificationManager = new Notifications.NotificationManager();
         this._connectActionsToMode();
-- 
2.14.3


From 4f5d1073d2049422b36c4b809a3f2ae6e8db3f4c Mon Sep 17 00:00:00 2001
From: Debarshi Ray <debarshir@gnome.org>
Date: Fri, 8 Dec 2017 17:36:49 +0100
Subject: [PATCH 5/7] application: Create the window only after detecting the
 PDF

This ensures that the TrackerControllers are started after the getting
started PDF has been initialized, so that it can be included in their
queries.

https://bugzilla.gnome.org/show_bug.cgi?id=791518
---
 src/application.js | 55 +++++++++++++++++++++++++++++++++---------------------
 1 file changed, 34 insertions(+), 21 deletions(-)

diff --git a/src/application.js b/src/application.js
index 81ae382770df..3c7110323dc3 100644
--- a/src/application.js
+++ b/src/application.js
@@ -126,9 +126,16 @@ var Application = new Lang.Class({
                              _("Show the version of the program"), null);
     },
 
-    _initGettingStarted: function() {
-        if (this.isBooks)
+    _initGettingStarted: function(callback) {
+        if (this.isBooks) {
+            Mainloop.idle_add(Lang.bind(this,
+                function() {
+                    callback();
+                    return GLib.SOURCE_REMOVE;
+                }));
+
             return;
+        }
 
         let manager = TrackerControl.MinerManager.new_full(false);
 
@@ -145,6 +152,7 @@ var Application = new Lang.Class({
             let file = files.shift();
             if (!file) {
                 log('Can\'t find a valid getting started PDF document');
+                callback();
                 return;
             }
 
@@ -166,11 +174,17 @@ var Application = new Lang.Class({
                             } catch (e) {
                                 log('Error indexing the getting started PDF: ' + e.message);
                             }
+
+                            callback();
                         });
                 }));
         }
 
-        checkNextFile.apply(this);
+        Mainloop.idle_add(Lang.bind(this,
+            function() {
+                checkNextFile.apply(this);
+                return GLib.SOURCE_REMOVE;
+            }));
     },
 
     _fullscreenCreateHook: function(action) {
@@ -507,27 +521,26 @@ var Application = new Lang.Class({
             return;
         }
 
-        this._initGettingStarted();
-
-        notificationManager = new Notifications.NotificationManager();
-        this._connectActionsToMode();
-        this._mainWindow = new MainWindow.MainWindow(this);
-        this._mainWindow.connect('destroy', Lang.bind(this, this._onWindowDestroy));
-
-        try {
-            this._extractPriority = TrackerExtractPriority();
-            this._extractPriority.SetRdfTypesRemote(['nfo:Document']);
-        } catch (e) {
-            log('Unable to connect to the tracker extractor: ' + e.toString());
-        }
+        this.hold();
+        this._initGettingStarted(Lang.bind(this,
+            function() {
+                this.release();
+                notificationManager = new Notifications.NotificationManager();
+                this._connectActionsToMode();
+                this._mainWindow = new MainWindow.MainWindow(this);
+                this._mainWindow.connect('destroy', Lang.bind(this, this._onWindowDestroy));
+
+                try {
+                    this._extractPriority = TrackerExtractPriority();
+                    this._extractPriority.SetRdfTypesRemote(['nfo:Document']);
+                } catch (e) {
+                    log('Unable to connect to the tracker extractor: ' + e.toString());
+                }
 
-        // start miners
-        this._startMiners();
+                // start miners
+                this._startMiners();
 
-        Mainloop.idle_add(Lang.bind(this,
-            function() {
                 callback();
-                return GLib.SOURCE_REMOVE;
             }));
     },
 
-- 
2.14.3


From 95a69188445605be846adb94054a792c09b5e997 Mon Sep 17 00:00:00 2001
From: Debarshi Ray <debarshir@gnome.org>
Date: Tue, 12 Dec 2017 16:35:22 +0100
Subject: [PATCH 6/7] trackerController: Don't submit queries until started

https://bugzilla.gnome.org/show_bug.cgi?id=791518
---
 src/trackerController.js | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/trackerController.js b/src/trackerController.js
index 05b7e89f1586..55f1343f1417 100644
--- a/src/trackerController.js
+++ b/src/trackerController.js
@@ -283,6 +283,10 @@ const TrackerController = new Lang.Class({
             return;
 
         this.sortBy = sortBy;
+
+        if (!this._isStarted)
+            return;
+
         this._refreshInternal(RefreshFlags.RESET_OFFSET);
     },
 
-- 
2.14.3


From 9e57889aeaa9d56ca61059012cccc37a9145f92c Mon Sep 17 00:00:00 2001
From: Debarshi Ray <debarshir@gnome.org>
Date: Fri, 8 Dec 2017 20:47:17 +0100
Subject: [PATCH 7/7] trackerController: Assert against premature queries

The TrackerControllers are only supposed to issue their queries once
all the pieces of the application that are necessary to formulate them
have been initialized. This is indicated by the invocation of the start
method. For example, if the queries are submitted before the detection
of the getting started PDF has completed, then they won't have the
path to PDF and it will be missing from the UI.

Throwing an exception forces one to think about the details surrounding
each query submission path, as opposed to having the silently dropping
the request.

https://bugzilla.gnome.org/show_bug.cgi?id=791518
---
 src/trackerController.js | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/trackerController.js b/src/trackerController.js
index 55f1343f1417..e45f3a7fc5c3 100644
--- a/src/trackerController.js
+++ b/src/trackerController.js
@@ -234,7 +234,8 @@ const TrackerController = new Lang.Class({
     },
 
     _refreshInternal: function(flags) {
-        this._isStarted = true;
+        if (!this._isStarted)
+            throw(new Error('!this._isStarted'));
 
         if (flags & RefreshFlags.RESET_OFFSET)
             this._offsetController.resetOffset();
@@ -294,6 +295,7 @@ const TrackerController = new Lang.Class({
         if (this._isStarted)
             return;
 
+        this._isStarted = true;
         this._refreshInternal(RefreshFlags.NONE);
     }
 });
-- 
2.14.3