Fix deferred load errors

R=hausner@google.com

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

runtime/bin/loader.cc

@@ -212,18 +212,20 @@ void Loader::BlockUntilComplete() {
 }
 
 
-bool Loader::ProcessResultLocked(Loader::IOResult* result) {
-  // A negative result tag indicates a loading error occurred in the service
-  // isolate. The payload is a C string of the error message.
-  if (result->tag < 0) {
-    error_ =
-        Dart_NewUnhandledExceptionError(
-            Dart_NewStringFromUTF8(result->payload,
-                                   result->payload_length));
-
-    return false;
+static bool LibraryHandleError(Dart_Handle library, Dart_Handle error) {
+  if (!Dart_IsNull(library) && !Dart_IsError(library)) {
+    ASSERT(Dart_IsLibrary(library));
+    Dart_Handle res = Dart_LibraryHandleError(library, error);
+    if (Dart_IsNull(res)) {
+      // Error was handled by library.
+      return true;
+    }
   }
+  return false;
+}
 
+
+bool Loader::ProcessResultLocked(Loader::IOResult* result) {
   // We have to copy everything we care about out of |result| because after
   // dropping the lock below |result| may no longer valid.
   Dart_Handle uri =
@@ -233,6 +235,25 @@ bool Loader::ProcessResultLocked(Loader::IOResult* result) {
     library_uri =
         Dart_NewStringFromCString(reinterpret_cast<char*>(result->library_uri));
   }
+
+  // A negative result tag indicates a loading error occurred in the service
+  // isolate. The payload is a C string of the error message.
+  if (result->tag < 0) {
+    Dart_Handle library = Dart_LookupLibrary(uri);
+    Dart_Handle error = Dart_NewStringFromUTF8(result->payload,
+                                               result->payload_length);
+    // If a library with the given uri exists, give it a chance to handle
+    // the error. If the load requests stems from a deferred library load,
+    // an IO error is not fatal.
+    if (LibraryHandleError(library, error)) {
+      return true;
+    }
+    // Fall through
+    error_ = Dart_NewUnhandledExceptionError(error);
+    return false;
+  }
+
+
   // Check for payload and load accordingly.
   bool is_snapshot = false;
   const uint8_t* payload = result->payload;
@@ -402,8 +423,11 @@ Dart_Handle Loader::LibraryTagHandler(Dart_LibraryTag tag,
 
   // The outer invocation of the tag handler for this isolate. We make the outer
   // invocation block and allow any nested invocations to operate in parallel.
-  bool blocking_call = !isolate_data->HasLoader();
+  const bool blocking_call = !isolate_data->HasLoader();
 
+  // If we are the outer invocation of the tag handler and the tag is an import
+  // this means that we are starting a deferred library load.
+  const bool is_deferred_import = blocking_call && (tag == Dart_kImportTag);
   if (!isolate_data->HasLoader()) {
     // The isolate doesn't have a loader -- this is the outer invocation which
     // will block.
@@ -440,8 +464,23 @@ Dart_Handle Loader::LibraryTagHandler(Dart_LibraryTag tag,
     delete loader;
 
     // An error occurred during loading.
-    if (Dart_IsError(error)) {
-      return error;
+    if (!Dart_IsNull(error)) {
+      if (false && is_deferred_import) {
+        // This blocks handles transitive load errors caused by a deferred
+        // import. Non-transitive load errors are handled above (see callers of
+        // |LibraryHandleError|). To handle the transitive case, we give the
+        // originating deferred library an opportunity to handle it.
+        Dart_Handle deferred_library = Dart_LookupLibrary(url);
+        if (!LibraryHandleError(deferred_library, error)) {
+          // Library did not handle it, return to caller as an unhandled
+          // exception.
+          return Dart_NewUnhandledExceptionError(error);
+        }
+      } else {
+        // We got an error during loading but we aren't loading a deferred
+        // library, return the error to the caller.
+        return error;
+      }
     }
 
     // Finalize loading. This will complete any futures for completed deferred

runtime/bin/loader.h

@@ -38,7 +38,6 @@ class Loader {
   IsolateData* isolate_data_;
   // Remember the first error that occurs during loading.
   Dart_Handle error_;
-
   // This monitor is used to protect the pending operations count and the
   // I/O result queue.
   Monitor* monitor_;

runtime/lib/lib_prefix.dart

@@ -32,7 +32,7 @@ class _LibraryPrefix {
       // prefix. If that is the case, we must invalidate the dependent
       // code and complete the future now since there will be no callback
       // from the VM.
-      if (hasCompleted) {
+      if (hasCompleted && !completer.isCompleted) {
         _invalidateDependentCode();
         completer.complete(true);
         _outstandingLoadRequests.remove(pair);
@@ -53,19 +53,27 @@ _completeDeferredLoads() {
   // which have not completed, remember them for next time in
   // stillOutstandingLoadRequests.
   var stillOutstandingLoadRequests = new List<List>();
-  for (int i = 0; i < _outstandingLoadRequests.length; i++) {
-    var prefix = _outstandingLoadRequests[i][0];
-    if (prefix._load()) {
-      var completer = _outstandingLoadRequests[i][1];
-      var error = prefix._loadError();
-      if (error != null) {
-        completer.completeError(error);
-      } else {
-        prefix._invalidateDependentCode();
-        completer.complete(true);
-      }
+  var completedLoadRequests = new List<List>();
+
+  // Make a copy of the outstandingRequests because the call to _load below
+  // may recursively trigger another call to |_completeDeferredLoads|, which
+  // can cause |_outstandingLoadRequests| to be modified.
+  var outstandingRequests = _outstandingLoadRequests.toList();
+  for (int i = 0; i < outstandingRequests.length; i++) {
+    var prefix = outstandingRequests[i][0];
+    var completer = outstandingRequests[i][1];
+    var error = prefix._loadError();
+    if (completer.isCompleted) {
+      // Already completed. Skip.
+      continue;
+    }
+    if (error != null) {
+      completer.completeError(error);
+    } else if (prefix._load()) {
+      prefix._invalidateDependentCode();
+      completer.complete(true);
     } else {
-      stillOutstandingLoadRequests.add(_outstandingLoadRequests[i]);
+      stillOutstandingLoadRequests.add(outstandingRequests[i]);
     }
   }
   _outstandingLoadRequests = stillOutstandingLoadRequests;

runtime/vm/object.cc

@@ -10936,12 +10936,17 @@ bool LibraryPrefix::LoadLibrary() const {
     pending_deferred_loads.Add(deferred_lib);
     const String& lib_url = String::Handle(zone, deferred_lib.url());
     Dart_LibraryTagHandler handler = isolate->library_tag_handler();
+    Object& obj = Object::Handle(zone);
     {
       TransitionVMToNative transition(thread);
       Api::Scope api_scope(thread);
-      handler(Dart_kImportTag,
-              Api::NewHandle(thread, importer()),
-              Api::NewHandle(thread, lib_url.raw()));
+      obj = Api::UnwrapHandle(
+              handler(Dart_kImportTag,
+                      Api::NewHandle(thread, importer()),
+                      Api::NewHandle(thread, lib_url.raw())));
+    }
+    if (obj.IsError()) {
+      Exceptions::PropagateError(Error::Cast(obj));
     }
   } else {
     // Another load request is in flight.

tests/standalone/deferred/alpha.dart

@@ -0,0 +1,2 @@
+// beta.dart does not exist!
+import 'beta.dart';

tests/standalone/deferred/exists.dart

@@ -0,0 +1 @@
+var x = 99;

tests/standalone/deferred/transitive_error.dart

@@ -0,0 +1 @@
+import 'alpha.dart';

tests/standalone/deferred_transitive_import_error_test.dart

@@ -0,0 +1,29 @@
+// Copyright (c) 2016, 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.
+
+import "package:expect/expect.dart";
+// A deferred library that doesn't exist.
+import 'package:foo/foo.dart' deferred as foo;
+// A deferred library that does exist.
+import 'deferred/exists.dart' deferred as exists;
+// A deferred library that transitively will fail due to a file not found.
+import 'deferred/transitive_error.dart' deferred as te;
+
+main() async {
+  // Attempt to load foo which will fail.
+  var fooError;
+  await foo.loadLibrary().catchError((e) {
+    fooError = e;
+  });
+  Expect.isNotNull(fooError);
+  await exists.loadLibrary();
+  Expect.equals(99, exists.x);
+  /* TODO(johnmccutchan): Implement transitive error reporting.
+  var teError;
+  await te.loadLibrary().catchError((e) {
+    teError = e;
+  });
+  Expect.isNotNull(teError);
+  */
+}