[akregator part] Make sure part is created before restore window
ClosedPublic

Authored by anthonyfieroni on Jul 16 2017, 5:33 PM.

Details

Summary

This is internal change in Qt 5.9 i think. Call winId before setCentralWidget creates a temporary widget then it owns a setted one.

BUG: 381822
BUG: 378513
BUG: 381825
BUG: 377129

Test Plan

restore
readPropertiesInternal
applyMainWindowSettings
HERE
It's called winId, but part it's not created (it's done here)

Diff Detail

Repository
R201 Akregator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
anthonyfieroni created this revision.Jul 16 2017, 5:33 PM
Restricted Application added a project: KDE PIM. · View Herald TranscriptJul 16 2017, 5:33 PM

I face the problem, when you start with session restore there is not windowhandle when call here -> https://phabricator.kde.org/source/akregator/browse/master/src/mainwindow.cpp;a03c9d7001bdc92059e8cf02eb6f00b4a8019ee6$123
in kxmlgui https://phabricator.kde.org/source/kxmlgui/browse/master/src/kmainwindow.cpp;af76e8add07ffca2104f4ecb82ded43059e6abfb$666 the window becaume parent of part widget and when it's delete it cause part to delete itself.
Test:
akregator --session ~/.config/session/<session-id>

winId() in kxmlgui should not die? David how to fix this issue?

dvratil requested changes to this revision.Jul 23 2017, 9:55 AM
dvratil added a subscriber: dvratil.
dvratil added inline comments.
src/mainwindow.cpp
118 ↗(On Diff #16781)

Now that the Frame is wrapped in QPointer, it should be OK leave the part to auto-delete itself. Otherwise we would be memory-leaking the part?

139 ↗(On Diff #16781)

Why remove this check? I would assume it's there to prevent re-loading the part if it is already loaded.

This revision now requires changes to proceed.Jul 23 2017, 9:55 AM
src/mainwindow.cpp
118 ↗(On Diff #16781)

I can't understaind why Qt deletes part' widget (this is only in session restored mode), someone else set a centralWidget ?
Backtrace -> https://paste.kde.org/pzbqweaj3

139 ↗(On Diff #16781)

It has a check inside loadPart.

anthonyfieroni retitled this revision from [akregator part] Protect part from auto delete to [akregator part] Make sure part is created before restore window.
anthonyfieroni edited the summary of this revision. (Show Details)
anthonyfieroni edited the test plan for this revision. (Show Details)
anthonyfieroni edited edge metadata.
dfaure accepted this revision.Sep 9 2017, 12:55 PM
This revision was automatically updated to reflect the committed changes.