Improve DartLoader error handling.

sky/engine/core/script/dart_loader.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "sky/engine/config.h"
 #include "sky/engine/core/script/dart_loader.h"
 
 #include "base/callback.h"
 #include "mojo/common/data_pipe_drainer.h"
 #include "sky/engine/core/script/dart_dependency_catcher.h"
@@ skipping 41 matching lines @@
   const KURL& url() const { return url_; }
 
  protected:
   DartLoader* loader_;
   // TODO(abarth): Should we be using SharedBuffer to buffer the data?
   Vector<uint8_t> buffer_;
 
  private:
   // MojoFetcher::Client
   void OnReceivedResponse(mojo::URLResponsePtr response) override {
+    if (response->status_code != 200) {
+      loader_->DidFailJob(this);
+      return;
+    }
     // TODO(abarth): Handle network errors.
     drainer_ = adoptPtr(new DataPipeDrainer(this, response->body.Pass()));
   }
 
   // DataPipeDrainer::Client
   void OnDataAvailable(const void* data, size_t num_bytes) override {
     buffer_.append(static_cast<const uint8_t*>(data), num_bytes);
   }
   // Subclasses must implement OnDataComplete.
 
   KURL url_;
   MojoFetcher fetcher_;
   OwnPtr<DataPipeDrainer> drainer_;
 };
 
 class DartLoader::ImportJob : public Job {
  public:
   using Job::Job;
 
  private:
   // DataPipeDrainer::Client
   void OnDataComplete() override {
-    loader_->DidCompleteImportJob(this, buffer_);
+    // DataPipeDrainer calls OnDataComplete regardless of errors.
+    if (!loader_->saw_error())
+      loader_->DidCompleteImportJob(this, buffer_);
   }
 };
 
 class DartLoader::SourceJob : public Job {
  public:
   SourceJob(DartLoader* loader, const KURL& url, Dart_Handle library)
       : Job(loader, url), library_(loader->dart_state(), library) {}
 
   Dart_PersistentHandle library() const { return library_.value(); }
 
  private:
   // DataPipeDrainer::Client
   void OnDataComplete() override {
-    loader_->DidCompleteSourceJob(this, buffer_);
+    // DataPipeDrainer calls OnDataComplete regardless of errors.
+    if (!loader_->saw_error())

[abarth-chromium, 2015/02/13 23:50:29]

Why do we only do this once for the entire lifetime of the isolate?

+      loader_->DidCompleteSourceJob(this, buffer_);
   }
 
   DartPersistentValue library_;
 };
 
 // A DependencyWatcher represents a request to watch for when a given set of
 // dependencies (either libraries or parts of libraries) have finished loading.
 // When the dependencies are satisfied (including transitive dependencies), then
 // the |callback| will be invoked.
 class DartLoader::DependencyWatcher {
@@ skipping 59 matching lines @@
       callback.Run();
   }
 
  private:
   DartLoader& loader_;
   OwnPtr<DartDependencyCatcher> catcher_;
   DartDependency* resolved_dependency_;
 };
 
 DartLoader::DartLoader(DartState* dart_state)
-    : dart_state_(dart_state->GetWeakPtr()), dependency_catcher_(nullptr) {
+    : dart_state_(dart_state->GetWeakPtr()),
+      dependency_catcher_(nullptr),
+      saw_error_(false) {
 }
 
 DartLoader::~DartLoader() {
 }
 
 Dart_Handle DartLoader::HandleLibraryTag(Dart_LibraryTag tag,
                                          Dart_Handle library,
                                          Dart_Handle url) {
   DCHECK(Dart_IsLibrary(library));
   DCHECK(Dart_IsString(url));
@@ skipping 35 matching lines @@
   KURL parsed_url(ParsedURLString, StringFromDart(url));
   OwnPtr<Job> job = adoptPtr(new SourceJob(this, parsed_url, library));
   if (dependency_catcher_)
     dependency_catcher_->AddDependency(job.get());
   jobs_.add(job.release());
   return Dart_True();
 }
 
 void DartLoader::DidCompleteImportJob(ImportJob* job,
                                       const Vector<uint8_t>& buffer) {
+  DCHECK(!saw_error_);
   DCHECK(dart_state_);
   DartIsolateScope scope(dart_state_->isolate());
   DartApiScope api_scope;
 
   WatcherSignaler watcher_signaler(*this, job);
 
   String url_string = job->url().string();
   LogIfError(Dart_LoadLibrary(
       StringToDart(dart_state_.get(), url_string),
       Dart_NewStringFromUTF8(buffer.data(), buffer.size()), 0, 0));
 
   pending_libraries_.remove(url_string);
   jobs_.remove(job);
 }
 
 void DartLoader::DidCompleteSourceJob(SourceJob* job,
                                       const Vector<uint8_t>& buffer) {
+  DCHECK(!saw_error_);
   DCHECK(dart_state_);
   DartIsolateScope scope(dart_state_->isolate());
   DartApiScope api_scope;
 
   WatcherSignaler watcher_signaler(*this, job);
 
   LogIfError(Dart_LoadSource(
       Dart_HandleFromPersistent(job->library()),
       StringToDart(dart_state_.get(), job->url().string()),
       Dart_NewStringFromUTF8(buffer.data(), buffer.size()), 0, 0));
 
   jobs_.remove(job);
 }
 
+void DartLoader::DidFailJob(Job* job) {
+  DCHECK(!saw_error_);
+  saw_error_ = true;
+  DCHECK(dart_state_);
+  DartIsolateScope scope(dart_state_->isolate());
+  DartApiScope api_scope;
+
+  WatcherSignaler watcher_signaler(*this, job);
+
+  LOG(ERROR) << "Library Load failed: " << job->url().string().utf8().data();
+  // TODO(eseidel): Call Dart_LibraryHandleError in the SourceJob case?
+
+  jobs_.remove(job);
+}
+
 }  // namespace blink