D5889: Keep QIcon::fromTheme in main thread

[Rik Mills]

rikmills added a comment.


  This breaks build on Neon and Kubuntu CI with Qt 5.7.1
  
  
http://build.neon.kde.org/job/xenial_unstable_plasma_breeze_bin_amd64/120/console
  
  05:10:59 In file included from 
/workspace/build/build-qt4/kstyle/moc_breezeshadowhelper.cpp:9:0,
  05:10:59  from 
/workspace/build/build-qt4/kstyle/breeze_automoc.cpp:33:
  05:10:59 
/workspace/build/build-qt4/kstyle/../../kstyle/breezeshadowhelper.h:162:24: 
error: ‘KWayland’ was not declared in this scope
  05:10:59  QMap _widgetSurfaces;
  05:10:59 ^
  05:10:59 
/workspace/build/build-qt4/kstyle/../../kstyle/breezeshadowhelper.h:162:51: 
error: template argument 2 is invalid
  05:10:59  QMap _widgetSurfaces;

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D5889

To: davidedmundson, #plasma, graesslin
Cc: rikmills, graesslin, plasma-devel, #frameworks, ZrenBot, spstarr, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, 
lukas

D5889: Keep QIcon::fromTheme in main thread

[David Edmundson]

This revision was automatically updated to reflect the committed changes.
Closed by commit R127:92bf5a9828d1: Keep QIcon::fromTheme in main thread 
(authored by davidedmundson).

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5889?vs=14602=14611

REVISION DETAIL
  https://phabricator.kde.org/D5889

AFFECTED FILES
  src/client/plasmawindowmanagement.cpp

To: davidedmundson, #plasma, graesslin
Cc: graesslin, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, lukas

D5889: Keep QIcon::fromTheme in main thread

[Martin Flöser]

graesslin accepted this revision.
graesslin added a comment.
This revision is now accepted and ready to land.


  Eh what? That is not thread save?!? Oh sh*** I didn't expect that. Good catch 
and a nice solution.

REPOSITORY
  R127 KWayland

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D5889

To: davidedmundson, #plasma, graesslin
Cc: graesslin, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, lukas

D5889: Keep QIcon::fromTheme in main thread

[David Edmundson]

davidedmundson updated this revision to Diff 14602.
davidedmundson added a comment.
Restricted Application edited projects, added Plasma; removed Plasma on Wayland.


  simplify a line

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5889?vs=14601=14602

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D5889

AFFECTED FILES
  src/client/plasmawindowmanagement.cpp

To: davidedmundson, #plasma
Cc: plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, lukas

D5889: Keep QIcon::fromTheme in main thread

[David Edmundson]

davidedmundson created this revision.
Restricted Application added projects: Plasma on Wayland, Frameworks.
Restricted Application added subscribers: Frameworks, plasma-devel.

REVISION SUMMARY
  QIcon::fromTheme is not thread safe, we can't have it done in the future 
watcher.
  We can solve this by returning null in the watcher, and doing fromTheme in 
the main thread
  when the watcher finishes if we don't have an icon.
  
  (offtopic, I once made the obvious patch to Qt, but it was rejected with
  the response that QIcon isn't entirely thread safe, and that apparently
  meant none of it should be, which I don't fully agree with but whatever...)

TEST PLAN
  Current unit tests still pass

REPOSITORY
  R127 KWayland

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D5889

AFFECTED FILES
  src/client/plasmawindowmanagement.cpp

To: davidedmundson, #plasma
Cc: plasma-devel, #frameworks, ZrenBot, spstarr, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, hein, lukas