Remove Field::FindFieldIndex() and update service protocol

This paves the way to remove functions and fields from top-level
classes. They are looked up via the class dictionary when resolving
names.

[email protected]

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

Author: mhausner

runtime/observatory/tests/service/get_object_rpc_test.dart

@@ -774,7 +774,7 @@ var tests = [
   (Isolate isolate) async {
     // Call eval to get a class id.
     var evalResult = await eval(isolate, 'new _DummyClass()');
-    var id = "${evalResult['class']['id']}/fields/0";
+    var id = "${evalResult['class']['id']}/fields/dummyVar";
     var params = {
       'objectId': id,
     };
@@ -796,7 +796,7 @@ var tests = [
   (Isolate isolate) async {
     // Call eval to get a class id.
     var evalResult = await eval(isolate, 'new _DummyClass()');
-    var id = "${evalResult['class']['id']}/fields/9999";
+    var id = "${evalResult['class']['id']}/fields/mythicalField";
     var params = {
       'objectId': id,
     };

runtime/vm/object.cc

@@ -3074,46 +3074,6 @@ void Class::AddFields(const GrowableArray<const Field*>& new_fields) const {
 }
 
 
-intptr_t Class::FindFieldIndex(const Field& needle) const {
-  Thread* thread = Thread::Current();
-  if (EnsureIsFinalized(thread) != Error::null()) {
-    return -1;
-  }
-  REUSABLE_ARRAY_HANDLESCOPE(thread);
-  REUSABLE_FIELD_HANDLESCOPE(thread);
-  REUSABLE_STRING_HANDLESCOPE(thread);
-  Array& fields_array = thread->ArrayHandle();
-  Field& field = thread->FieldHandle();
-  String& field_name = thread->StringHandle();
-  fields_array ^= fields();
-  ASSERT(!fields_array.IsNull());
-  String& needle_name = String::Handle(thread->zone());
-  needle_name ^= needle.name();
-  const intptr_t len = fields_array.Length();
-  for (intptr_t i = 0; i < len; i++) {
-    field ^= fields_array.At(i);
-    field_name ^= field.name();
-    if (field_name.Equals(needle_name)) {
-      return i;
-    }
-  }
-  // No field found.
-  return -1;
-}
-
-
-RawField* Class::FieldFromIndex(intptr_t idx) const {
-  const Array& flds = Array::Handle(fields());
-  if ((idx < 0) || (idx >= flds.Length())) {
-    return Field::null();
-  }
-  Field& field = Field::Handle();
-  field ^= flds.At(idx);
-  ASSERT(!field.IsNull());
-  return field.raw();
-}
-
-
 template <class FakeInstance>
 RawClass* Class::New(intptr_t index) {
   ASSERT(Object::class_class() != Class::null());
@@ -7561,14 +7521,17 @@ const char* Field::ToCString() const {
       "Field <%s.%s>:%s%s%s", cls_name, field_name, kF0, kF1, kF2);
 }
 
+
 void Field::PrintJSONImpl(JSONStream* stream, bool ref) const {
   JSONObject jsobj(stream);
   Class& cls = Class::Handle(owner());
-  intptr_t id = cls.FindFieldIndex(*this);
-  ASSERT(id >= 0);
-  intptr_t cid = cls.id();
+  String& field_name = String::Handle(name());
+  ASSERT(cls.LookupField(field_name) == this->raw());
+  field_name = String::EncodeIRI(field_name);
   AddCommonObjectProperties(&jsobj, "Field", ref);
-  jsobj.AddFixedServiceId("classes/%" Pd "/fields/%" Pd "", cid, id);
+  jsobj.AddFixedServiceId("classes/%" Pd "/fields/%s",
+                          cls.id(), field_name.ToCString());
+
   const String& user_name = String::Handle(PrettyName());
   const String& vm_name = String::Handle(name());
   AddNameProperties(&jsobj, user_name, vm_name);

runtime/vm/object.h

@@ -1148,8 +1148,6 @@ class Class : public Object {
   void SetFields(const Array& value) const;
   void AddField(const Field& field) const;
   void AddFields(const GrowableArray<const Field*>& fields) const;
-  intptr_t FindFieldIndex(const Field& field) const;
-  RawField* FieldFromIndex(intptr_t idx) const;
 
   // Returns an array of all fields of this class and its superclasses indexed
   // by offset in words.

runtime/vm/object_test.cc

@@ -3769,54 +3769,6 @@ static RawClass* GetClass(const Library& lib, const char* name) {
 }
 
 
-TEST_CASE(FindFieldIndex) {
-  const char* kScriptChars =
-      "class A {\n"
-      "  var a;\n"
-      "  var b;\n"
-      "}\n"
-      "class B {\n"
-      "  var d;\n"
-      "}\n"
-      "test() {\n"
-      "  new A();\n"
-      "  new B();\n"
-      "}";
-  Dart_Handle h_lib = TestCase::LoadTestScript(kScriptChars, NULL);
-  EXPECT_VALID(h_lib);
-  Dart_Handle result = Dart_Invoke(h_lib, NewString("test"), 0, NULL);
-  EXPECT_VALID(result);
-  Library& lib = Library::Handle();
-  lib ^= Api::UnwrapHandle(h_lib);
-  EXPECT(!lib.IsNull());
-  const Class& class_a = Class::Handle(GetClass(lib, "A"));
-  const Array& class_a_fields = Array::Handle(class_a.fields());
-  const Class& class_b = Class::Handle(GetClass(lib, "B"));
-  const Field& field_a = Field::Handle(GetField(class_a, "a"));
-  const Field& field_b = Field::Handle(GetField(class_a, "b"));
-  const Field& field_d = Field::Handle(GetField(class_b, "d"));
-  intptr_t field_a_index = class_a.FindFieldIndex(field_a);
-  intptr_t field_b_index = class_a.FindFieldIndex(field_b);
-  intptr_t field_d_index = class_a.FindFieldIndex(field_d);
-  // Valid index.
-  EXPECT_GE(field_a_index, 0);
-  // Valid index.
-  EXPECT_GE(field_b_index, 0);
-  // Invalid index.
-  EXPECT_EQ(field_d_index, -1);
-  Field& field_a_from_index = Field::Handle();
-  field_a_from_index ^= class_a_fields.At(field_a_index);
-  ASSERT(!field_a_from_index.IsNull());
-  // Same field.
-  EXPECT_EQ(field_a.raw(), field_a_from_index.raw());
-  Field& field_b_from_index = Field::Handle();
-  field_b_from_index ^= class_a_fields.At(field_b_index);
-  ASSERT(!field_b_from_index.IsNull());
-  // Same field.
-  EXPECT_EQ(field_b.raw(), field_b_from_index.raw());
-}
-
-
 TEST_CASE(FindClosureIndex) {
   // Allocate the class first.
   const String& class_name = String::Handle(Symbols::New("MyClass"));

runtime/vm/service.cc

@@ -1271,22 +1271,24 @@ static RawObject* LookupHeapObjectClasses(Thread* thread,
     return func.raw();
 
   } else if (strcmp(parts[2], "fields") == 0) {
-    // Field ids look like: "classes/17/fields/11"
+    // Field ids look like: "classes/17/fields/name"
     if (num_parts != 4) {
       return Object::sentinel().raw();
     }
-    intptr_t id;
-    if (!GetIntegerId(parts[3], &id)) {
+    const char* encoded_id = parts[3];
+    String& id = String::Handle(zone, String::New(encoded_id));
+    id = String::DecodeIRI(id);
+    if (id.IsNull()) {
       return Object::sentinel().raw();
     }
-    Field& field = Field::Handle(zone, cls.FieldFromIndex(id));
+    Field& field = Field::Handle(zone, cls.LookupField(id));
     if (field.IsNull()) {
       return Object::sentinel().raw();
     }
     return field.raw();
 
   } else if (strcmp(parts[2], "functions") == 0) {
-    // Function ids look like: "classes/17/functions/11"
+    // Function ids look like: "classes/17/functions/name"
     if (num_parts != 4) {
       return Object::sentinel().raw();
     }