GNOME Bugzilla – Bug 791518
Initialize the getting started PDF only when presenting a UI, and before any SPARQL has been submitted
Last modified: 2018-06-15 05:04:33 UTC
There are some problems with our handling of the getting started PDF. The code to detect and index it is shared with the search provider, which means that it runs on every gnome-shell search. This is a bit wasteful. We should try to keep the search providers as lean as possible. I think that we should treat it like the online content. ie., we need not have to guarantee knowing about it until someone has invoked the application at least once. Secondly, we should finish initializing the getting started PDF before submitting any SPARQL for the application. Otherwise, the queries won't include the PDF's path and will be missing. For example, if you can get the timing just right on an empty system, the TrackerControllers issue the queries just before the PDF is initialized, but the following queries to update the OffsetControllers' happens right after. This leads to an empty ViewContainer, but the no-results page isn't shown because the OffsetController isn't empty. Subsequent CHANGED events from TrackerChangeMonitor are ignored because the document isn't already in the DocumentManager.
Created attachment 365437 [details] [review] application: Insert the getting started PDF only when showing a window
Created attachment 365438 [details] [review] application: Make window creation complete asynchronously
Created attachment 365439 [details] [review] application: Tie the catch block more tightly to the failure site
Created attachment 365440 [details] [review] application: Shuffle some code around
Created attachment 365441 [details] [review] application: Create the window only after detecting the PDF
Created attachment 365442 [details] [review] trackerController: Don't submit queries until started
Created attachment 365443 [details] [review] trackerController: Assert against premature queries
These patches are also in gnome-documents:wip/rishi/init-getting-started.
Review of attachment 365437 [details] [review]: Makes sense, thanks.
Review of attachment 365438 [details] [review]: I'm not sure this is strictly needed... It should be fine to synchronously locate the getting started document during app startup, before the tracker controllers are started, no?
Review of attachment 365439 [details] [review]: Looks good.
Review of attachment 365440 [details] [review]: OK
Review of attachment 365441 [details] [review]: Same comment -- I'd like to keep the startup initialization sequence synchronous if possible.
Review of attachment 365442 [details] [review]: OK
Review of attachment 365443 [details] [review]: Looks good.
Created attachment 372579 [details] [review] application: Tie the catch block more tightly to the failure site Rebased and pushed to master.
Created attachment 372580 [details] [review] application: Synchronously initialize the getting started PDF
Created attachment 372581 [details] [review] trackerController: Don't submit queries until started
Created attachment 372582 [details] [review] trackerController: Assert against premature queries
Created attachment 372583 [details] [review] application: Insert the getting started PDF only when showing a window
Comment on attachment 372581 [details] [review] trackerController: Don't submit queries until started Pushed to master.
Comment on attachment 372582 [details] [review] trackerController: Assert against premature queries Pushed to master.
I pushed the remaining patches to master so that I don't forget about them. They seem to work well in my testing, and I think I addressed all the issues pointed out during the review; but please let me know if something is off.
Thanks! I took a quick look at the patches and they all look good to me.