Show errors reported for invalid setBreakpoint calls.

Show errors reported for invalid setBreakpoint calls.

R=devoncarew@google.com
BUG=

Committed: https://code.google.com/p/dart/source/detail?r=40720

[devoncarew, 2014-09-25 21:29:10]

Two main comments:

- we'll want to set both file and packages breakpoints if they're both
applicable
- we'll want a more subtle user notification of breakpoint failure then a model
dialog

https://codereview.chromium.org/605833002/diff/1/editor/tools/plugins/com.goo...
File
editor/tools/plugins/com.google.dart.tools.debug.core/src/com/google/dart/tools/debug/core/server/ServerBreakpointManager.java
(right):

https://codereview.chromium.org/605833002/diff/1/editor/tools/plugins/com.goo...
editor/tools/plugins/com.google.dart.tools.debug.core/src/com/google/dart/tools/debug/core/server/ServerBreakpointManager.java:240:
String filePath = breakpoint.getFilePath2();
Perhaps instead we should call this getActualFilePath().

https://codereview.chromium.org/605833002/diff/1/editor/tools/plugins/com.goo...
editor/tools/plugins/com.google.dart.tools.debug.core/src/com/google/dart/tools/debug/core/server/ServerBreakpointManager.java:245:
String url = getPackagesUrlForFilePath(filePath);
We want to set both the file and the packages breakpoints. We won't know in
which context the code might be running.

https://codereview.chromium.org/605833002/diff/1/editor/tools/plugins/com.goo...
editor/tools/plugins/com.google.dart.tools.debug.core/src/com/google/dart/tools/debug/core/server/ServerBreakpointManager.java:257:
DebugUIHelper.getHelper().showError("Unable to set breakpoint", error);
This setBreakpoint() method may not be called as a result of a direct user
action. We don't want to show a modal dialog here, esp. per failure.

I would suggest instead that we write something to the console, via
target.writeToStdout() (as when a breakpoint location is moved by the VM).

[scheglov, 2014-09-25 22:09:57]

PTAL

https://codereview.chromium.org/605833002/diff/1/editor/tools/plugins/com.goo...
File
editor/tools/plugins/com.google.dart.tools.debug.core/src/com/google/dart/tools/debug/core/server/ServerBreakpointManager.java
(right):

https://codereview.chromium.org/605833002/diff/1/editor/tools/plugins/com.goo...
editor/tools/plugins/com.google.dart.tools.debug.core/src/com/google/dart/tools/debug/core/server/ServerBreakpointManager.java:240:
String filePath = breakpoint.getFilePath2();
On 2014/09/25 21:29:10, devoncarew wrote:
> Perhaps instead we should call this getActualFilePath().

Done.

https://codereview.chromium.org/605833002/diff/1/editor/tools/plugins/com.goo...
editor/tools/plugins/com.google.dart.tools.debug.core/src/com/google/dart/tools/debug/core/server/ServerBreakpointManager.java:245:
String url = getPackagesUrlForFilePath(filePath);
On 2014/09/25 21:29:09, devoncarew wrote:
> We want to set both the file and the packages breakpoints. We won't know in
> which context the code might be running.

Done.

https://codereview.chromium.org/605833002/diff/1/editor/tools/plugins/com.goo...
editor/tools/plugins/com.google.dart.tools.debug.core/src/com/google/dart/tools/debug/core/server/ServerBreakpointManager.java:257:
DebugUIHelper.getHelper().showError("Unable to set breakpoint", error);
On 2014/09/25 21:29:09, devoncarew wrote:
> This setBreakpoint() method may not be called as a result of a direct user
> action. We don't want to show a modal dialog here, esp. per failure.
> 
> I would suggest instead that we write something to the console, via
> target.writeToStdout() (as when a breakpoint location is moved by the VM).

Done.

[devoncarew, 2014-09-26 17:06:09]

lgtm

Looks great!

[scheglov, 2014-09-26 17:14:26]

Committed patchset #2 (id:20001) manually as 40720 (presubmit successful).