Bugzilla – Full Text Bug Listing
| Summary: | views automatically opened based on debug context are hidden/closed on perspective switch | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Natasha D'Silva <ndsilva> | ||||||||
| Component: | Debug | Assignee: | Pawel Piech <pawel.1.piech> | ||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||
| Severity: | major | ||||||||||
| Priority: | P3 | CC: | boxall, darin.eclipse, Michael_Rennie, pawel.1.piech, pwebster | ||||||||
| Version: | 3.6 | Flags: | pawel.1.piech:
review?
(darin.eclipse) |
||||||||
| Target Milestone: | 3.6.2 | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | All | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Natasha D'Silva
Created attachment 179284 [details]
patch with a partial fix.
Created attachment 179429 [details]
More targeted fix.
The bug is caused by the fact that there is a gap between when the activated views are cleaned up in perspectivePreDeactivate() and when the perspective switch actually occurs. During this gap, if activateChain() is called it will activate views as seen by the view context manager, but not according to the perspective manager. When the perspective is restored the views are not re-activated because the view context manager thinks they're already activated.
This patch adds an fActivePerspecive field to let the view context manager track the active perspective and avoid activating views after cleanup() is called. I'm not sure if it's not an overkill but it seems to work.
This patch does not address re-activating the view after perspective switch. This could be fixed by adding another field and persisting it with the bindings class. Although, this view activation mechanism is already kind of shaky so I don't know if we want to keep adding to it without creating some kind of test suite first.
what level of code does this patch work with? I have a very recent 3.6.1 eclipse and the patch doesn't compile/match. The calls to activateChain(String, IPerspectiveDescriptor) in ViewContextService.contextActivated() don't match the activateChain(String) in ViewContextService. It looks like the source for 3.6.1 doesn't match what you have in the patch. The patch will be required on 3.6.1 unless it was your intent for me to patch another version of eclipse for test purposes. (In reply to comment #3) > what level of code does this patch work with? The patch applies to 3.7, where a change was already made to fix a leak. Created attachment 179491 [details]
Targeted fix for 3.6.1.
(In reply to comment #5) > Created an attachment (id=179491) [details] > Targeted fix for 3.6.1. I installed the patch and ran a quick test and it resolves the problem. I'll try a few more things and report back the status. Thank you for the quick turnaround on the 3.6.1 patch! (In reply to comment #5) > Created an attachment (id=179491) [details] > Targeted fix for 3.6.1. I ran some more testing and reviewed the patch changes. If I read the patch correctly the key change is saving the active perspective rather then getting it fresh each time it is required. I'm guessing that the old way had the potential to return a different perspective from what was expected. It behaves much better and in fact seems to behave the same as 3.4 It also has the minor problem (that is in 3.4) related to focus of views that were not opened by the user. If the user gives focus to a view that was opened by the debug context and then switches prespectives it will not get focus on going back to the original perspective. Focus is given to the next view that was already open or was manually opened by the user. Is there a bug open for that misbehavior? If not I can open one. BTW this minor problem is not a regression or required for 3.6.1 (In reply to comment #7) > If the user gives focus to a view that was opened > by the debug context and then switches prespectives it will not get focus on > going back to the original perspective. Focus is given to the next view that > was already open or was manually opened by the user. > Is there a bug open for that misbehavior? If not I can open one. BTW this > minor problem is not a regression or required for 3.6.1 I don't know of any bugs for this problem, please open one. I committed the fix for 3.7 and I'll wait till next week to commit the fix for 3.6.2. Thanks for your help. Hi Pawel, Thanks for the patch, I tried it and it does fix the problem of the views missing when we return to the debug perspective. So now, the behavior is that the view is actually closed and re-opened. This means: -if the view was visible before the perspective switch and tabbed beside any platform debug views, it's now hidden. -expansion state and any other state related information stored in the view is lost. I traced it and the view's closure is triggered from ViewFactory#releaseView, in the call to WorkbenchPage#partRemoved(). Should a separate defect be opened for this problem? I tested in 3.4.2. and the behaviour is the same (view closed and reopened). I think the issue I commented on is similar to what Alan mentioned. Sorry, I hit commit by accident. I was also going to add that since the major problem has been fixed by the patch, that you could go ahead and check it into 3.6.1 when you're ready. Thanks. I committed the 3.6.1 fix. Darin please review if you get a chance. (In reply to comment #10) > So now, the behavior is that the view is actually closed and re-opened. This > means: > -if the view was visible before the perspective switch and tabbed beside any > platform debug views, it's now hidden. > -expansion state and any other state related information stored in the view is > lost. > I traced it and the view's closure is triggered from ViewFactory#releaseView, > in the call to WorkbenchPage#partRemoved(). > Should a separate defect be opened for this problem? Please do file a separate bug. The reason that the view is closed on perspective switch is that the debugger's view management mechanism does not have an API to control which views are "open" for perspectives that are not active. When a debug session ends, the debugger cannot retroactively go to through closed perspective and clean up closed views. I think a possible fix would be for the debugger to somehow track which views were supposed to be open and close them when the perspective is finally opened again, but this could happen after a workbench restart. In this case the debugger plugin may not be loaded when the perspective in question is activated. If the debugger plugin is not loaded to handle view cleanup, then we'll have a different sort of bug to deal with. Perhaps the best solution would be to push the view-context binding management to the workbench plugin. If you can influence the UI team to take accept such a contribution, it would be very helpful. |