Before: A pointer to the favicon gets written in the BookmarkMatch and using this pointer the icon gets created when the Plasma::QueryMatch gets created.
Now: Using the favicon pointer the icon gets created and written to the BookmarkMatch, when the QueryMatch is created the icon value is read.
This makes the code easier to understand and the BookmarkMatch is independent of the Favicon class.
Should compile and display icons.
| No Linters Available |
| No Unit Test Coverage |
| Buildable 24473 | |
| Build 24491: arc lint + arc unit |
Sorry, I wasn't aware about that and haven't found anything when I had a quick look at the docs before submitting this patch.
But if I may ask: Why is this now not thread save but was before?
Before this patch the icon was created in the BookmarkMatch::asQueryMatch method which was called from the secondary thread.
And with this patch the icon gets created in the Browser::match method which gets called from the same thread.
I think this is related to https://phabricator.kde.org/D5889?id=14601 ?
I think we are save here, as the QIcon is created from an image stored on disk (absolute path to e.g. PNG), not from a theme.
Thanks for the response, I was not sure if QIcon being not thread safe was meant in general or only for QIcon::fromTheme.
PS: When I said docs I meant the QIcon class docs, but thanks 😃
@davidedmundson - is QIcon(QImage(...)) save?
the AppstreamRunner suffers from the same problem, it uses match->setIcon(/*wrapped*/QIcon::fromTheme(<name>)).
@davidedmundson - is QIcon(QImage(...)) save?
For the way it's used in the runner, yes.
Before this patch the icon was created in the BookmarkMatch::asQueryMatch method which was called from the secondary thread.
Then my comment is happily refuted. I read the title and wanted to provide context for why it might have been like that making an assumption.