This Bugzilla instance is deprecated, and most Eclipse projects now use GitHub or Eclipse GitLab. Please see the deprecation plan for details.
Bug 252677 - Add a breadcrumb control to manage the active debug context.
Summary: Add a breadcrumb control to manage the active debug context.
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Debug (show other bugs)
Version: 3.5   Edit
Hardware: PC All
: P3 enhancement (vote)
Target Milestone: 3.5 M5   Edit
Assignee: Darin Wright CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 242489 258196
Blocks: 252669
  Show dependency tree
 
Reported: 2008-10-29 16:39 EDT by Pawel Piech CLA
Modified: 2009-05-15 09:48 EDT (History)
11 users (show)

See Also:


Attachments
Patch with initial prototype. (137.71 KB, patch)
2008-10-29 16:39 EDT, Pawel Piech CLA
no flags Details | Diff
Screenshot. (3.25 KB, image/png)
2008-10-29 17:05 EDT, Pawel Piech CLA
no flags Details
Updated patch. (309.23 KB, patch)
2008-11-19 18:02 EST, Pawel Piech CLA
no flags Details | Diff
Updated screenshot. (20.82 KB, image/png)
2008-11-19 23:47 EST, Pawel Piech CLA
no flags Details
Patch with cleaner APIs. (328.18 KB, patch)
2008-12-03 17:57 EST, Pawel Piech CLA
no flags Details | Diff
Updated patch (350.01 KB, patch)
2008-12-12 13:07 EST, Pawel Piech CLA
no flags Details | Diff
Patch with Breadcrumb in Debug view (compact mode). (301.05 KB, patch)
2009-01-13 19:21 EST, Pawel Piech CLA
no flags Details | Diff
Patch with all fixes. (315.06 KB, patch)
2009-01-20 13:05 EST, Pawel Piech CLA
no flags Details | Diff
Cleaned up patch for breadcrumb in Debug view. (321.68 KB, patch)
2009-01-21 00:24 EST, Pawel Piech CLA
no flags Details | Diff
Patch which addresses the remaining issues. (312.05 KB, patch)
2009-01-22 16:05 EST, Pawel Piech CLA
no flags Details | Diff
New and Noteworthy (75.61 KB, application/octet-stream)
2009-01-23 13:24 EST, Pawel Piech CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Pawel Piech CLA 2008-10-29 16:39:31 EDT
Created attachment 116473 [details]
Patch with initial prototype.

This feature should allow the debugger to be used without having the Debug view open.  

I've been working on prototyping this feature on and off for the last couple of months.  This is what I have so far in the prototype (+), and what I know that needs to be done (-).

1) A breadcrumb control which appears on the status bar.  
 + It displays the active debug context in the window.  
 + The elements in the context path can be selected creating a drop down with a tree control.
 + The drop down contains a sub-tree of debug elements, which can be selected to set a new active debug context.
 - I tried to keep the breadcrumb implementation as close to the one in JDT editor, but I think I'll have to break this approach to make it production quality.  The one in JDT uses a ITreeContentProvider, where we really need a tree path-based version.
 - The drop down positioning and initial size is wrong.  Positioning needs to be based on the status bar location, the size calculation needs to be changed to account for the lazy nature of the debug viewer.
 - The content in the drop down uses the debug view presentation context, but not the launch manager input.  This works sometimes, but other times it breaks assumptions in the element content providers.

2) An implementation of a view-less debug context provider. 
 + It behaves in the same way as the debug view.  I.e. it's root element is the launch manager, and selection in the provider is updated according to the delta events in the model.
 + This provider is created automatically for each window so that it provides a debug context when the Debug view is not open.
 - For some reason this content provider takes precedence initially over the real debug view.  Managing this could be tricky.
 - When the view-less content provider is not active, it should update its selection based on the active window context.  This will allow an easy transition for the user when the debug view is closed.
 - Likewise, I think that the debug view should update its selection based on the active context in the view-less provider.

3) An additional drop-down menu which allows the user to select the active debug context provider.  
 + The contexts framework needs to support multiple view-less providers in much the same way that multiple view-based context providers can be registered.  This menu allows the users to select between them.
 - There needs to be a way to get label information about each context provider.
 - This drop down menu should really be part of the breadcrumb, but for prototyping purposes it was easier to implement it as a menu.

P.S. This feature builds on the view-less flexible hierarchy viewer that I've been working on in bug 242489.
Comment 1 Pawel Piech CLA 2008-10-29 17:05:29 EDT
Created attachment 116476 [details]
Screenshot.
Comment 2 Pawel Piech CLA 2008-11-19 18:02:48 EST
Created attachment 118324 [details]
Updated patch.

This patch (which also includes changes for bug 242489), is a much better version of the feature.

+ Breadcrumb correctly follows the window selection and sets the selection.
+ Debugging without Debug view open is possible and it actually works quite well.
- There are too many refreshes in the breadcrumb when stepping.  This is partly due to the lazy label provider, and partly because of bugs in the view-less flexible hierarchy viewer.
+ I rewrote the breadcrumb viewer to be tree path based so it fits better with the flexible heirarchy viewer, but it also makes it more different than the breadcrumb in JDT.
+ The drop down popup is now positioned correctly depending on which side the coolbar is attached, and its height is sized correctly.
- The drop down width is still not sized correctly because of the lazy label updates in flexible hierarchy viewer.
+/- The popup shows the entire contents of the viewer rather than elements starting at the level where it was clicked.  I actually find that this makes the popup more useful, but perhaps it's a less-standard behavior for a breadcrumb... if there is such a thing.
+ The drop down selects the element that was clicked on by the user.  So the user can navigate from the location he selected.

Besides the -'s above, there are some other issues:

_Need to implement a context menu_
It should be easy to add a menu with object contributions, but should we also try to add view contributions?

_Should the user be able to manually select the active context provider?_
Currently this is the case using the temporary pull-down action, but I find that it can make the UI very confusing.  For example, the Debug view may be active, but the user can overrides the active provider and sets it to another provider.  On the other hand, what if someone wants to provide a default context provider which is not based on the debug view?  If the user cannot select a view-less provider manually, then there can only be one such provider.

_Can we use the breadcrumb within other views as a way to pin them?_
Technically this should be simple.  When a view is pinned, it creates a view-less context provider and registers it with the window service.  The usability problem is what happens when user leaves the pinned view?  Will the acive context be different than the one in debug view?
Comment 3 Pawel Piech CLA 2008-11-19 23:47:41 EST
Created attachment 118331 [details]
Updated screenshot.
Comment 4 Samantha Chan CLA 2008-11-20 11:12:57 EST
Also very interested in this :)
Comment 5 Darin Wright CLA 2008-11-20 16:37:45 EST
Perhaps its too soon to talk about implementation details, but API tooling notes that there are API leaks exposed in IDebugContextProviderExtension. Specifically it exposes non-API types IPresentationContext and IModelDelta. We wouldn't want to ship like this, so you might want to think about how it could be avoided.
Comment 6 Pawel Piech CLA 2008-11-20 17:18:47 EST
(In reply to comment #5)
I agree, in fact the whole implementation of the debug context breadcrumb
hinges on the fact that the context provider uses a flexible hierarchy viewer,
which is provisional.  

So I think we need to decide whether this is an acceptable limitation, then I
think we can make this new interface provisional as well.  Otherwise we'll need
to make the breadcrumb more generic and we'll probably need to create a whole
layer of new interfaces/extension points to support this.  Since currently
there is only one real debug context provider, the Debug view, I think it's OK
to have this restriction.  We could fail gracefully on the off chance that
someone out there really has their own debug context provider.
Comment 7 Pawel Piech CLA 2008-12-03 17:57:14 EST
Created attachment 119444 [details]
Patch with cleaner APIs.

I reworked the APIs to avoid polluting public interfaces with provisional ones.  To do this I created a new interface: IDebugContextSelectorFactory, which is used to create the control in the breadcrumb drop down.  The other up-shot of this change is that the control could be anything and not just a tree viewer.  E.g. PDP could use this interface to implement their icon-based viewer.

This change also fixes a major bug in the last patch, where the new context couldn't be selected if it wasn't previously revealed in the context provider's viewer.
Comment 8 Pawel Piech CLA 2008-12-12 13:07:43 EST
Created attachment 120351 [details]
Updated patch

I spent the last week trying to clean up the stability issues, I also added a context menu which turned out quite easy.  Also, I added a debug toolbar to be able to step through the code easily without the Debug view.  With all this, I think the breadcrumb is quite usable now and I plan to make a feature patch out of it as soon as 3.5M4 is finalized.

The following are the biggest remaining problems that I know of:
- On Linux (which I use) the context popup is not sized correctly.  I think this is a SWT issue (bug 258196), but for the moment it makes it rather difficult to see the stack frame being selected.  I think on Windows the popup sizing is fine, though I haven't tried it yet.
- The virtual viewer which tracks the debug model while the Debug view is closed is not yet "lazy".  This means, that for an application with a lot of threads and stack frames, the performance can be rather degraded.  
- The context menu does not pick up object contributions.  This really limits how useful the context menu can be, but I don't have an easy solution for it.  Basically, workbench requires a site (part, editor, etc.) to register a context menu, but the breadcrumb doesn't have a site, it only belongs to the window... so I think the only option is to ask for a new API.  Also, some context actions don't track their state right now.
- The breadcrumb and the debug view need to be invisible by default.  The breadcrumb is currently always visible, even in other perspectives.

That's all!  It's really not bad, so give it a try.  I myself haven't been using it much yet but I plan to try Debug-view less debugging for a while and see how it feels.
Comment 9 Curtis Windatt CLA 2008-12-16 11:03:02 EST
We should look at ways of making the variables view pop to the front.  I always need it when debugging, but it's completely useless in the Java perspective when I'm not.  We discussed an on demand popup var view at one point, but I can't see that being done for 3.5.
Comment 10 Pawel Piech CLA 2008-12-16 12:38:29 EST
(In reply to comment #9)
This should already be working.  You just need to add the Java perspective to the list of perspectives where view management is active (Preferences->Run/Debug->View Management).  

That said, I'm finding this to be a little flaky when trying to use it for PDE debugging.  Most of the time the views are activated only after I hit a breakpoint.  Also, when debugging it I noticed that the ViewContextService.contextActivated() handler ignores an empty selection context, so I'm not sure how it's supposed to decide that a given debug session is no longer active.
Comment 11 Pawel Piech CLA 2008-12-16 13:41:19 EST
To make debugging in other perspectives more friendly, we could also extend the viewContextBinding mechanism to selective enable action sets.  This way, a user working in a C/C++ or Java perspective, could have the debugging menu items and toolbars appear only when actively debugging.
Comment 12 Pawel Piech CLA 2008-12-16 19:55:48 EST
I've poked around the new customize perspective dialog to see what would be the best mechanism for controlling the visibility of this new feature, and I think the option with the best chances of success in 3.5 is to define a new action set, then control the crumb's visibility using this set as well.

Unfortunately control contributions to the toolbar are currently affected by a couple of bugs: bug 201589 and bug 259048.  There is a possible workaround for the former, but the latter could be a show stopper.
Comment 13 Pawel Piech CLA 2009-01-13 19:21:37 EST
Created attachment 122484 [details]
Patch with Breadcrumb in Debug view (compact mode).

Based on feedback from bug 260075, I redesigned the breadcrumb feature so that it works as a "compact" mode in Debug view.  When debug view is resized so that it's height is only as big as one line, then the Debug view automatically switches to showing the breadcrumb.

Moving the breadcrumb inside DV significantly simplifies the patch:
- no conroversial UI,
- there is no final APIs affected by this change,
- the VirtualTreeViewer (bug 242489) is not used, although some features introduced in that enhancment are still needed,
- full Debug View context menu is supported.
Comment 14 Darin Wright CLA 2009-01-13 21:58:50 EST
Thanks Pawel, I think this make the feature plays much better now (and is much less contraversial w.r.t UI guidelines)!

The auto-convert works nicely. As does detaching the view. I found that "Copy Stack" and "Find..." did not work as expected. Copy only seemed to copy one frame instead of all (even when the thread was selected), and "Find..." did not select the chosen element (but we should make sure these features are also working in the regular view).

As well, I noticed that  "No Active Context" is displayed when a launch terminates. However, you can then re-select the terminated launch via the drop down in the bread crumb control. Is there a reason you chose to show "no active context" instead of the terminated launch?

Comment 15 Pawel Piech CLA 2009-01-14 14:10:57 EST
(In reply to comment #14)
> Thanks Pawel, I think this make the feature plays much better now (and is much
> less contraversial w.r.t UI guidelines)!
Cool!  I also think that having the breadcrumb in the Debug view has some definite advantages.

> The auto-convert works nicely. As does detaching the view. I found that "Copy
> Stack" and "Find..." did not work as expected. Copy only seemed to copy one
> frame instead of all (even when the thread was selected), and "Find..." did not
> select the chosen element (but we should make sure these features are also
> working in the regular view).
I played with Find and it seems to work some of the time.  It also works only some of the time in the tree view.  I'll investigate this more closely.  I started looking at the Copy action and found that it uses the tree viewer selection directly as the root of the copy, rather than the active debug context.  I think this should be easy to fix.

> As well, I noticed that  "No Active Context" is displayed when a launch
> terminates. However, you can then re-select the terminated launch via the drop
> down in the bread crumb control. Is there a reason you chose to show "no active
> context" instead of the terminated launch?

The "No Active Context" item indicates that there is no selection in the view (i.e. the active debug context is empty).  When terminating a debug session, the currently selected element, usually a stack frame, is removed and no IModelDelta.SELECT is issued to replace it.  As a result the selection is lost.  I have a potential solution to this:  We could change the DefaultSelectionPolicy.replaceInvalidSelection() implementation to select the parent path instead of returning null as it does now.  Unfortunately this is not enough, because e.g. the parent of a stack frame is a thread, and its also invalid after termination.  But we could also change the InternalTreeModelViewer.handleInvalidSelection() to call the above replaceInvalidSelection() repeatedly until a valid or null selection is returned.  The possible negative side effect could be that some implementation may not want the parent element to be selected automatically.  Besides that I tried this solution and it works quite beautifully.

Another issue that I discovered, and which I'll investigate, is that run control key bindings are not active when the debug view drop-down is open. 

But overall, if you think this feature is 3.5 worthy, I will continue to test it, fix issues, and clean up the code.  The biggest question I have as far as the implementation side is how to deal with the virtual viewer features.  Like I said in my last comment, this breadcrumb actually does not depend on the virtual viewer itself, but it does depend on some of its features.  I would still really like to add the virtual viewer in 3.5, even if it's just in a test plugin, but I don't want to delay this feature since the virtual viewer itself adds a lot of code to review.  

Finally, there is also the SWT bug in gtk, which makes the breadcrumb rather annoying to use on Linux.  I filed bug 258196 and a patch, but it's been greeted with complete silence...  
Comment 16 Pawel Piech CLA 2009-01-20 12:45:49 EST
Over the last few days I've been testing and debugging the breadcrumb and I think I have all the issues, that were discovered so far, addressed:

_Find action_
The find action is actually broken for Debug view in general.  It does not take into consideration the selection policy.  The fix is trivial though.

_Copy action_
This is one was more troublesome.  The virtual copy action uses the tree viewer directly to extract the selection.  It then creates the virtual model based on that selection.  I had to create a new copy action just for the Debug view, which overrides the standard virtual copy, and in this overridden action I return a different selection if the breadcrumb is active.  The only down side was that I had to make one of the InternalTreeModelViewer package methods public (but not API... not even provisional).

_No Active Context after terminate_
This one was also a little tricky and I described the fix in the previous comment.  I've been testing it last few days and it works very well.

_Keyboard short-cuts stop working sometimes_
It turns out that keyboard shortcuts don't work if there is no control that has keyboard focus.  The problem with the breadcrumb is that if the breadcrumb item with focus was removed (like when a stack frame that was active was deleted), the keyboard focus in control was lost.  I added logic to always switch focus to the last breadcrumb item when the focused item is removed.  This also fixed many issues with the breadcrumb dropdown staying open too long. 

I think that's all that was still problematic.  I will work on reviewing cleaning up and commenting the patch and plan to commit it by the end of the week for M5.
Comment 17 Pawel Piech CLA 2009-01-20 13:05:30 EST
Created attachment 123106 [details]
Patch with all fixes.
Comment 18 Pawel Piech CLA 2009-01-21 00:24:20 EST
Created attachment 123162 [details]
Cleaned up patch for breadcrumb in Debug view.

I combed through the changes and removed dead code added missing comments, and I think it's ready to commit.  This patch combines fixes for this bug as well as bug 242489 on which this one depends.  The changes in this patch which could be controversial are the following:

1) IPresentationContext.PROPERTY_POPUP
The drop down viewer, which is re-created every time, needs to install model proxies for all elements.  If the model proxies are programmed to expand and select certain items upon installation, the drop-down viewer may behave erratically.  This property allows the model proxy to discover whether it is in a popup.  The patch also adds logic to the standard model proxies to check for this property.

2) Change in DefaultSelectionPolicy.replaceInvalidSelection()
This change prevents the debug view selection from being lost upon certain operation, e.g. terminate.  When the breadcrumb is visible, if the selection in the view is lost, the "No Active Context" text is shown, even though there is still content in the view, which can be misleading to the user.  The change is in the DefaultSelectionPolicy, which is used by many debuggers, so theoretically if someone really wanted there to be no selection in Debug view after terminate or target resume, this change would be undesirable.

3) Use of Display.asynExec() instead of UIJob 
This change is not strictly necessary for any functionality, but it does allow for easier testing of the tree model viewers.  The UIJob causes the viewer to depend on OSGI, which means that the viewer cannot be used in a simple java app.  Also, the simple Display.asyncExec() is actually faster because UIJob actually first executes a Job in a thread pool just to call Display.syncExec().  Since none of the UI jobs in the model viewer report any progress to the user, I can't think of any down side to this change.

4) Debug option: org.eclipse.debug.ui/debug/viewers/presentationId 
Often when debugging a view I get lost in traffic from other views.  This option allows me to narrow down which actual view (Debug, Variables, etc.) should be traced.  On the down side, it does make the trace statements a bit uglier.

5) Removed the public static debug flag from AbstractModelProxy (replaced by equivalent in ModelContentProvider)
This is a very minor change in a provisional API, but theoretically someone out there might be using it.  If it's a real concern, I could leave the flag in for backward compatibility.
Comment 19 Darin Wright CLA 2009-01-21 21:55:08 EST
updating milestone to 3.5 M5 (from 3.4)
Comment 20 Darin Wright CLA 2009-01-21 22:36:14 EST
I'm OK with these mentioned issues:

(2) Change in DefaultSelectionPolicy.replaceInvalidSelection()
(4) Debug option: org.eclipse.debug.ui/debug/viewers/presentationId 
(5) Removed the public static debug flag from AbstractModelProxy

Issues we should resolve:

(1) IPresentationContext.PROPERTY_POPUP

If other model proxies ignore this new property (which, by default, they will, since it did not previously exist), what happens? Does the bread crumb work. If not, it seems that we should find a workaround to this. If the bread crumb viewer is visible, can it just repsond differently to the deltas being fired by the proxies?

(3) Use of Display.asynExec() instead of UIJob 

The benefit of the UIJob is that they get canceled after a part/workbench is closed. The async's can attempt to run after their part or the workbench is closed/shutdown. Thus, async code typically has to check if it is OK to run first. We always had cases in the past of NPE's etc, since an async was running when its associated part/viewer no longer existed.

* ITreeModelViewer

This interface appears in the provisonal package, but I don't see it used here. It's use seems to be restricted to the implementation package. If that's the case, can we just move it to the internal package? The virtual viewer is provided as an alternate implementation to the tree model viewer (which is fine), but I'm not sure we need to expose the ITreeModelViewer.

* On WinXP, I see a bunch of errors as follows when I hold down the "step over" key:

org.eclipse.swt.SWTException: Invalid thread access
	at org.eclipse.swt.SWT.error(SWT.java:3828)
	at org.eclipse.swt.SWT.error(SWT.java:3743)
	at org.eclipse.swt.SWT.error(SWT.java:3714)
	at org.eclipse.swt.widgets.Widget.error(Widget.java:463)
	at org.eclipse.swt.widgets.Widget.checkWidget(Widget.java:355)
	at org.eclipse.swt.widgets.Control.setRedraw(Control.java:3124)
	at org.eclipse.debug.internal.ui.viewers.breadcrumb.BreadcrumbViewer.disableRedraw(BreadcrumbViewer.java:750)
	at org.eclipse.debug.internal.ui.viewers.breadcrumb.BreadcrumbViewer.internalRefresh(BreadcrumbViewer.java:401)
	at org.eclipse.jface.viewers.StructuredViewer$7.run(StructuredViewer.java:1430)
	at org.eclipse.jface.viewers.StructuredViewer.preservingSelection(StructuredViewer.java:1365)
	at org.eclipse.jface.viewers.StructuredViewer.preservingSelection(StructuredViewer.java:1328)
	at org.eclipse.jface.viewers.StructuredViewer.refresh(StructuredViewer.java:1428)
	at org.eclipse.jface.viewers.StructuredViewer.refresh(StructuredViewer.java:1387)
	at org.eclipse.debug.internal.ui.viewers.breadcrumb.AbstractBreadcrumb.refresh(AbstractBreadcrumb.java:127)
	at org.eclipse.debug.internal.ui.views.launch.LaunchViewBreadcrumb.labelUpdatesComplete(LaunchViewBreadcrumb.java:313)
	at org.eclipse.debug.internal.ui.viewers.model.TreeModelLabelProvider$3.run(TreeModelLabelProvider.java:364)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:37)
	at org.eclipse.debug.internal.ui.viewers.model.TreeModelLabelProvider.notifyUpdate(TreeModelLabelProvider.java:357)
	at org.eclipse.debug.internal.ui.viewers.model.TreeModelLabelProvider.updateComplete(TreeModelLabelProvider.java:348)
	at org.eclipse.debug.internal.ui.viewers.model.TreeModelLabelProvider.complete(TreeModelLabelProvider.java:276)
	at org.eclipse.debug.internal.ui.viewers.model.LabelUpdate.done(LabelUpdate.java:146)
	at org.eclipse.debug.internal.ui.model.elements.ElementLabelProvider$LabelUpdater.run(ElementLabelProvider.java:166)
	at org.eclipse.debug.internal.ui.model.elements.ElementLabelProvider$LabelJob.run(ElementLabelProvider.java:71)
	at org.eclipse.core.internal.jobs.Worker.run(Worker.java:55)


Comment 21 Pawel Piech CLA 2009-01-21 23:56:02 EST
(In reply to comment #20)
> I'm OK with these mentioned issues:
> <snip>
Cool :-)

> Issues we should resolve:
> 
> (1) IPresentationContext.PROPERTY_POPUP
> <snip>
The side effect of the delta's upon install, is that when the drop-down viewer is opened, the standard model proxy behavior is to expand/select the default thread/process/etc, as they normally would when the Debug view is first opened.  This is actually not the end of the world, but the desired behavior is to save and restore the state as the user configured it.  I could instead use a style flag to tell the content provider to essentially ignore the select/expand flags coming from the model.  The only down side to this approach would be that the drop-down would no longer respond to suspend events as the regular Debug view does.... which actually doesn't seem so bad.

> (3) Use of Display.asynExec() instead of UIJob 
> <snip>
This is a good point, which I haven't considered.  And on a quick glance I have not added the checks in the runnables to avoid NPEs after the viewer is disposed.  Personally I'm still not a fan of UIJobs, but I'll revert to them to keep things rolling smoothly :-)

> * ITreeModelViewer
> <snip>
I was intending to replace Viewer with ITreeModelViewer in IModelProxy.installed(Viewer) declaration.  This would give the proxy a more useful interface to work with, and avoid the temptation to cast the viewer to tree viewer and essentially mess with the internals of the model viewer.  However, it would be hasty to try to make this kind of change right so I'd be happy to put it off for a time when we look again at the flexible hierarchy API as a whole.  I.e. i'll move it to the internal package.

> * On WinXP, I see a bunch of errors as follows when I hold down the "step over"
> <snip>
I see where this is coming from.  I'll add a UIJob to handle this.  I'll try to do some testing on Windows tomorrow as well.
Comment 22 Pawel Piech CLA 2009-01-22 16:05:46 EST
Created attachment 123426 [details]
Patch which addresses the remaining issues.

I addressed the remaining issues raised in comment #20:

> (1) IPresentationContext.PROPERTY_POPUP
I got rid of this flag, and replaced it with a new SWT.POP_UP style flag to the viewer constructor.  When this flag is used, the viewer ignores select/expand events from the model.

> (3) Use of Display.asynExec() instead of UIJob 
Switched to UIJobs.

> * ITreeModelViewer
Moved into the internal package.  We can consider bringing it out in the future.

> * On WinXP, I see a bunch of errors as follows when I hold down the "step over"
I added a UIJob to guard for this.

Please let me know if there's anything else to be done.  Otherwise, I'll keep testing today and plan to commit tomorrow.
Comment 23 Darin Wright CLA 2009-01-22 16:53:31 EST
I will look at the latest patch again. Sam do you have any concerns?
Comment 24 Samantha Chan CLA 2009-01-22 17:02:51 EST
I have just started trying it out.. I will get back to you later tonite or early tomorrow.
Comment 25 Pawel Piech CLA 2009-01-22 17:37:58 EST
I just ran into an NPE in ModelContentProvider.modelChanged().  It was caused by a reference to a null fViewer.  I'll add a fix, but I won't keep reposting the patch unless anyone requests it.
Comment 26 Samantha Chan CLA 2009-01-22 18:36:33 EST
Hi Pawel,  

Thank you so much for the patch, I think it is a cool feature.

Here are some feedback after demo-ing this feature to my team:

1)  Resize to activate breadcrumbs - People liked this idea of activating breadcrumbs and thought it was cool.  Something to add is that it would be cool if there is a "Snap-To" feature for the breadcrumbs to activate.  It is sometimes difficult to resize to the right place to get it to activate just right, without going to0 small.  It will be great if we just drag all the way to the end, and snap to have the breadcrumb enabled.  In addition, we would like the breadcrumbs to have a minimum height, such that user cannot go pass that, and will always be able to see the bread crumbs.

In addition, we need a context menu / drop down menu to activate this feature such that the breadcrumb feature is accessible.

2)  We like that it's saving real-estate.

3)  We had a big discussion on the drop down menu, and what should show up in the drop down menu.  From what I understand, the drop down is a duplicated copy of the Debug view.  People have concerns with the following areas:

Why do all drop downs from the breadcrumbs show the same content?  The content shown in the drop-down menu is not sensitive to the selected item where the drop down is brought down from.  Each breadcrumb simply showed the same content in the drop down.  We think it will be much easier to navigate if the drop-down content is more context-sensitive.  For example, if the drop down is brought up from a stackframe, then we should only show siblings to the stackframes.  If the drop down is brought up from a thread, then we should only show siblings to the thread, and so on.

It is difficult to switch context with the drop down view.  The ordering of the content or selection  in the drop down doesn't know anything about what's selected in the bread-crumb.  In the case when we have many threads, or deep stacktrace, the user must hunt through the big tree in order to find what the need to switch to.

We wish there is a find item / filter field on the drop down (like the Open Java Type dialog), to allow us to find the item more easily.

It takes too many clicks to switch context, having a "Next Suspended Thtread" / "Last Suspended Thread" action will be good to allow us to go between suspended threads.  We also would like to have a short-cut key that allows us to switch context easily.

4) Losing important debug information when in breadcrumb mode.

We found that when debugging in the breadcrumb mode, the user has no way of knowing what is happening outside of the current debug context.  For example, while debugging something, if I launch another debug session, there is no feedback that another debug session has been launched.  We end up clicking the debug button over and over again.  

When debugging with threads, there is also no notification that another thread has been suspended, leaving the user to just sit there and wait for the current thread to execute.  The context also doesn't show thread monitors information nicely. 

We would like the breadcrumb to have the ability to provide feedback that something else has changed outside the current debug context and allow user to switch to the new context upon notification.

5)  Performance

We held down the step key to check out performance.  Stepping is noticeably slower after having breadcrumb installed.  This is especially true if there are multiple debug sessions.  I haven't looked at why, but stepping was much faster without breadcrumbs.

6)  If the hierarchy structure has changed, we lose active debug context.

For example, if you turn on thread groups while using breadcrumb, your debug context is lost.  Losing this in the tree viewer is bad, but not as bad in the breadcrumb scenario.  If the Debug view is in the tree viewer form, the user can at least see the suspended thread and stackframe.  This is not the case for breadcrumb mode, and can cause confusion.

7)  Is it possible for us to see multiple layers of breadcrumb.  For example, if I have two debug sessions, can I see them both in the Debug view in the breadcrumb mode?  

8)  Is the breadcrumb tied tightly with the debug view or is it portable.  For example, can I reuse it in the Memory View / Variables View or any other view?

9)  Switching context to thread / target is confusing

In this scenario, the user has suspended on a stackframe, instructional pointer updated in the editor.  Now the user switches to another thread, that is either running / suspended.  The instructional pointer stays in the editor, but it has nothing to do with the current breadcrumb selection.  The user gets this false impression that the current selection in the editor has something to do with the breadcrumb selection.

Secondly, after switching to a thread, it is not clear to the user that the thread actually may contain children, and can potentially switch to a stackframe.  Is it possible to add another drop down menu at the end if the current selection have children?  This gives the user some indication that they are not at the leaf of the tree structure, and further drill down is possible.

10)  Invoking Find action resulted in exception.   

java.lang.NullPointerException
at org.eclipse.debug.internal.ui.viewers.model.InternalTreeModelViewer$VirtualLabelUpdate.access$1(InternalTreeModelViewer.java:926)
at org.eclipse.debug.internal.ui.viewers.model.InternalTreeModelViewer$VirtualElement.getLabel(InternalTreeModelViewer.java:558)
at org.eclipse.debug.internal.ui.viewers.model.VirtualFindAction$FindLabelProvider.getText(VirtualFindAction.java:59)
at org.eclipse.ui.dialogs.FilteredList.setElements(FilteredList.java:278)
at org.eclipse.ui.dialogs.AbstractElementListSelectionDialog.setListElements(AbstractElementListSelectionDialog.java:178)
at org.eclipse.ui.dialogs.ElementListSelectionDialog.createDialogArea(ElementListSelectionDialog.java:64)
at org.eclipse.debug.internal.ui.viewers.FindElementDialog.createDialogArea(FindElementDialog.java:46)
at org.eclipse.jface.dialogs.Dialog.createContents(Dialog.java:760)
at org.eclipse.jface.window.Window.create(Window.java:431)
at org.eclipse.jface.dialogs.Dialog.create(Dialog.java:1089)
at org.eclipse.ui.dialogs.SelectionStatusDialog.create(SelectionStatusDialog.java:153)
at org.eclipse.ui.dialogs.AbstractElementListSelectionDialog.access$superCreate(AbstractElementListSelectionDialog.java:427)
at org.eclipse.ui.dialogs.AbstractElementListSelectionDialog.access$2(AbstractElementListSelectionDialog.java:426)
at org.eclipse.ui.dialogs.AbstractElementListSelectionDialog$4.run(AbstractElementListSelectionDialog.java:438)
at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70)
at org.eclipse.ui.dialogs.AbstractElementListSelectionDialog.create(AbstractElementListSelectionDialog.java:436)
at org.eclipse.jface.window.Window.open(Window.java:790)
at org.eclipse.ui.dialogs.AbstractElementListSelectionDialog.open(AbstractElementListSelectionDialog.java:422)
at org.eclipse.debug.internal.ui.viewers.model.VirtualFindAction.performFind(VirtualFindAction.java:131)
at org.eclipse.debug.internal.ui.viewers.model.VirtualFindAction.run(VirtualFindAction.java:105)
at org.eclipse.jface.action.Action.runWithEvent(Action.java:498)
at org.eclipse.jface.commands.ActionHandler.execute(ActionHandler.java:119)
at org.eclipse.core.commands.Command.executeWithChecks(Command.java:476)
at org.eclipse.core.commands.ParameterizedCommand.executeWithChecks(ParameterizedCommand.java:508)
at org.eclipse.ui.internal.handlers.HandlerService.executeCommand(HandlerService.java:169)
at org.eclipse.ui.internal.keys.WorkbenchKeyboard.executeCommand(WorkbenchKeyboard.java:471)
at org.eclipse.ui.internal.keys.WorkbenchKeyboard.press(WorkbenchKeyboard.java:823)
at org.eclipse.ui.internal.keys.WorkbenchKeyboard.processKeyEvent(WorkbenchKeyboard.java:879)
at org.eclipse.ui.internal.keys.WorkbenchKeyboard.filterKeySequenceBindings(WorkbenchKeyboard.java:570)
at org.eclipse.ui.internal.keys.WorkbenchKeyboard.access$3(WorkbenchKeyboard.java:511)
at org.eclipse.ui.internal.keys.WorkbenchKeyboard$KeyDownFilter.handleEvent(WorkbenchKeyboard.java:126)
at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
at org.eclipse.swt.widgets.Display.filterEvent(Display.java:1190)
at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1002)
at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1016)
at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1012)
at org.eclipse.swt.widgets.Widget.sendKeyEvent(Widget.java:1040)
at org.eclipse.swt.widgets.Widget.sendKeyEvent(Widget.java:1036)
at org.eclipse.swt.widgets.Widget.wmChar(Widget.java:1358)
at org.eclipse.swt.widgets.Control.WM_CHAR(Control.java:4019)
at org.eclipse.swt.widgets.Control.windowProc(Control.java:3912)
at org.eclipse.swt.widgets.Display.windowProc(Display.java:4584)
at org.eclipse.swt.internal.win32.OS.DispatchMessageW(Native Method)
at org.eclipse.swt.internal.win32.OS.DispatchMessage(OS.java:2392)
at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3468)
at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2384)
at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2348)
at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2200)
at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:495)
at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:333)
at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:490)
at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149)
at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:113)
at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:193)
at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110)
at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79)
at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:366)
at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:177)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:37)
at java.lang.reflect.Method.invoke(Method.java:599)
at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:550)
at org.eclipse.equinox.launcher.Main.basicRun(Main.java:505)
at org.eclipse.equinox.launcher.Main.run(Main.java:1237)
at org.eclipse.equinox.launcher.Main.main(Main.java:1213)
Comment 27 Pawel Piech CLA 2009-01-22 20:13:04 EST
(In reply to comment #26)
> Hi Pawel,  
> 
> Thank you so much for the patch, I think it is a cool feature.
> 
> Here are some feedback after demo-ing this feature to my team:
> 
> 1)  Resize to activate breadcrumbs - People liked this idea of activating
> breadcrumbs and thought it was cool.  Something to add is that it would be cool
> if there is a "Snap-To" feature for the breadcrumbs to activate. 
> <snip>
I agree that getting the Debug view size correctly is a little tricky.  I think I can implement the snap feature pretty easily.

> In addition, we need a context menu / drop down menu to activate this feature
> such that the breadcrumb feature is accessible.
Didn't think of that.  Will do.

> 2)  We like that it's saving real-estate.
:-) Having used it last couple of weeks I can happily say that I can now debug in the Java perspective.

> 3)  We had a big discussion on the drop down menu, and what should show up in
> the drop down menu.  From what I understand, the drop down is a duplicated copy
> of the Debug view.  People have concerns with the following areas:
> 
> Why do all drop downs from the breadcrumbs show the same content?  The content
> shown in the drop-down menu is not sensitive to the selected item where the
> drop down is brought down from.  Each breadcrumb simply showed the same content
> in the drop down.  We think it will be much easier to navigate if the drop-down
> content is more context-sensitive.  For example, if the drop down is brought up
> from a stackframe, then we should only show siblings to the stackframes.  If
> the drop down is brought up from a thread, then we should only show siblings to
> the thread, and so on.
I thought about this quite a bit as well.  Showing sub-portion of the model in the drop down viewer would require a lot more work, though it's not impossible.  The much simpler solution which I did implement, but is maybe to subtle to notice is that the item that you clicked on is the one that gets selected in the viewer.  Also, I actually thought that it's an asset to show the whole tree because it allows me to see the larger context.  I've been trying to use the JDT editor breadcrumb, and I've pretty much given up on it because I find it too cumbersome to go up in the hierarchy.  That said, there is a case for showing just the siblings of the selected item, and I think it would be a good extension to this feature.

> It is difficult to switch context with the drop down view.  The ordering of the
> content or selection  in the drop down doesn't know anything about what's
> selected in the bread-crumb.  In the case when we have many threads, or deep
> stacktrace, the user must hunt through the big tree in order to find what the
> need to switch to.
Hmm, the breadcrumb selection is used to set the initial selection in the drop-down.  This can be obscured by the user if he immediately mouses over the drop-down... maybe this was happening in your testing.  

> We wish there is a find item / filter field on the drop down (like the Open
> Java Type dialog), to allow us to find the item more easily.
Whatever we implement for the tree viewer in the Debug view will be easy to apply to the drop down as well.  The find that's in the viewer right now is essentially a selection dialog.  Do you think we should enable the find just for the drop down?  I'm a little worried that opening a modal dialog for the drop down may be pushing the boundary of a pop-up window behavior.  But the UI guys would know better.

> It takes too many clicks to switch context, having a "Next Suspended Thtread" /
> "Last Suspended Thread" action will be good to allow us to go between suspended
> threads.  We also would like to have a short-cut key that allows us to switch
> context easily.
Yes!  I think this is an excellent idea (as a JDT user), and it really applies to Debug view in general.  

> 4) Losing important debug information when in breadcrumb mode.
> 
> We found that when debugging in the breadcrumb mode, the user has no way of
> knowing what is happening outside of the current debug context.  For example,
> while debugging something, if I launch another debug session, there is no
> feedback that another debug session has been launched.  We end up clicking the
> debug button over and over again.  
> 
> When debugging with threads, there is also no notification that another thread
> has been suspended, leaving the user to just sit there and wait for the current
> thread to execute.  The context also doesn't show thread monitors information
> nicely. 
> 
> We would like the breadcrumb to have the ability to provide feedback that
> something else has changed outside the current debug context and allow user to
> switch to the new context upon notification.

I agree that the breadcrumb isn't quite the substitute for the full tree view, and some things e.g. the Java monitors are really designed for the tree view, so they don't work well in the breadcrumb.  In the end, we may not be able to completely compensate for the fact that we're using just one line.

I also find the lack of feedback upon starting a new debug session as particularly troublesome.  I could easily add a visual indicator that tree view content has changed, but the viewer doesn't have visibility into the model, so I couldn't distinguish between say stepping the current thread and a new launch being added.   Also, if there's any hope of moving the breadcrumb to other views as well, this kind of notification will need to travel over public APIs, which would add to their clutter.  So I'm afraid I don't have any good solutions to this problem at the moment.  Any concrete suggestions would be most helpful.

> 5)  Performance
> 
> We held down the step key to check out performance.  Stepping is noticeably
> slower after having breadcrumb installed.  This is especially true if there are
> multiple debug sessions.  I haven't looked at why, but stepping was much faster
> without breadcrumbs.
I found a mix here.  Stepping something simple like PDA seemed just as performant.  But stepping through something with a lot of files did seem slower for me.  I think it's because the breadcrumb recalculates its layout when any of the items is updated.  I think I could improve this a little by checking when the labels actually change.  But in the end if the thread name and the stack frame names change a lot, the breadcrumb will be slower.

> 6)  If the hierarchy structure has changed, we lose active debug context.
> 
> For example, if you turn on thread groups while using breadcrumb, your debug
> context is lost.  Losing this in the tree viewer is bad, but not as bad in the
> breadcrumb scenario.  If the Debug view is in the tree viewer form, the user
> can at least see the suspended thread and stackframe.  This is not the case for
> breadcrumb mode, and can cause confusion.
I didn't see this happen so much anymore with the latest patch.  I added logic in the default selection policy to select the parent context when the current selection becomes invalid.  For example if I select a system thread, and then change to not show system threads, the debug target is now selected.

> 7)  Is it possible for us to see multiple layers of breadcrumb.  For example,
> if I have two debug sessions, can I see them both in the Debug view in the
> breadcrumb mode?  
This would require some major new API, because the breadcrumb is focused on the active debug context.  But a simple solution could be to allow multiple Debug views to be open at the same time?

> 8)  Is the breadcrumb tied tightly with the debug view or is it portable.  For
> example, can I reuse it in the Memory View / Variables View or any other view?
This is really what I'm shooting for with this feature.  In our product the breadcrumb in Debug view will be nice, but what we really need is help with multi-context debugging.  I plan to keep working on the breadcrumb to add it to other views.  It will be too late for 3.5 though :-(

> 9)  Switching context to thread / target is confusing
> 
> In this scenario, the user has suspended on a stackframe, instructional pointer
> updated in the editor.  Now the user switches to another thread, that is either
> running / suspended.  The instructional pointer stays in the editor, but it has
> nothing to do with the current breadcrumb selection.  The user gets this false
> impression that the current selection in the editor has something to do with
> the breadcrumb selection.
I think this could be remedied in the source locator service.  Currently we erase the IP if another stack frame is selected.  We could also erase if just another thread is selected.

> Secondly, after switching to a thread, it is not clear to the user that the
> thread actually may contain children, and can potentially switch to a
> stackframe.  Is it possible to add another drop down menu at the end if the
> current selection have children?  This gives the user some indication that they
> are not at the leaf of the tree structure, and further drill down is possible.
I agree, but this will be a harder problem to deal with.  I designed the breadcrumb to work based on the active debug context so that we could more easily move it to other views, but the debug context API doesn't have a "has children" flag.  

> 10)  Invoking Find action resulted in exception.   
I've never seen this this exception.  But I can easily add a null pointer check in the suspect location.


Thank you very much for all this feedback.  If I understand it correctly though, you agree that the breadcrumb is already valuable enough as it is to commit and we can track the issues above as future enhancments.
Comment 28 Darin Wright CLA 2009-01-23 12:08:41 EST
I propose that this feature should be released for M5 and advertised as "new & noteworthy". During M6/M7 the feature should be polished and we should respond to issues reported by users. I think the following features should be considered in order to ship with 3.5:

* A way to turn the bread crumb off (a preference to turn auto-bread crumb off for users that don't want it)
* A "snap" action that creates a debug view in the bread crumb state with the correct height
* Navigator popups that show siblings rather than the entire debug hierarhcy (perhaps feedback from users will help guide this).

I have versioned the debug plug-ins in preparation for releasing the feature to HEAD, but I would like to ensure that we have Sam's agreement before proceeding. I know there are issues in complex debugging scenarios (multi-threaded, etc). However, I believe the value of this feature is for simple debugging scenarios (single thread). For complex debugging scenarios, perhaps the debug view works best (currently), and this feature can be evolved to assist more complex scenarios in the future.

Pawel, will you have time to address bugs/polish issues during the remainder of the 3.5 cycle, as they arise?
Comment 29 Pawel Piech CLA 2009-01-23 12:29:11 EST
(In reply to comment #28)
> Pawel, will you have time to address bugs/polish issues during the remainder of
> the 3.5 cycle, as they arise?
Yes I will.  I also agree that the above issues should be considered and they could all be addressed safely (no new API). 

I also agree with all the other points that Samantha raised, but I think they'll be more difficult to address and should probably be targeted for future releases.
Comment 30 Pawel Piech CLA 2009-01-23 13:24:12 EST
Created attachment 123572 [details]
New and Noteworthy
Comment 31 Samantha Chan CLA 2009-01-23 13:27:48 EST
Thank you Pawel and Darin.

I am trying to make sure that the list of things to be addressed in 3.5 is sufficient for us.  Let me take a look at this again, and I will get back to you this afternoon.
Comment 32 Pawel Piech CLA 2009-01-23 15:17:14 EST
(In reply to comment #26)
> When debugging with threads, there is also no notification that another thread
> has been suspended, leaving the user to just sit there and wait for the current
> thread to execute....

I think there may be enough information in the current APIs to let us address this problem as well:  Whenever a new thread is suspended, (or a program is launched) it usually generates a IModelDelta.SELECT event, but if a stack frame is already selected, this select request is suppressed.  We could detect when this happens and have a brief popup show up in the Debug view to notify the user of the event.  I'm thinking that the UI could be something similar the mail inbox notifications... and it would also be useful when working with the tree view not just the breadcrumb, since this problem really applies to Debug view in general.  Also, I suspect this would be considered a "major" feature and not likely in 3.5.
Comment 33 Samantha Chan CLA 2009-01-23 17:13:25 EST
I looked through the issues again.  I agree the feature provide value to debugging simple applications.  I agree that the feature should be released if we can get a commitment that the following issues, in addition to those listed by Darin, are addressed in 3.5 timeframe:  

* Performance - I saw noticeable performance degradation with the breadcrumb installed, and running in normal Debug view.  We need to clarify if the breadcrumbs are not visible, whether it will still process the debug events.  If the breadcrumbs are active even when they are hidden, then we have a feature request to completely disable the breadcrumbs if they are not visible.  (e.g. if the user has turned off the breadcrumb feature via a preference, we do not want the breadcrumb feature to interfere with Debug view performance.)  We need a commitment that performance analysis will be performed, and any performance defects found must be addressed.

* Scalability - The navigational drop down must scale, and can handle deep stacktrace, or many threads.  The feature may be not usable in complex multi-threaded environment, but it has to be able to support it.

* Ability to show notifications that something has changed outside of the current debug context.  By default, it can be as simple as showing something has changed.  In addition, the model should be able to provide more detailed information if it knows what's changed in the model.  (e.g. a new thread, a new launch, etc.)

* Shortcut keys / actions to switch context quickly.

* Docs - We need someone to write docs for this.

Pawel, you mentioned that the notification feature may be too big, but from our point of view, launching and getting feedback from new launches is part of the core scenario.  The platform must behave flawlessly in these scenarios.  That's why we would like to have this problem addressed.

Darin, do we have your approvals to address these issues in 3.5?

Thanks for all the hard work,
Samantha
Comment 34 Pawel Piech CLA 2009-01-23 17:42:31 EST
(In reply to comment #33)
> I looked through the issues again.  I agree the feature provide value to
> debugging simple applications.  I agree that the feature should be released if
> we can get a commitment that the following issues, in addition to those listed
> by Darin, are addressed in 3.5 timeframe:  
I see that you're intent on keeping 3.5 interesting :-)

> * Performance - I saw noticeable performance degradation with the breadcrumb
> installed, and running in normal Debug view.  We need to clarify if the
> breadcrumbs are not visible, whether it will still process the debug events. 
> If the breadcrumbs are active even when they are hidden, then we have a feature
> request to completely disable the breadcrumbs if they are not visible.  (e.g.
> if the user has turned off the breadcrumb feature via a preference, we do not
> want the breadcrumb feature to interfere with Debug view performance.)  
I guess I didn't even think about it, but yes currently the breadcrumb viewer does remain active (listening to events and refreshing) even when not visible.  This should be trivial to address and there's also other optimizations I can try to improve performance while the breadcrumb is visible.  

> We need
> a commitment that performance analysis will be performed, and any performance
> defects found must be addressed.
I wish I knew more about the existing performance tests.  Would any of them be helpful here?  I've done a lot of manual testing in the past to maximize stepping performance in DSF, but I'm not sure what would be acceptable as a performance analysis here.

> * Scalability - The navigational drop down must scale, and can handle deep
> stacktrace, or many threads.  The feature may be not usable in complex
> multi-threaded environment, but it has to be able to support it.
The drop down is based on the same viewer as the Debug view tree view, so it should have the same scalability (i.e. limited by SWT, which allocates arrays matching number of expanded elements in the tree).  I've been testing with the ManyThreads stress tests example and I haven't seen any issues there.  Have you noticed yourself any issues with drop down scalability?

> * Ability to show notifications that something has changed outside of the
> current debug context.  By default, it can be as simple as showing something
> has changed.  In addition, the model should be able to provide more detailed
> information if it knows what's changed in the model.  (e.g. a new thread, a new
> launch, etc.)
This is definitely the fun one :-). 

> * Shortcut keys / actions to switch context quickly.
This would need probably follow from whatever solution we decide on notifications.

> * Docs - We need someone to write docs for this.
I think I should be able to handle this as well.
Comment 35 Samantha Chan CLA 2009-01-23 17:55:35 EST
(In reply to comment #34)
> I see that you're intent on keeping 3.5 interesting :-)
Yup... :)  Always looking for adventures... 

Performance:
We typically use Yourkit to measure performance, and target specific areas where I am looking performance problems.  All I am looking for here is that we are doing comparisons between having breadcrumbs vs not having it, and make sure that the breadcrumb feature does not cause "noticable" performance degradations.  (The performance degradation with stepping is noticable in the UI now.)

Existing performance tests cannot test UI performance (not that I know of).  I think we have to test this manually.

Scalability:
Yes, I have seen issues with having deep stack trace, and many threads... The UI sometimes hangs for a few seconds.  I will try to open defects as I see them.

Thanks again Pawel.  
Comment 36 Darin Wright CLA 2009-01-23 17:58:58 EST
OK - looks like the consensus is to commit the new feature, work on polish/performance and other issues that arise.
Comment 37 Pawel Piech CLA 2009-01-23 18:34:29 EST
I committed the fix as in the last patch, plus fixes for the last two NPEs found.  Darin please review.
Comment 38 Pawel Piech CLA 2009-01-23 19:26:46 EST
I created following bugs for follow up work:
bug 262260 - [breadcrumb] Debug view should show notifications of model changes when in Breadcrumb mode,
bug 262261 - [breadcrumb] Add actions to Debug view to switch between tree view, breadcrumb, or automatic modes.
bug 262262 - [breadcrumb] Add actions to quickly switch between contexts in Debug view.
bug 262263 - [breadcrumb] Debug view breadcrumb should not impact Debug view performance when not visible.
bug 262265 - [breadcrumb] Add user documentation for the Debug view breadcrumb.
bug 262266 - [breadcrumb] In the Debug view breadcrumb drop-down add a mode to show only selected element's siblings.
bug 262267 - [source lookup] When selecting a new thread or target in debug view, the IP in editor should be erased.
bug 262268 - [breadcrumb] Add sticky behavior to automatic breadcrumb activation in Debug view

Did I miss anything?  One area I wasn't sure of is the breadcrumb mode activation actions.  Should the breadcrumb be a preference?  Or should it be a layout mode selected from the view-menu (like the detail pane in variables view)?  I like the idea of "snapping" the view to the correct size, but I don't think there's an API for that.  Anyway we can have this discussion in bug 262261.

Thank you all for all the feedback and support... and for approving the feature for 3.5 :-D
Comment 39 Darin Wright CLA 2009-05-15 09:48:00 EDT
Verified.