Workaround for the bug 393630 - SystemTray part
ClosedPublic

Authored by trmdi on Feb 7 2019, 9:02 AM.

Details

Summary

Avoid using plasmoid.rootItem in SystemTray's ConfigEntries.qml.

BUG: 393630

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
trmdi created this revision.Feb 7 2019, 9:02 AM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 7 2019, 9:02 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
trmdi requested review of this revision.Feb 7 2019, 9:02 AM

I notice that something with model goes wrong. I've try to fix it in D18249 but unfortunately I don't see problem in.

trmdi added a comment.Feb 7 2019, 9:39 AM
This comment was removed by trmdi.

If the only usage is the SNI model lets just rebuild the model on the ConfigEntries side.
You're right that sharing objects between contexts seems to cause issues.

It's backed by a datasource so all the heavy operations are already implicitly shared.

We should only need

  PlasmaCore.DataSource {
         id: statusNotifierSource
         engine: "statusnotifieritem"
         interval: 0
         onSourceAdded: {
            connectSource(source)
         }
         Component.onCompleted: {
             connectedSources = sources
         }
   }


PlasmaCore.SortFilterModel {
       id: statusNotifierModel
       sourceModel: PlasmaCore.DataModel {
           dataSource: statusNotifierSource
       }
   }

and then port all the plasmoid.rootItem to just our local statusNotifierModel

@davidedmundson it works. I really don't know that sharing model between contexts cause a such behavior. It's caused by some ref counting ? Did you know why, it's internal Qt issue that we can fix or it's expected ?

davidedmundson requested changes to this revision.Feb 7 2019, 4:06 PM

I really don't know that sharing model between contexts cause a such behavior.

Sorry, I used the wrong word. Sharing contexts is fine. Sharing across engines is problematic.

QQmlData is shared between engines, but the relevant QObjectWrapper is
not. Since 749a7212e903d8e8c6f256edb1836b9449cc7fe1 when a
QObjectWrapper is deleted it resets the shared QQmlData propertyCache.

Any code path on the first engine that use the propertycache need to rebuild it

(In an ideal world we wouldn't have 2 engines, but that's a whole different topic)

This revision now requires changes to proceed.Feb 7 2019, 4:06 PM

Ok, so @trmdi you can apply exactly David suggest and discard D18804, i've test it and it works. Push it in 5.15 not 5.16 that's bug fix not feature. 5.12 can be also interested path as LTS version.

trmdi added a comment.Feb 7 2019, 5:04 PM
This comment was removed by trmdi.
trmdi updated this revision to Diff 51146.EditedFeb 8 2019, 3:57 AM
trmdi edited the summary of this revision. (Show Details)

Use @davidedmundson 's approach.

@ngraham Could you please help me to discard D18804 ? This patch does not require it anymore.
And land it.

Thanks everyone !

trmdi edited the summary of this revision. (Show Details)Feb 8 2019, 4:15 AM
davidedmundson accepted this revision.Feb 8 2019, 4:58 AM
This revision is now accepted and ready to land.Feb 8 2019, 4:58 AM
fvogt added a comment.Feb 8 2019, 8:27 AM

Use @davidedmundson 's approach.

@ngraham Could you please help me to discard D18804 ? This patch does not require it anymore.

You can select "Abandon revision" as action.

And land it.
Thanks everyone !

trmdi added a comment.Feb 8 2019, 8:42 AM

You can select "Abandon revision" as action.

Done. Thank you. :)

This revision was automatically updated to reflect the committed changes.