BookmarksRunner: Change caching databases

Authored by alex on Wed, May 13, 4:51 PM.



The copying/updating of the cache has been removed from the FetchSqlite class.
This allows the browsers to have handle the caching differently.

The firefox sqlite files are only copied if they changed. Before they were copied
for each match session.

The chrome favicon database is also only copied if needed
and the check is also made in the prepare method and not just when
the profile is initialized.

Test Plan

For both firefox and chrome:
Plugin shows icons for results.
Add new bookmarks, close browser, the new bookmarks should show up with icons.

Diff Detail

R120 Plasma Workspace
avid_unnecessary_copying (branched from master)
No Linters Available
No Unit Test Coverage
Build Status
Buildable 26958
Build 26976: arc lint + arc unit
alex created this revision.Wed, May 13, 4:51 PM
Restricted Application added a project: Plasma. · View Herald TranscriptWed, May 13, 4:51 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
alex requested review of this revision.Wed, May 13, 4:51 PM
alex updated this revision to Diff 82789.Wed, May 13, 7:44 PM
alex retitled this revision from WIP BookmarksRunner: Change caching databases to BookmarksRunner: Change caching databases.
alex edited the summary of this revision. (Show Details)
alex edited the test plan for this revision. (Show Details)

Copy chrome files only if they changed

alex updated this revision to Diff 82790.Wed, May 13, 7:48 PM

Add space

meven added inline comments.Thu, May 14, 6:23 AM

Might as well check the returned value for error


Maybe add a if block


Can be removed as well


We might want to reuse database connection as long as the databaseFile did not change

alex added inline comments.Fri, May 15, 2:02 PM

I will do that in a follow up patch :-)

alex added inline comments.Fri, May 15, 2:29 PM

If this fails only the icons are affected and the default icon will be displayed, this case will be handled in the FetchSqlite class


I would rather do that in another patch.

alex updated this revision to Diff 82946.Fri, May 15, 2:32 PM

Use enum for caching return type

meven accepted this revision.Fri, May 15, 3:04 PM

Seems good to me

This revision is now accepted and ready to land.Fri, May 15, 3:04 PM
alex updated this revision to Diff 82954.Fri, May 15, 3:43 PM

Adjust docstring for Browser::updateCacheFile

alex closed this revision.Sat, May 23, 9:40 PM