Bugzilla – Full Text Bug Listing
| Summary: | [Progress] Need way to listen to when DeferredTreeContentManager is finished | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Brian Bauman <baumanbr> | ||||||
| Component: | UI | Assignee: | Tod Creasey <Tod_Creasey> | ||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | lesojones, remy.suen, tom.schindl | ||||||
| Version: | 3.4 | ||||||||
| Target Milestone: | 3.4 M5 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows XP | ||||||||
| Whiteboard: | |||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 191365 | ||||||||
| Attachments: |
|
||||||||
|
Description
Brian Bauman
Don't you control the contents though?Why don't you know when you are done? The DeferredTreeContentManager adds a PendingUpdateAdapter object to the tree as a place holder. It then runs a job to do the searching for children which is where our code adds children to the Tree (by calling the anonymous IElementCollector.add(..) class in DeferredTreeContentManager). At this point, I have added all the children I need to and I am done populating the tree so my code returns. This more or less finishes the Job which was created to populate the contents in the background. The PendingUpdateAdapter object is still in the tree, so another Job is run to remove it (see DeferredTreeContentManager.runClearPlaceholderJob(...)). This job modifies the tree by removing the PendingAdapter which is really the event I need to listen to (so I can accurate get the element count from the Tree and reset the scrollbar to the top of the Tree). So I know when I am done populating the Tree, but DeferredTreeContentManager still has some clean up to do and I can't tell when that has been completed. Hope that makes sense :) Doesn't your model have the accurate count from the job you used to populate it? Do you not have access to the data from that job? Relying on the tree contents is always dangerous as we optimize population of the tree so it is never as reliable as your model. We have a total count from our models, but the view uses filters and the count is updated based on how many items are visible. So for this we use the count of the tree, which has not had us go wrong sense. Just for the record, I was able to get the count working with some type checking in the tree (mainly seeing if the last element was a PendingUpdateAdapter). The part I can't solve it trying to reset the scroll bar to the top of the list when everything is finished. I figured if there was this notification, I could do that and update the count at the same time, killing two birds with one stone. If the platform could manually reset the focus of the tree to the top of the list after removing the PendingUpdateAdapter, I would be fine with that also. Brian having a model that is unaware of what it is showing is pretty dangerous. I think you are going to get bitten by a viewer optimization at some point. I think the listener is the best choice because it is the least invasive solution. If you propose a patch I can look at it. Tom, I may be misunderstanding you, but your last comment seemed to imply that the model should become viewer aware? Grovelling apologies Tod, I called you Tom. Sorry. Quite the opposite Les - I am concerned that Brian is relying on the viewer rather than his model to understand what is being shown. Tod, if I understand Brian correctly, he's showing the number of items visible in the tree, which can be controlled via ViewerFilter instances - these are inherently viewer specific and the model knows nothing of these. When a new filter is applied, the tree viewer will (potentially) show a different subset of elements; again the model is unaware of this. Without querying the viewer itself (or in this case viewer.getTree().getItemCount()) to get the number of visible items, how would you get this information? Created attachment 88373 [details]
proposed patch
This is the simplest modification which allows me to add the listener I need. It should be pretty simple as all I did was refactor out the creation of the "clearJob" into a protected function responsible for creating the job.
Created attachment 88456 [details]
Suggested API
He is my proposal - add an listener for the update job and I will apply it when the clear job is created.
> Here is my proposal - add an listener for the update job and I will apply it when
> the clear job is created.
That looks great! That will give us everything we will need. Thanks for looking into this Tod.
Patch released for build >20080131-0800 Brian can you verify this please? Verified on I20080204-0800. Thanks Tod for working with me to get this in for the milestone. The Plug-ins view is working great with the loading in the background! |