Also allow invoking session restoration logic when apps are manually launched

Authored by ngraham on Nov 1 2019, 3:28 PM.



Right now session restoration logic only ever gets invoked when apps are started
automatically after rebooting, because of the check for qApp->isSessionRestored().
This limits the feature to only working for that use case, since qApp->isSessionRestored()
is set by Qt and we can't override it in the app itself to signal that we want the session
restoration behavior for other reasons. As a result, we can't use it to implement the
behavior of restoring the condition of app when it's manually launched, which has been
requested for several of our apps.

This patch removes the check for qApp->isSessionRestored(), which opens the door to
implementing the requested features in our apps.

CCBUG: 397463
CCBUG: 413564

Test Plan

D11382 now works

Normal session restoration behavior after a reboot is unchanged

Apps with session restoration behavior but without a user-facing option to invoke it
don't restore session at inappropriate times.

Diff Detail

R263 KXmlGui
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
ngraham created this revision.Nov 1 2019, 3:28 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptNov 1 2019, 3:28 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
ngraham requested review of this revision.Nov 1 2019, 3:28 PM


This method behaves weirdly and not what I expected from reading this code.

If no-one has explicitly called KConfigGui::sessionConfig it creates a new one using qApp->sessionConfig() regardless of whether we're session restored or not. If we're not sessionRestored it'll effectively leak a bogus config with a broken ID name.

I think to accept the patch we would need to also change

kconfiggui.cpp:42 to be

  • if (!hasSessionConfig()) {

+ if (!hasSessionConfig() && qApp->isSessionRestored()) {

Can I get a review on this?

davidedmundson accepted this revision.Nov 20 2019, 10:06 AM

Normal session restoration behavior after a reboot is unchanged

Please make sure you try with a few apps that aren't just dolphin

This revision is now accepted and ready to land.Nov 20 2019, 10:06 AM

Yep, there are no changes in behavior or regressions when testing with Okular, Kate, System Settings and Discover.

This revision was automatically updated to reflect the committed changes.
mlaurent reopened this revision.Nov 26 2019, 8:04 AM
mlaurent added a subscriber: mlaurent.

This patch creates a bug in kontact as it creates 2 kontacts now.

Could you reevaluate this patch please ?


This revision is now accepted and ready to land.Nov 26 2019, 8:04 AM

This indeed seems to have negative impact on Kontact, while other people get duplicates it doesn't start at all for me anymore. With this patch reverted it's back to normal.

Found also by openQA: two kontact windows get opened.

This patch was supposed to be partnered with a change to KConfigGui::sessionConfig() to keep behaviour the same.

It seems that is not merged.

Oops, sorry. D25219 was just reviewed and has landed now; can you folks verify whether the problem in Kontact is fixed now?

I updated kconfig and now it's ok. I can't see the "2 kontact" bug.