Fix shutdown crash in Sky

Previously, we would try to close some Dart persistent handles after destroying
the isolate. Now we revoke the weak pointers these handles are using to refer
to the isolate before trying to tear them down.

Fixes https://github.com/domokit/mojo/issues/233

R=chinmaygarde@google.com, eseidel@chromium.org

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

engine/core/script/dart_controller.cc

@@ -246,7 +246,7 @@ static Dart_Isolate IsolateCreateCallback(const char* script_uri,
                                               nullptr,
                                               error);
     CHECK(isolate) << error;
-    dart_state->set_isolate(isolate);
+    dart_state->SetIsolate(isolate);
     CHECK(Dart_IsServiceIsolate(isolate));
     CHECK(!LogIfError(Dart_SetLibraryTagHandler(LibraryTagHandler)));
     {
@@ -276,7 +276,7 @@ static Dart_Isolate IsolateCreateCallback(const char* script_uri,
   Dart_Isolate isolate = Dart_CreateIsolate("sky:handle_watcher", "",
       kDartIsolateSnapshotBuffer, dart_state, error);
   CHECK(isolate) << error;
-  dart_state->set_isolate(isolate);
+  dart_state->SetIsolate(isolate);
 
   CHECK(!LogIfError(Dart_SetLibraryTagHandler(LibraryTagHandler)));
 
@@ -324,7 +324,7 @@ void DartController::CreateIsolateFor(PassOwnPtr<DOMDartState> state) {
       static_cast<DartState*>(dom_dart_state_.get()), &error);
   Dart_SetMessageNotifyCallback(MessageNotifyCallback);
   CHECK(isolate) << error;
-  dom_dart_state_->set_isolate(isolate);
+  dom_dart_state_->SetIsolate(isolate);
   Dart_SetGcCallbacks(DartGCPrologue, DartGCEpilogue);
   CHECK(!LogIfError(Dart_SetLibraryTagHandler(LibraryTagHandler)));
 
@@ -362,6 +362,7 @@ void DartController::ClearForClose() {
   // Don't use a DartIsolateScope here since we never exit the isolate.
   Dart_EnterIsolate(dom_dart_state_->isolate());
   Dart_ShutdownIsolate();
+  dom_dart_state_->SetIsolate(nullptr);
   dom_dart_state_.clear();
 }
 

engine/core/script/dom_dart_state.cc

@@ -16,6 +16,10 @@ DOMDartState::DOMDartState(Document* document, const KURL& url)
 }
 
 DOMDartState::~DOMDartState() {
+  // We've already destroyed the isolate. Revoke any weak ptrs held by
+  // DartPersistentValues so they don't try to enter the destroyed isolate to
+  // clean themselves up.
+  weak_factory_.InvalidateWeakPtrs();
 }
 
 void DOMDartState::DidSetIsolate() {

engine/tonic/dart_state.cc

@@ -32,11 +32,17 @@ DartState::DartState()
 DartState::~DartState() {
 }
 
-void DartState::DidSetIsolateInternal() {
+void DartState::SetIsolate(Dart_Isolate isolate) {
+  CHECK(!isolate_);
+  isolate_ = isolate;
+  if (!isolate_)
+    return;
+
   {
     Scope dart_scope(this);
     index_handle_.Set(this, ToDart("index"));
   }
+
   DidSetIsolate();
 }
 

engine/tonic/dart_state.h

@@ -48,11 +48,7 @@ class DartState : public base::SupportsUserData {
   base::WeakPtr<DartState> GetWeakPtr();
 
   Dart_Isolate isolate() { return isolate_; }
-  void set_isolate(Dart_Isolate isolate) {
-    CHECK(!isolate_);
-    isolate_ = isolate;
-    DidSetIsolateInternal();
-  }
+  void SetIsolate(Dart_Isolate isolate);
 
   DartClassLibrary& class_library() { return *class_library_; }
   DartStringCache& string_cache() { return *string_cache_; }
@@ -64,16 +60,15 @@ class DartState : public base::SupportsUserData {
   virtual void DidSetIsolate() {}
 
  private:
-  void DidSetIsolateInternal();
-
   Dart_Isolate isolate_;
   OwnPtr<DartClassLibrary> class_library_;
   OwnPtr<DartStringCache> string_cache_;
   OwnPtr<DartExceptionFactory> exception_factory_;
   OwnPtr<DartTimerHeap> timer_heap_;
+  DartPersistentValue index_handle_;
 
+ protected:
   base::WeakPtrFactory<DartState> weak_factory_;
-  DartPersistentValue index_handle_;
 
   DISALLOW_COPY_AND_ASSIGN(DartState);
 };