Reload native extensions when starting from a snapshot.

runtime/bin/loader.cc

Index: runtime/bin/loader.cc
diff --git a/runtime/bin/loader.cc b/runtime/bin/loader.cc
index d69def19f857ba943deb0db4c61c938685151b54..67fc94aeb2446319b308f5e4fc6b4e3b9d574a5a 100644
--- a/runtime/bin/loader.cc
+++ b/runtime/bin/loader.cc
@@ -11,6 +11,7 @@
 #include "bin/file.h"
 #include "bin/lockers.h"
 #include "bin/utils.h"
+#include "include/dart_tools_api.h"
 
 namespace dart {
 namespace bin {
@@ -361,6 +362,7 @@ bool Loader::ProcessResultLocked(Loader* loader, Loader::IOResult* result) {
   loader->monitor_->Exit();
 
   Dart_Handle dart_result = Dart_Null();
+  bool reload_extensions = false;
 
   switch (tag) {
     case Dart_kImportTag:
@@ -376,6 +378,7 @@ bool Loader::ProcessResultLocked(Loader* loader, Loader::IOResult* result) {
     case Dart_kScriptTag:
       if (payload_type == DartUtils::kSnapshotMagicNumber) {
         dart_result = Dart_LoadScriptFromSnapshot(payload, payload_length);
+        reload_extensions = true;
       } else if (payload_type == DartUtils::kKernelMagicNumber) {
         dart_result = Dart_LoadKernel(payload, payload_length);
       } else {
@@ -394,6 +397,15 @@ bool Loader::ProcessResultLocked(Loader* loader, Loader::IOResult* result) {
     return false;
   }
 
+  if (reload_extensions) {
+    dart_result = ReloadNativeExtensions();
+    if (Dart_IsError(dart_result)) {
+      // Remember the error if we encountered one.
+      loader->error_ = dart_result;
+      return false;
+    }
+  }
+
   return true;
 }
 
@@ -448,6 +460,70 @@ void Loader::InitForSnapshot(const char* snapshot_uri) {
 }
 
 
+#define RETURN_ERROR(result) \
+  if (Dart_IsError(result)) return result;
+
+Dart_Handle Loader::ReloadNativeExtensions() {
+  // For each library
+  Dart_Handle library_ids = Dart_GetLibraryIds();
+  RETURN_ERROR(library_ids);
+  intptr_t libraries_length = -1;
+  Dart_Handle result = Dart_ListLength(library_ids, &libraries_length);
+  RETURN_ERROR(result);
+  for (intptr_t i = 0; i < libraries_length; i++) {
+    int64_t library_id = -1;
+    result = Dart_IntegerToInt64(Dart_ListGetAt(library_ids, i),
+                                 &library_id);

[siva, 2016/11/02 23:48:38]

You could use Dart_ListGetRange to extract the library objects into
an array of handles, that way the expensive Dart_ListGetAt does not
run on every iteration of the loop.

+    RETURN_ERROR(result);
+    Dart_Handle import_ids = Dart_GetLibraryImports(library_id);
+    RETURN_ERROR(import_ids);
+    intptr_t imports_length = -1;
+    result = Dart_ListLength(import_ids, &imports_length);
+    RETURN_ERROR(result);
+
+    // For each import
+    for (intptr_t j = 1; j < imports_length; j += 2) {
+      int64_t import_id = -1;
+      result = Dart_IntegerToInt64(Dart_ListGetAt(import_ids, j),
+                                   &import_id);

[siva, 2016/11/02 23:48:38]

Ditto comment about Dart_ListGetAt

+      RETURN_ERROR(result);
+      const char* extension_uri = NULL;
+      result = Dart_StringToCString(Dart_GetLibraryURL(import_id),
+                                    &extension_uri);
+      RETURN_ERROR(result);
+
+      // If import starts with dart-ext:
+      if (strncmp(extension_uri, "dart-ext:", 9) == 0) {

[siva, 2016/11/02 23:48:38]

magic numbers 9 , 7 etc. use named constants.

[Cutch, 2016/11/03 17:45:34]

Also, these schemes already exist: DartUtils::kDartExtensionScheme

and there is already a function:

bool DartUtils::IsDartExtensionSchemeURL(const char* url);

+        const char* extension_path = DartUtils::RemoveScheme(extension_uri);
+
+        Dart_Handle library = Dart_GetLibraryFromId(library_id);
+        RETURN_ERROR(library);
+        const char* lib_uri = NULL;
+        result = Dart_StringToCString(Dart_GetLibraryURL(library_id),
+                                      &lib_uri);
+        RETURN_ERROR(result);
+
+        char* lib_path = NULL;
+        if (strncmp(lib_uri, "file://", 7) == 0) {
+          lib_path = DartUtils::RemoveFilename(
+              DartUtils::RemoveScheme(lib_uri));
+        } else {
+          lib_path = strdup(lib_uri);
+        }
+
+        result = Extensions::LoadExtension(lib_path,
+                                           extension_path,
+                                           library);
+        free(lib_path);
+        RETURN_ERROR(result);
+      }
+    }
+  }

[siva, 2016/11/02 23:48:38]

This seems like an expensive loop we have to run through always
irrespective of whether an extension exists of not.

How about adding a new API method

Dart_Handle Dart_GetExtensions();
that returns a list of tuples
<library, extension URL> of the extensions that have been loaded
into a library.

[Cutch, 2016/11/03 17:45:34]

I agree with the spirit, but, extensions are technically part of the embedder
and the scheme (dart-ext: is not standardized).

[siva, 2016/11/03 17:53:01]

Agree, maybe the API could be something like
Dart_Handle Dart_GetLibrariesMatchingScheme(Dart_Handle scheme)

+
+  return Dart_True();
+}
+
+
 Dart_Handle Loader::LoadUrlContents(Dart_Handle url,
                                     uint8_t** payload,
                                     intptr_t* payload_length) {