Bug 384460 - Provide Breakpoint Serialization/Deserialization API
Summary: Provide Breakpoint Serialization/Deserialization API
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Debug (show other bugs)
Version: 4.2   Edit
Hardware: PC Linux
: P3 enhancement (vote)
Target Milestone: 4.3 M1   Edit
Assignee: Pawel Piech CLA
QA Contact:
URL:
Whiteboard:
Keywords: api
Depends on:
Blocks: 155333
  Show dependency tree
 
Reported: 2012-07-06 07:48 EDT by Sebastian Schmidt CLA
Modified: 2012-07-31 09:21 EDT (History)
7 users (show)

See Also:


Attachments
Patch (21.79 KB, patch)
2012-07-21 12:08 EDT, Sebastian Schmidt CLA
no flags Details | Diff
mylyn/context/zip (539 bytes, application/octet-stream)
2012-07-21 12:08 EDT, Sebastian Schmidt CLA
no flags Details
Updated Patch #1 (21.79 KB, patch)
2012-07-23 12:56 EDT, Sebastian Schmidt CLA
no flags Details | Diff
Fix for "Show qualified names" in breakpoint export/import wizard. (1.22 KB, patch)
2012-07-23 14:24 EDT, Pawel Piech CLA
no flags Details | Diff
Decorator to add breakpoint paths to preview (4.32 KB, patch)
2012-07-30 14:40 EDT, Sebastian Schmidt CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sebastian Schmidt CLA 2012-07-06 07:48:37 EDT
I'm searching for an API to serialize/deserialize breakpoints to a generic format in order to attach them to Mylyn context. There is related code in ImportBreakpointsAction/ExportBreakpointsAction which fits to this need (i.e. serializes breakpoints to a xml file / deserializes them). 

Unfortunately, I can't find a way to do "just" a deserialization (i.e. XML to IBreakpoint conversion) - without directly importing the found breakpoints to BreakpointManager, checking for valid markers, ...

Is there a more suitable API to achieve this? Otherwise, would it be a  reasonable approach to provide a patch for moving code from Import/Export-Actions to a new API?
Comment 1 Pawel Piech CLA 2012-07-17 14:21:55 EDT
The ImportBreakpointsOperation/ExportBreakpointsOperation classes allow you to serialize breakpoints to/from a string buffer.  

If it's not what you need, then I'm afraid I don't understand the request.  BTW, you cannot deserialize a string into breakpoint objects without creating the corresponding markers.  You could import then into a property map I suppose, but I don't see how that would be useful.
Comment 2 Sebastian Schmidt CLA 2012-07-18 08:08:47 EDT
(In reply to comment #1)
> The ImportBreakpointsOperation/ExportBreakpointsOperation classes allow you to
> serialize breakpoints to/from a string buffer.  

Agreed. But if you use ImportBreakpointsOperation to restore breakpoints from a XML (in a StringBuffer), it

1. parses them into IBreakpoint objects
2. adds them to IBreakpointManager, thus adds them to the debug breakpoints view.

I'd like to be able to "just" parse the XML to IBreakpoint objects in order to display them in a custom view - without adding them to my workbench. In the end it would help me to just prevent the fManager.addBreakpoints() call in ImportBreakpointsOperation from happening.

Here is the use case: We export breakpoints that are used during an activated Mylyn tasks as part of the context (i.e. attaching the result of an ExportBreakpointsOperation to Mylyn's context file). Than, Mylyn offers to show the context of any task in a custom CNF-based view. 
In this view I want to be able to show breakpoints related to a context by restoring them from the attached breakpoints XML. But without adding them to the users workbench.
Comment 3 Pawel Piech CLA 2012-07-18 12:22:21 EDT
(In reply to comment #2)
> In this view I want to be able to show breakpoints related to a context by
> restoring them from the attached breakpoints XML. But without adding them to
> the users workbench.

Creating the breakpoints (and markers) without registering them with the breakpoint manager is not something that we usually do and could introduce some instability.  For example,  as soon as you create the marker the breakpoint will show up in the editor.  If you don't dispose that breakpoint, the marker in editor will remain.  Also, the label provider for the breakpoint will be called with the unregistered breakpoint but it could make an assumption that the breakpoint should be registered resulting in NPEs.  

For this same reason the import breakpoint dialog does not "preview" the breakpoints that are to be imported, which would have been a nice feature.  I didn't implement the import/export breakpoint feature though, so maybe Mike has some better insights.
Comment 4 Michael Rennie CLA 2012-07-18 15:53:46 EDT
> (In reply to comment #2)
> In this view I want to be able to show breakpoints related to a context by
> restoring them from the attached breakpoints XML. But without adding them to
> the users workbench.

That makes sense, but the original feature was always intended to simply restore breakpoints 'as-is', since all we had to go on was what was in the XML file describing them. I could see adding another constructor that would allow a consumer to specify if they wanted markers restored / the breakpoints registered.

In the interim you could always sanitize the XML to remove the 'registered' entry, which would have the same net effect. You could also register an IBreakpointImportParticipant that will be consulted after the breakpoint has been re-created. You could unregister it then - see ImportBreakpointOperation#rereateBreakpoint(..) line 305.

(In reply to comment #3)
> 
> Creating the breakpoints (and markers) without registering them with the
> breakpoint manager is not something that we usually do and could introduce some
> instability.  For example,  as soon as you create the marker the breakpoint
> will show up in the editor.  If you don't dispose that breakpoint, the marker
> in editor will remain.  Also, the label provider for the breakpoint will be
> called with the unregistered breakpoint but it could make an assumption that
> the breakpoint should be registered resulting in NPEs.  
> 

We have always intended for breakpoints to be registered (or not) see IBreakpoint#REGISTERED for more details - the Java debugger makes use of unregistered (invisible) breakpoints for some of our features like run to line and suspend on uncaught exceptions - but we do create a marker for them on the workspace root, which happens to not show up (see RunToLineAdapter in JDT debug for more info). You also can create a breakpoint object without a marker, see IBreakpoint#setMarker. Like Pawel mentioned though, unregistered / no marker breakpoints could cause problems depending on what you want to do with them.

> For this same reason the import breakpoint dialog does not "preview" the
> breakpoints that are to be imported, which would have been a nice feature.  

That's not entirely true, we could create bp objects and fake up items in a list, there has just not been any time to do so :)
Comment 5 Sebastian Schmidt CLA 2012-07-21 12:08:14 EDT
Created attachment 219020 [details]
Patch
Comment 6 Sebastian Schmidt CLA 2012-07-21 12:08:17 EDT
Created attachment 219021 [details]
mylyn/context/zip
Comment 7 Sebastian Schmidt CLA 2012-07-21 12:08:29 EDT
(In reply to comment #4) 
> I could see adding another constructor that would allow a
> consumer to specify if they wanted markers restored / the breakpoints
> registered.

I created a patch for that.

> > For this same reason the import breakpoint dialog does not "preview" the
> > breakpoints that are to be imported, which would have been a nice feature.
> 
> That's not entirely true, we could create bp objects and fake up items in a
> list, there has just not been any time to do so :)

I added an additional page to the import wizard allowing users to select which breakpoints they would like to import from a bkpt file. Also included in the Patch. Unfortunately, I wasn't able to find the platform / debug Gerrit path. Thus, I attached the patch to this bug. Is the project registered with Gerrit?
Comment 8 Pawel Piech CLA 2012-07-23 12:04:27 EDT
Thank you Sebastian.  The patch looks good to commit, I just need to figure out what should be the new version of debug plugins since the patch makes an API addition.
Comment 9 Michael Rennie CLA 2012-07-23 12:34:20 EDT
(In reply to comment #8)
> Thank you Sebastian.  The patch looks good to commit, I just need to figure out
> what should be the new version of debug plugins since the patch makes an API
> addition.

3.9.0, a non-breaking API change has been made: http://wiki.eclipse.org/Version_Numbering#When_to_change_the_minor_segment
Comment 10 Michael Rennie CLA 2012-07-23 12:38:28 EDT
Testing the patch I get:

java.lang.reflect.InvocationTargetException
	at org.eclipse.jface.operation.ModalContext.runInCurrentThread(ModalContext.java:477)
	at org.eclipse.jface.operation.ModalContext.run(ModalContext.java:372)
	at org.eclipse.jface.wizard.WizardDialog.run(WizardDialog.java:1028)
	at org.eclipse.debug.internal.ui.importexport.breakpoints.WizardImportBreakpointsPage.finish(WizardImportBreakpointsPage.java:214)
	at org.eclipse.debug.internal.ui.importexport.breakpoints.WizardImportBreakpoints.performFinish(WizardImportBreakpoints.java:97)
	at org.eclipse.jface.wizard.WizardDialog.finishPressed(WizardDialog.java:827)
	at org.eclipse.jface.wizard.WizardDialog.buttonPressed(WizardDialog.java:432)
	at org.eclipse.jface.dialogs.Dialog$2.widgetSelected(Dialog.java:624)
	at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:248)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1053)
	at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4169)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3758)
	at org.eclipse.jface.window.Window.runEventLoop(Window.java:825)
	at org.eclipse.jface.window.Window.open(Window.java:801)
	at org.eclipse.debug.internal.ui.importexport.breakpoints.ImportBreakpoints.run(ImportBreakpoints.java:45)
	at org.eclipse.debug.internal.ui.actions.AbstractDebugActionDelegate.runWithEvent(AbstractDebugActionDelegate.java:321)
	at org.eclipse.ui.internal.PluginAction.runWithEvent(PluginAction.java:241)
	at org.eclipse.jface.action.ActionContributionItem.handleWidgetSelection(ActionContributionItem.java:584)
	at org.eclipse.jface.action.ActionContributionItem.access$2(ActionContributionItem.java:501)
	at org.eclipse.jface.action.ActionContributionItem$5.handleEvent(ActionContributionItem.java:411)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1053)
	at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4169)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3758)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$9.run(PartRenderingEngine.java:1031)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:925)
	at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:86)
	at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:585)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:540)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:124)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196)
	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:353)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:180)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:601)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:629)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:584)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1438)
	at org.eclipse.equinox.launcher.Main.main(Main.java:1414)
Caused by: java.lang.ArrayIndexOutOfBoundsException: 6
	at org.eclipse.debug.internal.ui.importexport.breakpoints.WizardImportBreakpointsPage$1.removeUncheckedBreakpoints(WizardImportBreakpointsPage.java:233)
	at org.eclipse.debug.internal.ui.importexport.breakpoints.WizardImportBreakpointsPage$1.run(WizardImportBreakpointsPage.java:223)
	at org.eclipse.jface.operation.ModalContext.runInCurrentThread(ModalContext.java:464)
	... 46 more
Root exception:
java.lang.ArrayIndexOutOfBoundsException: 6
	at org.eclipse.debug.internal.ui.importexport.breakpoints.WizardImportBreakpointsPage$1.removeUncheckedBreakpoints(WizardImportBreakpointsPage.java:233)
	at org.eclipse.debug.internal.ui.importexport.breakpoints.WizardImportBreakpointsPage$1.run(WizardImportBreakpointsPage.java:223)
	at org.eclipse.jface.operation.ModalContext.runInCurrentThread(ModalContext.java:464)
	at org.eclipse.jface.operation.ModalContext.run(ModalContext.java:372)
	at org.eclipse.jface.wizard.WizardDialog.run(WizardDialog.java:1028)
	at org.eclipse.debug.internal.ui.importexport.breakpoints.WizardImportBreakpointsPage.finish(WizardImportBreakpointsPage.java:214)
	at org.eclipse.debug.internal.ui.importexport.breakpoints.WizardImportBreakpoints.performFinish(WizardImportBreakpoints.java:97)
	at org.eclipse.jface.wizard.WizardDialog.finishPressed(WizardDialog.java:827)
	at org.eclipse.jface.wizard.WizardDialog.buttonPressed(WizardDialog.java:432)
	at org.eclipse.jface.dialogs.Dialog$2.widgetSelected(Dialog.java:624)
	at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:248)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1053)
	at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4169)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3758)
	at org.eclipse.jface.window.Window.runEventLoop(Window.java:825)
	at org.eclipse.jface.window.Window.open(Window.java:801)
	at org.eclipse.debug.internal.ui.importexport.breakpoints.ImportBreakpoints.run(ImportBreakpoints.java:45)
	at org.eclipse.debug.internal.ui.actions.AbstractDebugActionDelegate.runWithEvent(AbstractDebugActionDelegate.java:321)
	at org.eclipse.ui.internal.PluginAction.runWithEvent(PluginAction.java:241)
	at org.eclipse.jface.action.ActionContributionItem.handleWidgetSelection(ActionContributionItem.java:584)
	at org.eclipse.jface.action.ActionContributionItem.access$2(ActionContributionItem.java:501)
	at org.eclipse.jface.action.ActionContributionItem$5.handleEvent(ActionContributionItem.java:411)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1053)
	at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4169)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3758)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$9.run(PartRenderingEngine.java:1031)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:925)
	at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:86)
	at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:585)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:540)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:124)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196)
	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:353)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:180)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:601)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:629)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:584)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1438)
	at org.eclipse.equinox.launcher.Main.main(Main.java:1414)

Also on the second page of the wizard it would be good to show the resource path the breakpoint would be re-created on (if possible)
Comment 11 Sebastian Schmidt CLA 2012-07-23 12:56:03 EDT
Created attachment 219062 [details]
Updated Patch #1
Comment 12 Sebastian Schmidt CLA 2012-07-23 12:56:15 EDT
(In reply to comment #10)
> Testing the patch I get:
> 
> java.lang.reflect.InvocationTargetException
> at

Wooops, sorry. Stupid mistake... I updated the patch.

> Also on the second page of the wizard it would be good to show the resource path
> the breakpoint would be re-created on (if possible)

Agreed. Do you have a suggestion on how to implement this the best way? Should we add a decorator for the view?
Comment 13 Michael Rennie CLA 2012-07-23 13:54:09 EDT
(In reply to comment #12) 
> 
> Agreed. Do you have a suggestion on how to implement this the best way? Should
> we add a decorator for the view?

A decorator would be nice, also a text box along the bottom of the wizard might look nice too (something like the open type dialog). 

Looking at the code I see you run the import operation to create the breakpoints to show in the second page, so I am not sure how show the resource name. I assumed you were just showing the bp infos from the .bkpt file, in which case we could have just pulled out the IE_NODE_RESOURCE field and used that.
Comment 14 Pawel Piech CLA 2012-07-23 14:24:27 EDT
Created attachment 219067 [details]
Fix for "Show qualified names" in breakpoint export/import wizard.

(In reply to comment #10)

> Also on the second page of the wizard it would be good to show the resource
> path the breakpoint would be re-created on (if possible)

IMO showing the resource patch might be confusing if it points to the workspace root.  A cleaner solution would be to have the wizard to show a fully qualified path of breakpoints (if the breakpoints view was configured to do so).  It looks like this feature was broken when we migrated the bp view to flex hierarchy and we should fix it anyway.
Comment 15 Pawel Piech CLA 2012-07-24 18:38:08 EDT
It looks like I muddied the waters a bit.  Mike, do you think having the fully qualified names option is enough or should Sebastian add the resource path as you suggested?
Comment 16 Michael Rennie CLA 2012-07-25 10:04:24 EDT
(In reply to comment #15)
> It looks like I muddied the waters a bit.  Mike, do you think having the fully
> qualified names option is enough or should Sebastian add the resource path as
> you suggested?

The qualified names problem should be fixed as it is a regression. As for the path names, yes, I still think it would be good to know where the breakpoint would be created, especially if you have more that one breakpoint for the same type name but for different resources - i.e. same class from different projects.
Comment 17 Sebastian Schmidt CLA 2012-07-30 14:40:24 EDT
Created attachment 219334 [details]
Decorator to add breakpoint paths to preview

Apply "Updated Patch #1" first.
Comment 18 Sebastian Schmidt CLA 2012-07-30 14:40:39 EDT
(In reply to comment #16)
> The qualified names problem should be fixed as it is a regression. As for the
> path names, yes, I still think it would be good to know where the breakpoint
> would be created, especially if you have more that one breakpoint for the same
> type name but for different resources - i.e. same class from different projects.

I attached a second patch including a decorator which adds the target resource path to the breakpoints in the preview view.
Comment 20 Dani Megert CLA 2012-07-31 08:23:33 EDT
This change causes API Tools errors because a wrong tag (3.8 instead of 3.9) was used.

Fixed with http://git.eclipse.org/c/platform/eclipse.platform.debug.git/commit/?id=b0ae59569631fa54b9063bd5a4f151a77aaf6821

Please make sure that you
- have a correct API baseline set in your workspaces
- don't lower the default severity for problems
Comment 21 Pawel Piech CLA 2012-07-31 09:21:49 EDT
(In reply to comment #20)
> - have a correct API baseline set in your workspaces
Yep, I forgot to update my baseline.  Thanks!