This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 197707

Summary: Breakpoint should have a possibility to enable/disable other breakpoints when hit.
Product: debugger Reporter: Martin Entlicher <mentlicher>
Component: JavaAssignee: Martin Entlicher <mentlicher>
Severity: normal CC: anebuzelsky, jglick
Priority: P2 Keywords: API, API_REVIEW_FAST, PLAN
Version: 7.0.1   
Hardware: All   
OS: All   
Issue Type: ENHANCEMENT Exception Reporter:
Attachments: The API change which is necessary to implement this.
A modified API change.
The updated API change with usage from UI module.

Description Martin Entlicher 2011-04-13 14:23:15 UTC
When a breakpoint is hit, it should be possible to automatically enable or disable some other breakpoints.
Breakpoint properties should have an option to select other breakpoints that are to be enabled or disabled when the breakpoint is hit.
This functionality is valuable in GUI debugging for instance.
Comment 1 Martin Entlicher 2011-12-12 17:25:00 UTC
Created attachment 114057 [details]
The API change which is necessary to implement this.
Comment 2 Martin Entlicher 2011-12-12 17:27:18 UTC
Please review the attached simple API change that adds the ability to enable/disable breakpoints when one is hit. An implementation usage of this API follows.
I'll add a test as well.
Comment 3 Jesse Glick 2011-12-12 19:31:34 UTC
[JG01] enableDisableDependentBreakpoints does not do null checks, and it is not clear what null means in this case. Probably breakpointsToEnable and breakpointsToDisable and the corresponding getters and setters should be @NonNull, with Collections.emptySet() as the default value.

[JG02] Seems the new methods should be added to org.netbeans.api.debugger.Breakpoint, as nothing about them seems specific to JPDA and they might be useful for non-JVM debuggers as well.
Comment 4 Martin Entlicher 2011-12-13 10:49:55 UTC
[JG01] O.K. I'll make the methods @NonNull, it will be safer to use them.

[JG02] The functionality behind these methods rely on the implementation. I was thinking to add them to the Breakpoint class, but using these methods will have no effect until the appropriate logic is written into the implementation. This is why I've added them to JPDABreakpoint only, where we can guarantee the functionality.
We can move it into Breakpoint class and add a method like canHaveDependentBreakpoints() returning false by default and make the methods to throw IllegalStateException when canHaveDependentBreakpoints() is false.
Originally I thought that it's best when every Breakpoint implementation class adds this functionality as appropriate, but perhaps having it directly on the Breakpoint class allows to hide the implementation classes, which might be better and more universal.
Comment 5 Jesse Glick 2011-12-13 13:21:10 UTC
(In reply to comment #4)
> I was
> thinking to add them to the Breakpoint class, but using these methods will have
> no effect until the appropriate logic is written into the implementation.

Of course; but the Javadoc could simply say that a given debugger implementation may or may not honor dependent breakpoints.
Comment 6 Martin Entlicher 2011-12-13 16:54:23 UTC
Created attachment 114105 [details]
A modified API change.

I've modified the patch by adding the methods directly to the Breakpoint class.
Also I've introduced a method canHaveDependentBreakpoints() which is a clean way how to determine if the new methods actually change the behavior or not.
Comment 7 Jesse Glick 2011-12-13 17:04:29 UTC
[JG03] Then api.debugger.jpda and debugger.jpda should require 1.34 spec version of api.debugger.

BTW "Can not have..." should generally be "Cannot have...". (The latter means "it is impossible to have..." which is what you want; the former is ambiguous in writing, possibly meaning "it is possible to not have..." if "not" is stressed in speech or italicized in writing.)
Comment 8 Jaroslav Tulach 2011-12-14 08:19:19 UTC
Y01 I don't understand why you need "setBreakpointsToEnable" and "setBreakpointsToDisable"! The only use is in JPDA breakpoint class and it is restricted only to BreakpointsFromGroup. It will be much easier if the implementation just sets the name of the group to the JPDA breakpoint and the breakpoint, when asked for getBreakpointsToE/D would read the breakpoints from the group. Remove the setters from where they don't belong.

Y02 As you don't need setters, you can probably also remove "canHaveDependentBreakpoints" method.
Comment 9 Martin Entlicher 2011-12-15 11:02:53 UTC
Created attachment 114219 [details]
The updated API change with usage from UI module.

[JG03] Thanks, I've updated the version dependency.
       Also thanks for the language correction.

[Y01] I did not provide the associated change in the UI module as I've been working on them. The setter methods are called from the debugger.jpda.ui module, where the user specifies the breakpoint group. Also, I've designed the API to be more generic intentionally. The original idea was to use just a group name, but I think it's valuable to make use of the other grouping we have (by files, projects, breakpoint types, etc.) This is why the API uses just Set<Breakpoint> to be generic enough.

I'm attaching the API change with all associated UI changes.
Comment 10 Martin Entlicher 2011-12-19 11:02:29 UTC
Thanks for the review, I plan to integrate the change today COB.
Comment 11 Martin Entlicher 2011-12-19 19:46:02 UTC
Pushed as changeset:   209146:b22274554ce8
Comment 12 Quality Engineering 2011-12-20 15:56:50 UTC
Integrated into 'main-golden'
Log: #197707: Dependent breakpoints introduced. Methods to get/set breakpoints to enable/disable others were added to the Breakpoint class. It's used by JPDA breakpoints.