Queue binding operations without checking thread

[Jacalz]

Description:

Once #5509 is merged, running binding callbacks from main no longer print a warning so we cna avoid checking the thread. The result is much faster performance.

I also fixed a bug where we were scheduling s.DataChanged() onto main, which in itself then runs a trigger() which schedules the work onto main. That was unnecessary overhead.

Checklist:

• Tests included.
• Lint and formatter run with no errors.
• Tests all pass.

[coveralls]

coverage: 62.585% (-0.001%) from 62.586%
when pulling da0856d on Jacalz:binding-func-scheduling
into 3f7f987 on fyne-io:develop.

[andydotxyz]

I /think/ that DataChanged should be called on main as it is part of the public API and could be called back into user facing code.

I also fixed a bug where we were scheduling s.DataChanged() onto main, which in itself then runs a trigger() which schedules the work onto main. That was unnecessary overhead.

There may be a bug in here, but I think the right solution is slightly different.

[Jacalz]

There may be a bug in here, but I think the right solution is slightly different.

I'm not sure I agree. We said that data bindings should be thread safe, did we not? Also, the old code was inherently wrong as it scheduled a call onto main which then just scheduled a call onto main. I don't see this being any worse than it already was. There is no functional change there in this PR, it just avoids bouncing once on main before again going to main and running the actual stuff there.

[Jacalz]

DataChanged() for the conversion types literally only calls trigger() which itself schedules the data listener updates onto main. We don't need to call DataChanged() on ourselves from main.

[Jacalz]

Ah, I kind of see what you mean. We would match other binding types better by just calling .trigger() in those instances.

[andydotxyz]

DataChanged() for the conversion types literally only calls trigger() which itself schedules the data listener updates onto main. We don't need to call DataChanged() on ourselves from main.

The core of this is that DataChanged is in the listener API - anyone that called AddListener would expect the Fyne callback to be on the correct thread.

I think this is now correct.

[andydotxyz]

Thanks, I think that's the threading sorted now.

[Jacalz]

I think this is now correct.

Thanks. Yeah, I agree and your explanation matches my understanding as well now. Anyone actually calling DataChanged on those types would still get a double bounce on main unfortunately. Might be a problem for a future PR though