D16594: Add context to kcmodule connection to lambdas

2018-11-05 Thread David Edmundson 
This revision was automatically updated to reflect the committed changes.
Closed by commit R295:2da8aed41663: Add context to kcmodule connection to 
lambdas (authored by davidedmundson).

REPOSITORY
  R295 KCMUtils

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16594?vs=44659=44915

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

AFFECTED FILES
  src/kcmoduleqml.cpp

To: davidedmundson, #plasma, broulik, bruns
Cc: bruns, kde-frameworks-devel, michaelh, ngraham

D16594: Add context to kcmodule connection to lambdas

2018-11-03 Thread Stefan Brüns 
bruns accepted this revision.
bruns added inline comments.

INLINE COMMENTS

> davidedmundson wrote in kcmoduleqml.cpp:74
> Technically yes, but there's no sane scope for a code path where configModule 
> is emitting anything between deleting d and this.

Thanks for the confirmation, hope nobody finds an insane one ;-)

REPOSITORY
  R295 KCMUtils

BRANCH
  master

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

To: davidedmundson, #plasma, broulik, bruns
Cc: bruns, kde-frameworks-devel, michaelh, ngraham

D16594: Add context to kcmodule connection to lambdas

2018-11-01 Thread David Edmundson 
davidedmundson added inline comments.

INLINE COMMENTS

> bruns wrote in kcmoduleqml.cpp:74
> `this` is desctructed after `d` has been deleted, so the signal may be still 
> connected after d is deleted.
> 
> Does `disconnect(configModule, 0, this, 0)` work, when called in the 
> destructor?

Technically yes, but there's no sane scope for a code path where configModule 
is emitting anything between deleting d and this.

REPOSITORY
  R295 KCMUtils

BRANCH
  master

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

To: davidedmundson, #plasma, broulik
Cc: bruns, kde-frameworks-devel, michaelh, ngraham

D16594: Add context to kcmodule connection to lambdas

2018-11-01 Thread Stefan Brüns 
bruns added inline comments.

INLINE COMMENTS

> kcmoduleqml.cpp:74
>  setButtons((KCModule::Buttons)(int)d->configModule->buttons());
> -connect(configModule, ::ConfigModule::buttonsChanged, [=] {
> +connect(configModule, ::ConfigModule::buttonsChanged, this, 
> [=] {
>  setButtons((KCModule::Buttons)(int)d->configModule->buttons());

`this` is desctructed after `d` has been deleted, so the signal may be still 
connected after d is deleted.

Does `disconnect(configModule, 0, this, 0)` work, when called in the destructor?

REPOSITORY
  R295 KCMUtils

BRANCH
  master

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

To: davidedmundson, #plasma, broulik
Cc: bruns, kde-frameworks-devel, michaelh, ngraham

D16594: Add context to kcmodule connection to lambdas

2018-11-01 Thread Kai Uwe Broulik 
broulik accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R295 KCMUtils

BRANCH
  master

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

To: davidedmundson, #plasma, broulik
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D16594: Add context to kcmodule connection to lambdas

2018-11-01 Thread David Edmundson 
davidedmundson created this revision.
davidedmundson added a reviewer: Plasma.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
davidedmundson requested review of this revision.

REVISION SUMMARY
  configModule outlives KCModuleQML.
  
  BUG: 397894

TEST PLAN
  Compiled

REPOSITORY
  R295 KCMUtils

BRANCH
  master

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

AFFECTED FILES
  src/kcmoduleqml.cpp

To: davidedmundson, #plasma
Cc: kde-frameworks-devel, michaelh, ngraham, bruns