| Summary: | View closure doesn't pay attention to perspectives | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Jared Burns <jared_burns> | ||||||
| Component: | Debug | Assignee: | Samantha Chan <chanskw> | ||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P2 | CC: | chanskw, darin.eclipse | ||||||
| Version: | 3.1 | ||||||||
| Target Milestone: | 3.1 RC1 | ||||||||
| Hardware: | PC | ||||||||
| OS: | All | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
|
Description
Jared Burns
Hi Jared - I am looking at this bug... and I have a question about why the context listener needs to perists openedViewIds. The openedViewIds is used to keep track of views that are opened by context enablement. Are you trying to cover the case when there are context-enabled views opened when the workbench shuts down and when the workbench is restarted, you will know to close those views as a context goes away? Also, looking at how openedViewIds and viewIdsToNotOpen are saved in the preference. When they are saved, they use the following attributes: LaunchViewContextListener.PREF_VIEWS_TO_NOT_OPEN LaunchViewContextListener.PREF_OPENED_VIEWS However, when these two lists are loaded, the loading uses the following attributes: ATTR_OPENED_VIEWS ATTR_VIEWS_TO_NOT_OPEN So, when the the preferences are loaded, those two lists are always empty. I am not sure why the saving and loading are using different attribute names. Thanks for your help. Samantha Yikes. Yes, we should be persisting those lists so that our non-disruptive approach (opening/closing of views shouldn't conflict with user actions) is maintained across sessions. It looks like this is just totally busted so the view management has been ruder than intended. :-/ Created attachment 21279 [details] patch to org.eclipse.debug.ui * openedViewIds is now a hashmap instead of a global list of view ids. Perspective id is the key to the hashmap. ArrayList of view ids is the value of the hashmap. * When views are automatically opened or closed, LaunchViewContextListener finds the array list from the hashmap using the current perspective id. The view ids will then be added/removed from the array list. * Load and Save opened-views and views-to-not-open with the correct preference names. * Added code to save and load the new hashmap * Aslo included fix for Bug 94357. Please refer to that bug for details on the fix. Darin - Please apply patch. Thanks... Samantha There is a related problem:
* Debug to a breakpoint in the Java perspective (auto open views, don't switch
perspective automatically)
* The BPs and Var views open
* Switch to debug perspective
* terminate launch
* Switch back to Java Perspective
> the BP and Var views are still open, but shuold be closed
Samantha, can we close views we opened in other perspectives? Looks like we have no API to close views from other perspective. Also looked at using DebugUIPlugin.getActiveWorkbenchWindow().getPages(); to get all the perspectives and then close the views. But calling this method only returns the current perspective. If you switch back to the Java Perspective and relaunch, the views will be closed when the debug session is terminated. Comment #5 is a dup of Bug 57842. I opened this bug report solely for the test case described, because we can handle that case without any new API. OK. Unfortunate bug. We could track perspectives etc., and manage this ourselves, but I'd prefer to wait for the fix to bug 57841. Will track with bug 57842. This bug is worse than bug 57841 and the fix isn't dependant on any other bugs. Bug 57841 describes a problem where we leave views open that we shouldn't; this is just a potential inconvenience. This bug is that we end up wrongly closing views that are critical to debugging; "losing" the Breakpoints view is potentially a much bigger problem for the user. I agree, I meant that we will fix this bug, and track 57482. Applid patch (with minor cosmetic changes). Samantha, I think we should use a set in the hashtable (for keeping track of which views are open). It may perform better than lists (avoids linear searches). Please provide incremental patch. Created attachment 21298 [details]
patch2 to org.eclipse.debug.ui
* Use Hashset instead of ArrayList
* Moved updating of openedViewIds out of the loop to reduce number of updates.
Applied patch with minor changes (removed some unrequired #contains(..) tests and replacing the sets back into the hashmap, as they are the same object being manipulated). Please verify my changes, Samantha. Verified. |