[GTK] Don't invalidate style context when setting background color #2702

[ptziegler]

When setting the background color while inside an SWT.Activate event, pending events are not processed.

The gtk_style_context_invalidate() method is deprecated in GTK3 and doesn't have a GTK4 counterpart, as GTK invalidates the style context automatically.

With b7dee8a, this method call was already removed, but later reverted due to test failures in the Eclipse Platform. Rather than removing all references, this change only does so for the call to setBackground().

To reproduce, execute the Snippet388. When clicking on a tree/table item, the item should be selected. This is the current behavior on Windows and MacOS, but not on Linux.

Closes #2702

[ptziegler]

This is definitely a risky change, but perhaps it inspires someone.

[github-actions]

Test Results

  118 files  + 3    118 suites  +3   17m 14s ⏱️ + 4m 51s
4 653 tests +37  4 635 ✅ +36  18 💤 +1  0 ❌ ±0 
  338 runs  +18    334 ✅ +17   4 💤 +1  0 ❌ ±0 

Results for commit a838ee0. ± Comparison against base commit 1d450f7.

♻️ This comment has been updated with latest results.

[laeubi]

@ptziegler can you explain why you think it is risky and what the invalidate do? Will it simply "repaint" the control somehow? From what you describe it seems one only want to process such invalidate after all events are processed, so maybe one should check if the event queue is empty and if not schedule it (again) async?

[ptziegler]

can you explain why you think it is risky

By invalidating the style context asynchronously, I'm putting a lot of faith into the assumption that the context pointer is still valid at that point. The check whether the widget is disposed is just a workaround. Also as pointed out in #2702 (comment), invalidating the context asynchronously doesn't make much sense when it is automatically invalidated by GTK. So it will likely cause the same issues that were caused when the line was simply removed.

and what the invalidate do?

I'm really not an expert on GTK, but I think it simply "applies" the CSS style. Meaning updating the colors and bounds of the widget. In some of the tests that failed because of this, you got assertion errors like:

Warning: Button {Always r&un in background}
	Actual Width -> 166
	Recommended Width -> 168

Which sounds like the style was not updated in time.

From what you describe it seems one only want to process such invalidate after all events are processed, so maybe one should check if the event queue is empty

At least as simple display.readAndDispatch() doesn't work. From my experience, GTK is also very finicky once you're inside an event. There more funny stuff you try to do (like processing a selection-event within an activate-event), the more I expect other things to break. I think #697 is such an example where you can get a crash when restyling during a gtk_size_allocate.

Perhaps styling should be done asynchronously after activation? But that probably introduces a completely different set of problems...

[akurtakov]

I am more and more convinced that dropping invalidate (no immediate styling effect) in next cycle and see what(if anything) breaks. Gtk 4.x no longer even have the invalidate so there must be a way to work without it, and newer Gtk versions (since the time invalidate call was introduced) should be less fragile to styling.

[akurtakov]

Merging so it has maximum amount of test period.