GNOME Bugzilla – Bug 733747
EvPageCache should notify when a page gets cached
Last modified: 2014-09-11 12:13:38 UTC
Right now there is a procedure to ask to load a page on the cache, but the procedure is right now asynchronous. Several of the accessible elements depends on the contents of the cache. But right now you only can ask to load a specific page, but you don't know when that will finish. This leads to having several methods trying to initialize the internal children, hoping that at least one of those calls will be used when the cache is loaded. It would be better to have a signal that notifies when the cache gets the data of a specific page.
Created attachment 281708 [details] [review] Adds new signal EvPageCache::page-cached Is basically a forward of the EvJobPageData::finished, including the page number
Created attachment 281709 [details] [review] Use page cached signal to initialize children Now EvPageAccessible tries to initialize the children since the beginning, and if not, it connects to the cache, in order to do that when the page gets cached.
Ping: probably this bug and patches were overlooked due the flood of patches during the evince hackfest.
Review of attachment 281708 [details] [review]: LGTM ::: libview/ev-page-cache.c @@ +201,3 @@ + G_SIGNAL_RUN_LAST, + 0, + (GSignalAccumulator) NULL, NULL, I don't think you need the cast
Review of attachment 281709 [details] [review]: Ok
(In reply to comment #4) > Review of attachment 281708 [details] [review]: > > LGTM > > ::: libview/ev-page-cache.c > @@ +201,3 @@ > + G_SIGNAL_RUN_LAST, > + 0, > + (GSignalAccumulator) NULL, NULL, > > I don't think you need the cast Just pushed the patch removing this cast. So as I also pushed the other patch, closing bug.
Review of attachment 281708 [details] [review]: I think there is a typo in this patch. see below ::: libview/ev-page-cache.c @@ +330,1 @@ } Here you are passing the page index as the signal deatail. I think it should be: g_signal_emit (cache, ev_page_cache_signals[PAGE_CACHED], 0, job_data->page); no?
(In reply to comment #7) > Review of attachment 281708 [details] [review]: > > I think there is a typo in this patch. see below > > ::: libview/ev-page-cache.c > @@ +330,1 @@ > } > > Here you are passing the page index as the signal deatail. > I think it should be: > g_signal_emit (cache, ev_page_cache_signals[PAGE_CACHED], 0, job_data->page); > > no? Yes.
Fixed and pushed to master