Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 87586

Summary: View closure doesn't pay attention to perspectives
Product: [Eclipse Project] Platform Reporter: Jared Burns <jared_burns>
Component: DebugAssignee: 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 Flags
patch to org.eclipse.debug.ui
none
patch2 to org.eclipse.debug.ui none

Description Jared Burns CLA 2005-03-09 17:45:42 EST
When the LaunchViewContextManager is deciding which views it needs to close, it
just looks at what views it has opened. It doesn't consider which perspectives a
view was opened in.

This can cause us to close more views than we really should.

For example:
1. Debug to a breakpoint with view management turned on in the Java perspective.
Debug-related views (Variables, Breakpoints, etc.) are automatically opened.
2. Switch to the Debug perspective and terminate the launch.
3. The views that were opened (Variables, Breakpoints, etc.) are automatically
closed despite the fact that they weren't automatically opened in the Debug
perspective (they are already open by default).
Comment 1 Samantha Chan CLA 2005-05-13 14:57:38 EDT
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
Comment 2 Jared Burns CLA 2005-05-13 16:25:47 EDT
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. :-/
Comment 3 Samantha Chan CLA 2005-05-17 13:32:24 EDT
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.
Comment 4 Samantha Chan CLA 2005-05-17 13:32:55 EDT
Darin -
Please apply patch.  Thanks...
Samantha
Comment 5 Darin Wright CLA 2005-05-17 14:55:51 EDT
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 
Comment 6 Darin Wright CLA 2005-05-17 14:56:24 EDT
Samantha, can we close views we opened in other perspectives?
Comment 7 Jared Burns CLA 2005-05-17 15:59:49 EDT
Bug 57841
Comment 8 Samantha Chan CLA 2005-05-17 16:07:49 EDT
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 9 Jared Burns CLA 2005-05-17 16:16:35 EDT
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.
Comment 10 Darin Wright CLA 2005-05-17 16:31:13 EDT
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.
Comment 11 Jared Burns CLA 2005-05-17 16:38:13 EDT
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.
Comment 12 Darin Wright CLA 2005-05-17 16:43:22 EDT
I agree, I meant that we will fix this bug, and track 57482.
Comment 13 Darin Wright CLA 2005-05-17 16:50:50 EDT
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.
Comment 14 Samantha Chan CLA 2005-05-17 17:19:26 EDT
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.
Comment 15 Darin Wright CLA 2005-05-17 17:44:52 EDT
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).
Comment 16 Darin Wright CLA 2005-05-17 17:45:10 EDT
Please verify my changes, Samantha.
Comment 17 Samantha Chan CLA 2005-05-17 18:15:15 EDT
Verified.