Some changes to service protocol before we land 1.13.

- Split AddBreakpoint into two rpcs: AddBreakpoint and
  AddBreakpointWithScriptUri.

- Remove Isolate.entry.

Closes #24329

R=johnmccutchan@google.com

Review URL: https://codereview.chromium.org//1379163002 .

CHANGELOG.md

@@ -25,6 +25,10 @@
 * This is the last release to ship the VM's "legacy debug protocol".
   We intend to remove the legacy debug protocol in Dart VM 1.14.
 
+* The VM's Service Protocol has been updated to version 3.0 to take care
+  of a number of issues uncovered by the first few non-observatory
+  clients.  This is a potentially breaking change for clients.
+
 ## 1.12.0
 
 ### Language changes

runtime/observatory/lib/src/service/object.dart

@@ -1517,7 +1517,7 @@ class Isolate extends ServiceObjectOwner with Coverage {
     if (col != null) {
       params['column'] = col.toString();
     }
-    return invokeRpc('addBreakpoint', params);
+    return invokeRpc('addBreakpointWithScriptUri', params);
   }
 
   Future<ServiceObject> addBreakpointAtEntry(ServiceFunction function) {

runtime/vm/isolate.cc

@@ -1812,15 +1812,6 @@ void Isolate::PrintJSON(JSONStream* stream, bool ref) {
   }
   int64_t start_time_millis = start_time() / kMicrosecondsPerMillisecond;
   jsobj.AddPropertyTimeMillis("startTime", start_time_millis);
-  IsolateSpawnState* state = spawn_state();
-  if (state != NULL) {
-    const Object& entry = Object::Handle(this, state->ResolveFunction());
-    if (!entry.IsNull() && entry.IsFunction()) {
-      Function& func = Function::Handle(this);
-      func ^= entry.raw();
-      jsobj.AddProperty("entry", func);
-    }
-  }
   {
     JSONObject jsheap(&jsobj, "_heaps");
     heap()->PrintToJSONObject(Heap::kNew, &jsheap);

runtime/vm/service.cc

@@ -2041,17 +2041,9 @@ static bool GetCallSiteData(Isolate* isolate, JSONStream* js) {
 }
 
 
-static const MethodParameter* add_breakpoint_params[] = {
-  ISOLATE_PARAMETER,
-  new IdParameter("scriptId", false),
-  new IdParameter("scriptUri", false),
-  new UIntParameter("line", true),
-  new UIntParameter("column", false),
-  NULL,
-};
-
-
-static bool AddBreakpoint(Isolate* isolate, JSONStream* js) {
+static bool AddBreakpointCommon(Isolate* isolate,
+                                JSONStream* js,
+                                const String& script_uri) {
   const char* line_param = js->LookupParam("line");
   intptr_t line = UIntParameter::Parse(line_param);
   const char* col_param = js->LookupParam("column");
@@ -2064,28 +2056,6 @@ static bool AddBreakpoint(Isolate* isolate, JSONStream* js) {
       return true;
     }
   }
-  const char* script_id_param = js->LookupParam("scriptId");
-  const char* script_uri_param = js->LookupParam("scriptUri");
-  if (script_id_param == NULL && script_uri_param == NULL) {
-    js->PrintError(kInvalidParams,
-                   "%s expects the 'scriptId' or the 'scriptUri' parameter",
-                   js->method());
-    return true;
-  }
-  String& script_uri = String::Handle(isolate);
-  if (script_id_param != NULL) {
-    Object& obj =
-        Object::Handle(LookupHeapObject(isolate, script_id_param, NULL));
-    if (obj.raw() == Object::sentinel().raw() || !obj.IsScript()) {
-      PrintInvalidParamError(js, "scriptId");
-      return true;
-    }
-    const Script& script = Script::Cast(obj);
-    script_uri = script.url();
-  }
-  if (script_uri_param != NULL) {
-    script_uri = String::New(script_uri_param);
-  }
   ASSERT(!script_uri.IsNull());
   Breakpoint* bpt = NULL;
   bpt = isolate->debugger()->SetBreakpointAtLineCol(script_uri, line, col);
@@ -2100,6 +2070,46 @@ static bool AddBreakpoint(Isolate* isolate, JSONStream* js) {
 }
 
 
+static const MethodParameter* add_breakpoint_params[] = {
+  ISOLATE_PARAMETER,
+  new IdParameter("scriptId", true),
+  new UIntParameter("line", true),
+  new UIntParameter("column", false),
+  NULL,
+};
+
+
+static bool AddBreakpoint(Isolate* isolate, JSONStream* js) {
+  const char* script_id_param = js->LookupParam("scriptId");
+  Object& obj =
+      Object::Handle(LookupHeapObject(isolate, script_id_param, NULL));
+  if (obj.raw() == Object::sentinel().raw() || !obj.IsScript()) {
+    PrintInvalidParamError(js, "scriptId");
+    return true;
+  }
+  const Script& script = Script::Cast(obj);
+  const String& script_uri = String::Handle(script.url());
+  ASSERT(!script_uri.IsNull());
+  return AddBreakpointCommon(isolate, js, script_uri);
+}
+
+
+static const MethodParameter* add_breakpoint_with_script_uri_params[] = {
+  ISOLATE_PARAMETER,
+  new IdParameter("scriptUri", true),
+  new UIntParameter("line", true),
+  new UIntParameter("column", false),
+  NULL,
+};
+
+
+static bool AddBreakpointWithScriptUri(Isolate* isolate, JSONStream* js) {
+  const char* script_uri_param = js->LookupParam("scriptUri");
+  const String& script_uri = String::Handle(String::New(script_uri_param));
+  return AddBreakpointCommon(isolate, js, script_uri);
+}
+
+
 static const MethodParameter* add_breakpoint_at_entry_params[] = {
   ISOLATE_PARAMETER,
   new IdParameter("functionId", true),
@@ -3121,6 +3131,8 @@ static ServiceMethodDescriptor service_methods_[] = {
     NULL },
   { "addBreakpoint", AddBreakpoint,
     add_breakpoint_params },
+  { "addBreakpointWithScriptUri", AddBreakpointWithScriptUri,
+    add_breakpoint_with_script_uri_params },
   { "addBreakpointAtEntry", AddBreakpointAtEntry,
     add_breakpoint_at_entry_params },
   { "_addBreakpointAtActivation", AddBreakpointAtActivation,

runtime/vm/service/service.md

@@ -25,6 +25,7 @@ The Service Protocol uses [JSON-RPC 2.0][].
 - [Private RPCs, Types, and Properties](#private-rpcs-types-and-properties)
 - [Public RPCs](#public-rpcs)
 	- [addBreakpoint](#addbreakpoint)
+	- [addBreakpointWithScriptUri](#addbreakpointwithscripturi)
 	- [addBreakpointAtEntry](#addbreakpointatentry)
 	- [evaluate](#evaluate)
 	- [evaluateInFrame](#evaluateinframe)
@@ -372,8 +373,7 @@ in the section on [public types](#public-types).
 
 ```
 Breakpoint addBreakpoint(string isolateId,
-                         string scriptId [optional],
-                         string scriptUri [optional],
+                         string scriptId,
                          int line,
                          int column [optional])
 ```
@@ -381,8 +381,41 @@ Breakpoint addBreakpoint(string isolateId,
 The _addBreakpoint_ RPC is used to add a breakpoint at a specific line
 of some script.
 
-The _scriptId_ or _scriptUri_ parameter is used to specify the target
-script. One of these two parameters must always be provided.
+The _scriptId_ parameter is used to specify the target script.
+
+The _line_ parameter is used to specify the target line for the
+breakpoint. If there are multiple possible breakpoints on the target
+line, then the VM will place the breakpoint at the location which
+would execute soonest. If it is not possible to set a breakpoint at
+the target line, the breakpoint will be added at the next possible
+breakpoint location within the same function.
+
+The _column_ parameter may be optionally specified.  This is useful
+for targeting a specific breakpoint on a line with multiple possible
+breakpoints.
+
+If no breakpoint is possible at that line, the _102_ (Cannot add
+breakpoint) error code is returned.
+
+Note that breakpoints are added and removed on a per-isolate basis.
+
+See [Breakpoint](#breakpoint).
+
+### addBreakpointWithScriptUri
+
+```
+Breakpoint addBreakpointWithScriptUri(string isolateId,
+                                      string scriptUri,
+                                      int line,
+                                      int column [optional])
+```
+
+The _addBreakpoint_ RPC is used to add a breakpoint at a specific line
+of some script.  This RPC is useful when a script has not yet been
+assigned an id, for example, if a script is in a deferred library
+which has not yet been loaded.
+
+The _scriptUri_ parameter is used to specify the target script.
 
 The _line_ parameter is used to specify the target line for the
 breakpoint. If there are multiple possible breakpoints on the target
@@ -1655,11 +1688,6 @@ class Isolate extends Response {
   // running, this will be a resume event.
   Event pauseEvent;
 
-  // The entry function for this isolate.
-  //
-  // Guaranteed to be initialized when the IsolateRunnable event fires.
-  @Function entry [optional];
-
   // The root library for this isolate.
   //
   // Guaranteed to be initialized when the IsolateRunnable event fires.
@@ -2117,9 +2145,9 @@ class VM extends Response {
 
 version | comments
 ------- | --------
-1.0 draft 1 | initial revision
-1.1 | Describe protocol version 2.0.
-1.2 | Describe protocol version 3.0.  Added UnresolvedSourceLocation.  Added Sentinel return to getIsolate.
+1.0 | initial revision
+2.0 | Describe protocol version 2.0.
+3.0 | Describe protocol version 3.0.  Added UnresolvedSourceLocation.  Added Sentinel return to getIsolate.  Add AddedBreakpointWithScriptUri.  Removed Isolate.entry.
 
 
 [discuss-list]: https://groups.google.com/a/dartlang.org/forum/#!forum/observatory-discuss