BookmarksRunner: Change caching databases
ClosedPublic

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

Details

Summary

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

Repository
R120 Plasma Workspace
Branch
avid_unnecessary_copying (branched from master)
Lint
No Linters Available
Unit
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
runners/bookmarks/browsers/chrome.cpp
55

Might as well check the returned value for error

runners/bookmarks/browsers/firefox.cpp
69–70

Maybe add a if block

runners/bookmarks/fetchsqlite.cpp
42

Can be removed as well

87–90

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
runners/bookmarks/fetchsqlite.cpp
87–90

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

alex added inline comments.Fri, May 15, 2:29 PM
runners/bookmarks/browsers/chrome.cpp
55

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

runners/bookmarks/fetchsqlite.cpp
42

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