Bugzilla – Full Text Bug Listing
| Summary: | Invalid thread access on 'add depenencies' in Manifest editor | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] PDE | Reporter: | Gunnar Wagenknecht <gunnar> | ||||||
| Component: | UI | Assignee: | Curtis Windatt <curtis.windatt.public> | ||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | ankur_sharma, curtis.windatt.public | ||||||
| Version: | 3.7 | ||||||||
| Target Milestone: | 3.8 M2 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows 7 | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
|
Description
Gunnar Wagenknecht
The UI editor should handle the UI changes in the correct thread. It shouldn't assume that all model change events will be fired in the UI thread. Created attachment 199680 [details]
Proposed fix
Gunnar, can you please give the patch a try. The fix runs the editor updating in a UI thread. I changed both the import package and require bundle sections.
Patch applied to HEAD. Please verify Gunnar. There is a regression. I am seeing duplicate entries in the Imported Packages sections.
How to repro in I20110802-2000:
1. Create an empty plug-in. Let is contribute to UI (that is, o.e.ui and o.e.core.runtime be already there in the required plug-in section)
2. Add o.e.pde.ui, pde.core and pde.launching to the 'automated mgmt of dependencies'
2. Create a class and add references to some pde APIs. I used this
public class C {
IBaseModel model;
IBasePluginWizard wiz;
PDESourcePathProvider p;
}
3. In 'automated..' section select the import-package radio and click 'add dependencies'
Result: The imported packages and 2 entries for each of the three packages.
Try to delete them, you can delete only one instance of each.
Created attachment 201348 [details]
Fix for the regression
The regression is caused by the model having no way to coalesce model changes. In addition, the model updates are poorly designed as the first creation of the header comes as a CHANGE event for the bundle, while future updates come as INSERT events for just the import header. The required bundle section doesn't experience the same problem because it doesn't have this odd model setup. I looked at changing the model structure, but after sinking several hours into it I haven't found a workable solution.
This patch fixes it by making sure we don't add a duplicate package object to the UI.
I found a problem with the remove functionality. This is fixed by updating the buttons after performing the remove.
Applied the additional fix to HEAD. It appears the attached patch missed one of the changes. In RequiresSection.java (from the plugin editor, not the feature editor), the same updateButtons() call is made at the end of the remove inside the UIJob. Verified in I20110913-0200 |