Fixup Chrome/Firefox Bookmarks KRunner
Closed, ResolvedPublic

Description

The Bookmarks KRunner has several issues:

  • BookmarksRunner::prep() duplicates the Signal/Slot connection each time the browser instance from the factory is reused - D15306
  • The temporary directory for favicons is never cleared - D15391

Specific to the Firefox implementation of the Browser interface:

  • The runner returns duplicate matches, typically one match with a title, and one or more matches without title - D15357
  • On each prepare call, a new FetchSqlite instance is created, the old one is leaked. This also causes some warnings: - D16409, D16410

QSqlDatabasePrivate::removeDatabase: connection '.../.mozilla/.../places.sqlite' is still in use, all queries will cease to work.
QSqlDatabasePrivate::addDatabase: duplicate connection name '.../.mozilla/.../places.sqlite', old connection removed.

  • On each prepare call, the creation of the FetchSqlite instance creates a temporary copy of the places.sqlite and favicon.sqlite databases. These are 20 MByte resp. 15 MByte.

Specific to the Chrome implementation of the Browser interface:

The resource leaks should be fixed.

The temporary copies of the Sqlite databases are very likely unnecessary, and may even result in bad results. Sqlite3 databases are multi-process - see https://www.sqlite.org/lockingv3.html - and the readonly access required by the runner is even less critical. On the other hand, just copying the main database file without accompanying write-ahead-log may give bad results (i.e. while the wal is flushed/checkpointed to the main db file).

Note, apparently Firefox once used Exclusive mode for the database files, but apparently no longer does. Exclusive mode would have prohibited reads of the db file.

bruns created this task.Sep 5 2018, 12:37 AM
bruns added a project: Plasma.
bruns updated the task description. (Show Details)Sep 5 2018, 10:00 PM
bruns updated the task description. (Show Details)Sep 7 2018, 9:27 PM
bruns claimed this task.Sep 7 2018, 9:36 PM
bruns triaged this task as Normal priority.
bruns moved this task from To Do to Work in Progress on the Plasma board.
bruns updated the task description. (Show Details)Sep 8 2018, 2:27 PM
bruns updated the task description. (Show Details)Sep 8 2018, 8:04 PM
bruns updated the task description. (Show Details)Sep 9 2018, 11:14 PM
bruns updated the task description. (Show Details)Sep 10 2018, 12:27 PM
bruns updated the task description. (Show Details)Sep 13 2018, 8:59 PM
bruns updated the task description. (Show Details)
bruns updated the task description. (Show Details)Sep 14 2018, 12:34 AM
bruns updated the task description. (Show Details)Oct 3 2018, 2:22 AM
bruns updated the task description. (Show Details)Oct 24 2018, 6:46 PM
bruns updated the task description. (Show Details)Nov 15 2018, 12:14 AM

Is this still a work in progress @bruns ?

bruns added a comment.Oct 30 2019, 5:53 PM

I am not currently working on this, but the state is correct (temporary copies of the DBs should be avoided).

alex closed this task as Resolved.Dec 21 2020, 11:05 AM
alex added a subscriber: alex.

state is correct (temporary copies of the DBs should be avoided).

Have fixed that already, now it is checked it is checked if the database has changed.

And the icons are better cached too, which will result in fewer database queries and disk write operations.