Don't use LocalDataBaseManager as a singleton
ClosedPublic

Authored by dvratil on Apr 25 2017, 10:03 AM.

Details

Reviewers
mlaurent
Summary

Instead make LocalDatabaseManager an instance-based interface for a singleton backend. The reason is that when running Kontact, both Akregator, KMail and possibly others who use WebEngineViewer would share the same instance of LocalDataBaseManager and connect to its signal. This means that when you click on a link in KMail, KMail requests URL check from LocalDataBaseManager singleton and the singleton emits a signal, the signal is delivered to all applications that are connected to it, not just KMail. This results in each application opening the link. Currently you can reproduce when you enable phishing checks in both KMail and Akregator, launch Kontact, open KMail, then switch to Akregator and click any link. The link will get opened twice, once by KMail and once by Akregator.

This patch removes the singleton from LocalDataBaseManager and makes each WebEngineView own its own instance of LocalDataBaseManager. Internally, the actual implementation is still a singleton, however the public interface is non-singleton, which means that each application is connected to its own LocalDataBaseManager instance, thus avoiding the issue described above.

Test Plan

Ported Akregator to the new API, links no longer open twice.

Diff Detail

Repository
R94 PIM: Message Library
Lint
Lint Skipped
Unit
Unit Tests Skipped
dvratil created this revision.Apr 25 2017, 10:03 AM
Restricted Application added a subscriber: KDE PIM. · View Herald TranscriptApr 25 2017, 10:03 AM
mlaurent requested changes to this revision.Apr 26 2017, 5:10 AM

You need to adapt akregator too.

webengineviewer/src/checkphishingurl/localdatabasemanager.h
50 ↗(On Diff #13773)

remove extra space here

This revision now requires changes to proceed.Apr 26 2017, 5:10 AM
dvratil updated this revision to Diff 13869.Apr 27 2017, 12:42 PM
dvratil edited edge metadata.
mlaurent accepted this revision.Apr 27 2017, 7:01 PM

Seems ok.
Thanks

This revision is now accepted and ready to land.Apr 27 2017, 7:01 PM