Refactor service usage in Dartium

Source/bindings/dart/DartService.cpp

 // Copyright (c) 2013, the Dart project authors. Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "config.h"
 
 #include "bindings/dart/DartService.h"
 
 #include "DartApplicationLoader.h"
 #include "DartController.h"
 #include "DartDocument.h"
-#include "DartServiceInternal.h"
 #include "DartUtilities.h"
 #include "DartWindow.h"
 
 
-// These must be kept in sync with vmservice/constants.dart
-// TODO(johnmccutchan): Put these constants in one place.
-#define VM_SERVICE_ISOLATE_STARTUP_MESSAGE_ID 1
-#define VM_SERVICE_ISOLATE_SHUTDOWN_MESSAGE_ID 2
+namespace WebCore {
 
-// The following Resources class is used in combination with a generated
-// source file that expects underscores in names. The NOLINT tags are
-// used to suppress the errors.
-// TODO(johnmccutchan): Move VM service source into runtime and out of bin.
-namespace dart {
-namespace bin {
-class Resources {
-public:
-    static const int kNoSuchInstance = -1;
-
-    static int ResourceLookup(const char* path, const char** resource)
-    {
-        for (int i = 0; i < get_resource_count(); i++) {
-            resource_map_entry* entry = get_resource(i);
-            if (!strcmp(path, entry->path_)) {
-                *resource = entry->resource_;
-                ASSERT(entry->length_ > 0);
-                return entry->length_;
-            }
-        }
-        return kNoSuchInstance;
+#define SHUTDOWN_ON_ERROR(handle)                      \

[Jacob, 2014/01/08 18:47:10]

Consider moving this to DartUtils or using existing helpers in DartUtils (if
any).

[Cutch, 2014/01/10 21:34:26]

This code is specific to the service startup code. It also must be a macro
because it returns from the function.

+    if (Dart_IsError(handle)) {                        \
+        m_errorMsg = strdup(Dart_GetError(handle));    \
+        Dart_ExitScope();                              \
+        Dart_ShutdownIsolate();                        \
+        return false;                                  \
     }
 
-    static intptr_t get_resource_count()  // NOLINT.
-    {
-        return builtin_resources_count_;
-    }
-
-    static const char* get_resource_path(intptr_t i) // NOLINT.
-    {
-        return get_resource(i)->path_;
-    }
-
-private:
-    struct resource_map_entry {  // NOLINT.
-      const char* path_;  // NOLINT.
-      const char* resource_;  // NOLINT.
-      intptr_t length_;  // NOLINT.
-    };
-
-    // These fields are generated by resources_gen.cc.
-    static resource_map_entry builtin_resources_[];  // NOLINT.
-    static const intptr_t builtin_resources_count_;  // NOLINT.
-
-    static resource_map_entry* get_resource(int i)  // NOLINT.
-    {
-        ASSERT(i >= 0 && i < builtin_resources_count_);
-        return &builtin_resources_[i];
-    }
-
-    DISALLOW_IMPLICIT_CONSTRUCTORS(Resources);
-};
-
-}
-}
-namespace WebCore {
-
-#define SHUTDOWN_ON_ERROR(handle)                                   \
-    if (Dart_IsError(handle)) {                                     \
-        m_errorMsg = strdup(Dart_GetError(handle));                 \
-        goto error;                                                 \
-    }
-
-static const char* kScriptUri = "vmservice:";
-#define kLibraryResourceNamePrefix "/vmservice"
-static const char* kVMServiceDartiumLibraryScriptResourceName =
-    kLibraryResourceNamePrefix "/vmservice_dartium.dart";
-static const char* kVMServiceLibraryName =
-    kLibraryResourceNamePrefix "/vmservice.dart";
-
-
+const char* DartService::m_errorMsg = 0;
 Dart_Isolate DartService::m_isolate = 0;
-Dart_Port DartService::m_port = ILLEGAL_PORT;
-Dart_Port DartService::m_requestPort = ILLEGAL_PORT;
-const char* DartService::m_errorMsg = 0;
-
 
 bool DartService::Start(Document* document)
 {
     if (m_isolate) {
-        // Already running.
+        // Already started.
         return true;
     }
-    {
-        char* error = 0;
-
-        m_isolate = DartController::createIsolate(kScriptUri, "main", document, true, &error);
-        if (!m_isolate) {
-            m_errorMsg = error;
-            return false;
-        }
-
-        Dart_EnterScope();
-        // Set up the library tag handler for this isolate.
-        Dart_Handle result = Dart_SetLibraryTagHandler(LibraryTagHandler);
-        SHUTDOWN_ON_ERROR(result);
-
-        {
-            // Load source into service isolate.
-            Dart_Handle library =
-                LoadScript(kVMServiceDartiumLibraryScriptResourceName);
-            SHUTDOWN_ON_ERROR(library);
-            result = Dart_SetNativeResolver(library, DartService::NativeResolver);
-            SHUTDOWN_ON_ERROR(result);
-        }
-        // Make the isolate runnable so that it is ready to handle messages.
-        Dart_ExitScope();
-        Dart_ExitIsolate();
-
-        bool retval = Dart_IsolateMakeRunnable(m_isolate);
-        if (!retval) {
-            Dart_EnterIsolate(m_isolate);
-            Dart_ShutdownIsolate();
-            m_errorMsg = "Invalid isolate state - Unable to make it runnable.";
-            return false;
-        }
-
-        Dart_EnterIsolate(m_isolate);
-        Dart_EnterScope();
-
-        // Invoke main.
-        Dart_Handle library = Dart_RootLibrary();
-        Dart_Handle entryFunctioName = Dart_NewStringFromCString("main");
-        SHUTDOWN_ON_ERROR(entryFunctioName);
-        result = Dart_Invoke(library, entryFunctioName, 0, 0);
-        SHUTDOWN_ON_ERROR(result);
-
-        // Retrieve the ReceivePort that the service is waiting on. The _receivePort
-        // variable is setup in the call to main.
-        Dart_Handle portFieldName = Dart_NewStringFromCString("_receivePort");
-        SHUTDOWN_ON_ERROR(portFieldName);
-        Dart_Handle receivePort = Dart_GetField(library, portFieldName);
-        SHUTDOWN_ON_ERROR(receivePort);
-
-        m_port = DartServiceInternal::GetPortIdFromPort(receivePort);
-        if (m_port == ILLEGAL_PORT) {
-            Dart_ExitScope();
-            Dart_ShutdownIsolate();
-            m_errorMsg = "Invalid isolate state - Unable to get receivePort";
-            return false;
-        }
-
-        {
-            // Retrieve the ReceivePort that the service is waiting on. The _receivePort
-            // variable is setup in the call to main.
-            Dart_Handle portFieldName = Dart_NewStringFromCString("_requestPort");
-            SHUTDOWN_ON_ERROR(portFieldName);
-            Dart_Handle receivePort = Dart_GetField(library, portFieldName);
-            SHUTDOWN_ON_ERROR(receivePort);
-            m_requestPort = DartServiceInternal::GetPortIdFromPort(receivePort);
-            ASSERT(m_requestPort != ILLEGAL_PORT);
-        }
-
-        Dart_ExitScope();
-        Dart_ExitIsolate();
-        return true;
+    ASSERT(!Dart_CurrentIsolate());
+    Dart_Isolate isolate = Dart_GetServiceIsolate(document);

[siva, 2014/01/10 00:10:13]

As discussed offline we should see if it is possible to just get the ServicePort
here instead of the isolate

+    if (!isolate) {
+        m_errorMsg = "Internal error.";

[Jacob, 2014/01/08 18:47:10]

add a better error message

[Cutch, 2014/01/10 21:34:26]

Done.

+        return false;
     }
-error:
+    Dart_EnterIsolate(isolate);
+    Dart_EnterScope();
+    Dart_Handle result;
+    Dart_Handle library = LoadScript();
+    // Expect a library.
+    ASSERT(library != Dart_Null());

[Jacob, 2014/01/08 18:47:10]

why not assert that this is a library?

[Cutch, 2014/01/10 21:34:26]

Done.

+    SHUTDOWN_ON_ERROR(library);
+    library = Dart_RootLibrary();
+    result = Dart_SetNativeResolver(library, NativeResolver);
+    SHUTDOWN_ON_ERROR(result);
+    result = Dart_Invoke(library, Dart_NewStringFromCString("main"), 0, 0);
+    SHUTDOWN_ON_ERROR(result);
     Dart_ExitScope();
-    Dart_ShutdownIsolate();
-    m_isolate = 0;
-    m_port = ILLEGAL_PORT;
-    m_requestPort = ILLEGAL_PORT;
-    return false;
-}
-
-
-bool DartService::Stop()
-{
-    if (!m_isolate) {
-        // Already shutdown.
-        return true;
-    }
-    m_port = ILLEGAL_PORT;
-    m_requestPort = ILLEGAL_PORT;
-    Dart_Isolate isolate = m_isolate;
-    m_isolate = 0;
-    Dart_EnterIsolate(isolate);
-    DartController::shutdownIsolate(isolate);
+    Dart_ExitIsolate();
+    m_isolate = isolate;
     return true;
 }
 
 
 const char* DartService::GetErrorMessage()
 {
     return m_errorMsg ? m_errorMsg : "No error.";
 }
 
 
-Dart_Port DartService::port()
-{
-    return m_port;
-}
-
-
-bool DartService::IsRunning()
-{
-    return m_port != ILLEGAL_PORT;
-}
-
-
-static Dart_Handle MakeServiceControlMessage(Dart_Port portId, intptr_t code, Dart_Handle name)
-{
-    Dart_Handle result;
-    UNUSED_PARAM(result);
-    Dart_Handle list = Dart_NewList(4);
-    ASSERT(!Dart_IsError(list));
-    Dart_Handle codeHandle = Dart_NewInteger(code);
-    ASSERT(!Dart_IsError(codeHandle));
-    result = Dart_ListSetAt(list, 0, codeHandle);
-    ASSERT(!Dart_IsError(result));
-    Dart_Handle portIdHandle = Dart_NewInteger(portId);
-    ASSERT(!Dart_IsError(portIdHandle));
-    result = Dart_ListSetAt(list, 1, portIdHandle);
-    ASSERT(!Dart_IsError(result));
-    Dart_Handle sendPort = Dart_NewSendPort(portId);
-    ASSERT(!Dart_IsError(sendPort));
-    result = Dart_ListSetAt(list, 2, sendPort);
-    ASSERT(!Dart_IsError(result));
-    result = Dart_ListSetAt(list, 3, name);
-    ASSERT(!Dart_IsError(result));
-    return list;
-}
-
-
-bool DartService::SendIsolateStartupMessage()
-{
-    if (!IsRunning()) {
-        return false;
-    }
-    Dart_Handle name = Dart_DebugName();
-    ASSERT(!Dart_IsError(name));
-    Dart_Handle list = MakeServiceControlMessage(Dart_GetMainPortId(), VM_SERVICE_ISOLATE_STARTUP_MESSAGE_ID, name);
-    ASSERT(!Dart_IsError(list));
-    return Dart_Post(m_port, list);
-}
-
-
-bool DartService::SendIsolateShutdownMessage()
-{
-    if (!IsRunning())
-        return false;
-    Dart_Handle list = MakeServiceControlMessage(Dart_GetMainPortId(), VM_SERVICE_ISOLATE_SHUTDOWN_MESSAGE_ID, Dart_Null());
-    ASSERT(!Dart_IsError(list));
-    return Dart_Post(m_port, list);
-}
-
-
 DartServiceRequest::DartServiceRequest(const String& request) : m_request(request)
 {
 }
 
 DartServiceRequest::~DartServiceRequest()
 {
 }
 
 
-
 // The format of the message is:
 // [request string, address of DartServiceRequest].
 static Dart_Handle MakeServiceRequestMessage(DartServiceRequest* request)
 {
     intptr_t requestAddress = reinterpret_cast<intptr_t>(request);
     int64_t requestAddress64 = static_cast<int64_t>(requestAddress);
     Dart_Handle result;
     UNUSED_PARAM(result);
     Dart_Handle list = Dart_NewList(2);
     ASSERT(!Dart_IsError(list));
     Dart_Handle requestHandle = DartUtilities::stringToDartString(request->GetRequestString());
     ASSERT(!Dart_IsError(requestHandle));
     Dart_Handle addressHandle = Dart_NewInteger(requestAddress64);
     ASSERT(!Dart_IsError(addressHandle));
     result = Dart_ListSetAt(list, 0, requestHandle);
     ASSERT(!Dart_IsError(result));
     result = Dart_ListSetAt(list, 1, addressHandle);
     ASSERT(!Dart_IsError(result));
     return list;
 }
 
 
 void DartService::MakeServiceRequest(DartServiceRequest* request)
 {
     ASSERT(m_isolate);
     // TODO(johnmccutchan): Once the VM service is no longer a DOM isolate,
     // we must be careful about entering the isolate.

[siva, 2014/01/10 00:10:13]

Is this comment valid? You indicated that the service isolate has to be a DOM
isolate in order to post replies.

[Cutch, 2014/01/10 21:34:26]

The comment was optimistic that one day the service won't be running inside a
DOM isolate.

     DartIsolateScope isolateScope(m_isolate);
     DartApiScope apiScope;
     Dart_Handle message = MakeServiceRequestMessage(request);
-    Dart_Post(m_requestPort, message);
+    if (Dart_IsError(message)) {
+        return;
+    }
+    Dart_Handle library = Dart_RootLibrary();
+    if (Dart_IsError(library)) {
+        return;
+    }
+    Dart_Handle requestPortFieldName = Dart_NewStringFromCString("requestPort");
+    Dart_Handle requestPort = Dart_GetField(library, requestPortFieldName);
+    if (Dart_IsError(requestPort)) {
+        return;
+    }

[siva, 2014/01/10 00:10:13]

If the API handed back the service request port this sort of field access would
not be necessary.

+    Dart_SendPortSend(requestPort, message);
 }
 
 
-Dart_Handle DartService::GetSource(const char* name)
+Dart_Handle DartService::LoadScript()
 {
-    const char* vmserviceSource = 0;
-    int result = dart::bin::Resources::ResourceLookup(name, &vmserviceSource);
-    UNUSED_PARAM(result);
-    ASSERT(result != dart::bin::Resources::kNoSuchInstance);
-    return Dart_NewStringFromCString(vmserviceSource);
+    const char* kScriptChars = "\n"
+"library vmservice_dartium;\n"

[Jacob, 2014/01/08 18:47:10]

this script should be moved to an internal library
e.g. 
dart:_vmservice_dartium
rather than specified in code here.

[Cutch, 2014/01/10 21:34:26]

We do not want to add the VM service glue script to any internal libraries or as
part of the snapshot. It makes sense to define it here because it will only ever
be used in this file.

+"\n"
+"import 'dart:isolate';\n"
+"import 'dart:vmservice';\n"
+"\n"
+"\n"
+"// The receive port that service request messages are delivered on.\n"
+"SendPort requestPort;\n"
+"\n"
+"// The native method that is called to post the response back to DevTools.\n"
+"void postResponse(String response, int cookie) native \"VMService_PostResponse\";\n"
+"\n"
+"/// Dartium Service receives messages through the requestPort and posts\n"
+"/// responses via postResponse. It has a single persistent client.\n"
+"class DartiumClient extends Client {\n"
+"  DartiumClient(port, service) : super(service) {\n"
+"    port.listen((message) {\n"
+"      if (message == null) {\n"
+"        return;\n"
+"      }\n"
+"      if (message is! List) {\n"
+"        return;\n"
+"      }\n"
+"      if (message.length != 2) {\n"
+"        return;\n"
+"      }\n"
+"      if (message[0] is! String) {\n"
+"        return;\n"
+"      }\n"
+"      var uri = Uri.parse(message[0]);\n"
+"      var cookie = message[1];\n"
+"      onMessage(cookie, new Message.fromUri(uri));\n"
+"    });\n"
+"  }\n"
+"\n"
+"  void post(var seq, String response) {\n"
+"    postResponse(response, seq);\n"
+"  }\n"
+"\n"
+"  dynamic toJson() {\n"
+"    var map = super.toJson();\n"
+"    map['type'] = 'DartiumClient';\n"
+"  }\n"
+"}\n"
+"\n"
+"\n"
+"main() {\n"
+"  // Get VMService.\n"
+"  var service = new VMService();\n"
+"  var receivePort = new ReceivePort();\n"
+"  requestPort = receivePort.sendPort;\n"
+"  new DartiumClient(receivePort, service);\n"
+"}\n";
+    Dart_Handle url = Dart_NewStringFromCString("dart:vmservice_dartium");
+    ASSERT(!Dart_IsError(url));
+    Dart_Handle source = Dart_NewStringFromCString(kScriptChars);
+    ASSERT(!Dart_IsError(source));
+    Dart_Handle library = Dart_LoadScript(url, source, 0, 0);
+    return library;
 }
 
 
-Dart_Handle DartService::LoadScript(const char* name)
-{
-    Dart_Handle url = Dart_NewStringFromCString(name);
-    Dart_Handle source = GetSource(name);
-    return Dart_LoadScript(url, source, 0, 0);
-}
-
-
-Dart_Handle DartService::LoadSource(Dart_Handle library, const char* name)
-{
-    Dart_Handle url = Dart_NewStringFromCString(name);
-    Dart_Handle source = GetSource(name);
-    return Dart_LoadSource(library, url, source);
-}
-
-
-Dart_Handle DartService::LoadSources(Dart_Handle library, const char* names[])
-{
-    Dart_Handle result = Dart_Null();
-    for (int i = 0; names[i]; i++) {
-        result = LoadSource(library, names[i]);
-        if (Dart_IsError(result))
-            break;
-    }
-    return result;
-}
-
-
-static bool IsVMServiceURL(const char* url)
-{
-    static const intptr_t kLibraryResourceNamePrefixLen = strlen(kLibraryResourceNamePrefix);
-    return !strncmp(kLibraryResourceNamePrefix, url, kLibraryResourceNamePrefixLen);
-}
-
-
-static bool IsVMServiceLibrary(const char* url)
-{
-    return !strcmp(kVMServiceLibraryName, url);
-}
-
-static bool IsDartLibrary(const char* url)
-{
-    static const char* kDartPrefix = "dart:";
-    static const intptr_t kDartPrefixLen = strlen(kDartPrefix);
-    return !strncmp(kDartPrefix, url, kDartPrefixLen);
-}
-
-static Dart_Handle Canonicalize(const char* url)
-{
-    if (IsVMServiceURL(url)) {
-        // Already canonicalized.
-        return Dart_NewStringFromCString(url);
-    }
-    String path = String(kLibraryResourceNamePrefix) + String("/") + String(url);
-    return DartUtilities::stringToDartString(path);
-}
-
-
-Dart_Handle DartService::LibraryTagHandler(Dart_LibraryTag tag, Dart_Handle library, Dart_Handle url)
-{
-    if (!Dart_IsLibrary(library))
-        return Dart_NewApiError("not a library");
-    if (!Dart_IsString(url))
-        return Dart_NewApiError("url is not a string");
-    const char* urlString = 0;
-    Dart_Handle result = Dart_StringToCString(url, &urlString);
-    if (Dart_IsError(result))
-        return result;
-    Dart_Handle libraryUrl = Dart_LibraryUrl(library);
-    const char* libraryUrlString = 0;
-    result = Dart_StringToCString(libraryUrl, &libraryUrlString);
-    if (Dart_IsError(result))
-        return result;
-    if (IsDartLibrary(urlString))
-        return DartApplicationLoader::libraryTagHandlerCallback(tag, library, url);
-    switch (tag) {
-    case Dart_kCanonicalizeUrl:
-        return Canonicalize(urlString);
-        break;
-    case Dart_kImportTag: {
-        Dart_Handle source = GetSource(urlString);
-        if (Dart_IsError(source))
-            return source;
-        Dart_Handle lib = Dart_LoadLibrary(url, source);
-        if (Dart_IsError(lib))
-            return lib;
-        if (IsVMServiceLibrary(urlString)) {
-            // Install native resolver for this library.
-            result = Dart_SetNativeResolver(lib, DartService::NativeResolver);
-            if (Dart_IsError(result))
-                return result;
-        }
-        return lib;
-        }
-        break;
-    case Dart_kSourceTag: {
-        Dart_Handle source = GetSource(urlString);
-        if (Dart_IsError(source))
-            return source;
-        return Dart_LoadSource(library, url, source);
-        }
-        break;
-    default:
-        DART_UNIMPLEMENTED();
-        break;
-    }
-    ASSERT_NOT_REACHED();
-    return result;
-}
-
-
-void DartService::VmServiceShutdownCallback(void* callbackData)
-{
-    ASSERT(Dart_CurrentIsolate());
-    DartApiScope apiScope;
-    SendIsolateShutdownMessage();
-}
-
-static void SendServiceMessage(Dart_NativeArguments args)
-{
-    Dart_Handle sp = Dart_GetNativeArgument(args, 0);
-    Dart_Handle message = Dart_GetNativeArgument(args, 1);
-    DartServiceInternal::PostOOB(sp, message);
-}
-
 static void PostResponse(Dart_NativeArguments args)
 {
     Dart_Handle result;
     UNUSED_PARAM(result);
     Dart_Handle response = Dart_GetNativeArgument(args, 0);
     ASSERT(!Dart_IsError(response));
     Dart_Handle cookie = Dart_GetNativeArgument(args, 1);
     ASSERT(!Dart_IsError(cookie));
     int64_t requestAddress64 = 0;
     result = Dart_IntegerToInt64(cookie, &requestAddress64);
     ASSERT(!Dart_IsError(result));
     ASSERT(requestAddress64);
     intptr_t requestAddress = static_cast<intptr_t>(requestAddress64);
     ASSERT(requestAddress);
     DartServiceRequest* request = reinterpret_cast<DartServiceRequest*>(requestAddress);
     const char* responseString = 0;
     result = Dart_StringToCString(response, &responseString);
     ASSERT(!Dart_IsError(result));
     ASSERT(responseString);
     request->ResponseReady(responseString);
 }
 
+
 struct VmServiceNativeEntry {
     const char* name;
     int numArguments;
     Dart_NativeFunction function;
 };
 
 
 static VmServiceNativeEntry VmServiceNativeEntries[] = {
-    {"VMService_SendServiceMessage", 2, SendServiceMessage},
     {"VMService_PostResponse", 2, PostResponse}
 };
 
 
 Dart_NativeFunction DartService::NativeResolver(Dart_Handle name, int numArguments, bool* autoScopeSetup)
 {
     const char* functionName = 0;
     Dart_Handle result = Dart_StringToCString(name, &functionName);
     UNUSED_PARAM(result);
     ASSERT(!Dart_IsError(result));
     ASSERT(functionName);
     ASSERT(autoScopeSetup);
     *autoScopeSetup = true;
     intptr_t n = sizeof(VmServiceNativeEntries) / sizeof(VmServiceNativeEntries[0]);
     for (intptr_t i = 0; i < n; i++) {
         VmServiceNativeEntry entry = VmServiceNativeEntries[i];
         if (!strcmp(functionName, entry.name) && (numArguments == entry.numArguments)) {
             return entry.function;
         }
     }
     return 0;
 }
 
 }