GNOME Bugzilla – Bug 725393
The overview is empty at startup
Last modified: 2016-03-30 00:49:42 UTC
This sometimes happens due to a race condition. The overview is built using the frecent store, but if the history query hasn't been completed yet when the overview page is built, the frecent store is empty and the overview html has no items. We should get rid of the model entirely in the UI process, since we are already keeping a simpler model in the web process to update the DOM when it changes. And the UI process is already notifying the web process about history changes without using the frecent store.
Created attachment 270551 [details] [review] snapshot-service: Add a memory cache of snapshots paths saved And add a method to query that cache.
Created attachment 270552 [details] [review] snapshot-service: Update snapshots after one week
Created attachment 270553 [details] [review] snapshot-service: Add methods to get the snapshot path This is useful for the new overview that doesn't need the actual snapshot, but the path of the snapshot when it has been saved in the cache.
Created attachment 270554 [details] [review] overview: Remove the overview model from the UI process Now we query the history and snapshot services directly and the model is used only in the web process. This fixes a race condition when the overview page is created before the history query has completed leaving an empty overview.
Review of attachment 270552 [details] [review]: ::: lib/ephy-snapshot-service.c @@ +488,3 @@ /* Try to get the snapshot from the cache first if we have a URL */ uri = webkit_web_view_get_uri (web_view); + if (uri && current_time - mtime <= SNAPSHOT_UPDATE_THRESHOLD) IIRC we update the snapshots not only based on their age, but also on how frequently the page is visited. That way a page that's visited very often will be updated more frequently. This reduces that possibility.
(In reply to comment #5) > Review of attachment 270552 [details] [review]: > > ::: lib/ephy-snapshot-service.c > @@ +488,3 @@ > /* Try to get the snapshot from the cache first if we have a URL */ > uri = webkit_web_view_get_uri (web_view); > + if (uri && current_time - mtime <= SNAPSHOT_UPDATE_THRESHOLD) > > IIRC we update the snapshots not only based on their age, but also on how > frequently the page is visited. That way a page that's visited very often will > be updated more frequently. This reduces that possibility. Ah, it makes sense, I didn't notice that magic in the current code
(In reply to comment #6) > (In reply to comment #5) > > Review of attachment 270552 [details] [review] [details]: > > > > ::: lib/ephy-snapshot-service.c > > @@ +488,3 @@ > > /* Try to get the snapshot from the cache first if we have a URL */ > > uri = webkit_web_view_get_uri (web_view); > > + if (uri && current_time - mtime <= SNAPSHOT_UPDATE_THRESHOLD) > > > > IIRC we update the snapshots not only based on their age, but also on how > > frequently the page is visited. That way a page that's visited very often will > > be updated more frequently. This reduces that possibility. > > Ah, it makes sense, I didn't notice that magic in the current code Note that the time I'm passing there is the mtime stored in the history, not sure if that makes the trick, though
Review of attachment 270553 [details] [review]: ::: lib/ephy-snapshot-service.c @@ +705,3 @@ + } else { + g_simple_async_result_run_in_thread (result, + (GSimpleAsyncThreadFunc)get_snapshot_url_for_url_thread, get_snapshot_path_for_url_thread
Review of attachment 270554 [details] [review]: ::: embed/ephy-web-view.c @@ +1645,3 @@ + if (!ephy_web_view_is_history_frozen (view) && + ephy_embed_shell_get_mode (ephy_embed_shell_get_default ()) != EPHY_EMBED_SHELL_MODE_INCOGNITO) { The incognito bits might probably deserve a split commit.
Created attachment 270583 [details] [review] snapshot-service: Add a memory cache of snapshots paths saved And add a method to query that cache.
Created attachment 270584 [details] [review] snapshot-service: Update snapshots after one week
Created attachment 270585 [details] [review] snapshot-service: Add methods to get the snapshot path This is useful for the new overview that doesn't need the actual snapshot, but the path of the snapshot when it has been saved in the cache.
Created attachment 270586 [details] [review] history-service: Also return the thumbnail time by ephy_history_service_query_urls
Created attachment 270587 [details] [review] overview: Remove the overview model from the UI process Now we query the history and snapshot services directly and the model is used only in the web process. This fixes a race condition when the overview page is created before the history query has completed leaving an empty overview.
Review of attachment 270583 [details] [review]: Good
Review of attachment 270584 [details] [review]: We can do this.
Review of attachment 270585 [details] [review]: OK
Review of attachment 270586 [details] [review]: Good
Review of attachment 270587 [details] [review]: Looks good. ::: embed/web-extension/ephy-web-overview.c @@ +306,1 @@ + /* Check if the model was updated while the overview was loading */ Checks whether…