Fixes for #678 and #1739

[zkxvh]

Fixes for #678 and #1739.
Initially I wanted to fix #678, but after the fix the problems described in #1739 got worse, so I fixed them too.

Fixes #678
Fixes #1739

[zkxvh]

Fix for #678

This is the 1st commit in this branch.

Reproducing the crash:

• SIGSEGV in Tree.cellDataProc when calling TreeItem.setImage #678 (comment)
• [GTK3] Test Tree crash in SetData event #1611
• SIGSEGV in Tree.cellDataProc when calling TreeItem.setImage #678 (comment)

The cause of the crash is described here:
#678 (comment) In short, the crash happens due to read accesses to an already disposed renderer.
The sequence of action leading to the crash was:

• in a Tree with SWT.VIRTUAL a TreeItem is rendered for the first time or after clear()
  • Tree.cellDataProc() is executed for the item and one of the renderers
    • Tree.checkData(item) is called for the item
      • SWT.SetData event is created and sent for the item
        • TreeItem.setImage() is executed by the event handler for SWT.SetData``
        • Tree.createRenderers() executes and disposes the current renderer
    • further actions in Tree.cellDataProc() that access the already-disposed renderer (they think it's alive)

How it's fixed: in Tree.cellDataProc() wrap Tree.checkData(item) into Display.asyncExec().

Why fixed this way:

1. on one hand, Tree.cellDataProc() is a cell data function which is not supposed to change tree structure. Violation of this leads to C memory errors.
2. On the other hand, SWT.SetData event handlers are written by swt users and therefore can contain any code.

Using Display.asyncExec() to postpone SWT.SetData event handlers until Tree.cellDataProc() is finished seems like the most simple and bullet-proof solution to #678 and all similar bugs.

[zkxvh]

Fix for #1739

This is the 2nd and 3rd commits in this branch.

The 2nd commit adds native method gtk_tree_view_column_queue_resize(...) from GTK.
I'm not sure whether something else should be done when new native methods are added, so I made as a separate commit.

The 3rd commit is the fix for image display for Tree and Table (both for normal and SWT.VIRTUAL modes).
To test that images are displayed correctly now I used the following snippet-programs: snippets.zip.
Of course, it would be great to create unit tests for that, but I haven't found a way to get the size of the image shown in a tree/table, so manual tests is the best I can offer.

All the snippets are similar, here is what SetImage_TreeVirtual (it's a snippet for Tree with SWT.VIRTUAL) looks like:

At the top there are 4 rows of buttons with images.
Pressing any of the buttons sets its image to the tree cell at the row and column given in the button text.
Pressing the same button again removes the image from the cell.

Buttons -col,+col,col-,col+ can be used to add/delete columns.

This how Tree and Table should work (or at least this is how it seems to me after digging into the source code):

1. all images of a Tree/Table are of the same size, which is the size of the 1st image set to the Tree/Table. All the images added after the 1st one are automatically scaled to that fixed size.
2. for each column: initially no width for images is shown in the cells, but when the 1st image is set for any of the cell, then the width for images gets shown for all the cells of that column.
3. row height: for Trees and Tables without SWT.VIRTUAL the height of each row is changed automatically to fit all images in the row cells, for Trees and Tables with SWT.VIRTUAL fixed-height is used and the height of all rows is set to >=fixed_image_heigth when the 1st image is set for the Tree/Table

I used the following tests with SetImage_TreeVirtual to check that everything works:

1. restart snippet, press buttons [1,0], then [2, 1], then [3, 2].
   Expected result: all 3 images are shown with size of [1,0]
   1. same as (1) but press [2, 1] first, all images in the tree should have size of [2, 1]
   2. same as (1) but press [3, 2] first, all images in the tree should have size of [3, 2]
2. restart snippet, press button [32,1], then scroll down until cell [32,1] is visible, then press buttons [31,0], [33,2]
   Expected result: all 3 images are shown with size of [32,1], the height of all rows grows to fit the image when you scroll to [32,1]
   1. same as (2) but press [33,2] first, all images in the tree should have size of [33,2]
   2. same as (2) but press [31,0] first, all images in the tree should have size of [31,0]
3. same as (1) and (2) but for nested rows (use the 3d and 4nd rows of the buttons with images)
4. special test: Tree have special logic for a case when Tree.getColumns().length==0 - it shows some default column in this case. Using buttons -col,+col,col-,col+ make sure that the default column is copied to/from the 1st column when Tree.getColumns().length changes between 1 and 0.

The same manual tests I performed with snippet SetImage_Tree (Tree without SWT.VIRTUAL) and for Table (snippets SetImage_TableVirtual and SetImage_Table).

[iloveeclipse]

There is one test failing:

java.lang.AssertionError: SetData callback count not in range: 10001
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.eclipse.swt.tests.junit.Test_org_eclipse_swt_widgets_Tree.test_Virtual(Test_org_eclipse_swt_widgets_Tree.java:966)

I haven't checked if this is related to this PR or not, could you please check?

[github-actions]

Test Results

  289 files   -   205    289 suites   - 205   3m 45s ⏱️ - 5m 11s
4 084 tests  -   246  4 075 ✅  -   242   8 💤  -  5  1 ❌ +1 
8 246 runs   - 8 322  8 214 ✅  - 8 246  31 💤  - 77  1 ❌ +1 

For more details on these failures, see this check.

Results for commit 2edcaa6. ± Comparison against base commit a0a0485.

This pull request removes 246 tests.

org.eclipse.swt.graphics.ImageWin32Tests ‑ testImageDataForDifferentFractionalZoomsShouldBeDifferent
org.eclipse.swt.graphics.ImageWin32Tests ‑ testImageShouldHaveDimesionAsPerZoomLevel
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_dollarSign
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_emptyString
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_letterA
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_letters
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16LE_null
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_AsciiLetters
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_Asciiletter
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_LotsOfLetters
…

This pull request skips 3 tests.

org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser ‑ test_Constructor_multipleInstantiationsInDifferentThreads[browser flags: 0]
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser ‑ test_setUrl_remote_with_post[browser flags: 0]
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_widgets_Button ‑ test_traverseCheckButton

[zkxvh]

There is one test failing:

java.lang.AssertionError: SetData callback count not in range: 10001
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.eclipse.swt.tests.junit.Test_org_eclipse_swt_widgets_Tree.test_Virtual(Test_org_eclipse_swt_widgets_Tree.java:966)

I haven't checked if this is related to this PR or not, could you please check?

Sure, I'll check this test.
It looks like this test checks that for virtual trees SWT.SetData callbacks should only execute for visible rows, but now they seem to execute for all rows (which is bad).
This may be caused by the changes in the PR.

[zkxvh]

Test_org_eclipse_swt_widgets_Tree.test_Virtual() is fixed in commit 62b1a13.

The problem was in setScrollWidth() which is invoked at the end of cellDataProc(...).
As I understand, setScrollWidth() works when Tree.getColumnCount()==0, i.e. when the default column is shown.
For each rendered cell the code sets the width of the default column to be max(currentWidth, width_of_current_cell_content).

The fix removes recursive visiting of the nested cells for the current item - this should work because during rendering cellDataProc(...) should be invoked for every updated visible cell anyway.

I checked this case in SetImage_TreeVirtual snippet and the scroll width seems to increase when an image is inserted - i.e. it seems to work correctly.