Fix for issue 12526 (flutter#4218)

* Fix for issue 12526

Ensure that child isolates do not clear the dart_ui_state_ field present in the dart controller.

The commit flutter@dd1e0b5 implemented code to reset the dart_ui_state_ back to null when an isolate was being shutdown to ensure there was no use after free issues when the main isolate exeutes Isolate.current.kill() it however it was also clearning the field when a child isolate was shutdown causing SEGVs later.

* Address code format issues.

lib/ui/ui_dart_state.h

@@ -20,7 +20,6 @@ class Window;
 class IsolateClient {
  public:
   virtual void DidCreateSecondaryIsolate(Dart_Isolate isolate) = 0;
-  virtual void WillShutDownIsolate(Dart_Isolate isolate) = 0;
 
  protected:
   virtual ~IsolateClient();
@@ -35,7 +34,7 @@ class UIDartState : public tonic::DartState {
 
   UIDartState* CreateForChildIsolate();
 
-  IsolateClient* isolate_client() { return isolate_client_; }
+  IsolateClient* isolate_client() const { return isolate_client_; }
   void set_isolate_client(IsolateClient* isolate_client) {
     isolate_client_ = isolate_client;
   }
@@ -46,6 +45,8 @@ class UIDartState : public tonic::DartState {
   void set_debug_name_prefix(const std::string& debug_name_prefix);
   void set_font_selector(PassRefPtr<FontSelector> selector);
   PassRefPtr<FontSelector> font_selector();
+  bool is_controller_state() const { return is_controller_state_; }
+  void set_is_controller_state(bool value) { is_controller_state_ = value; }
 
  private:
   void DidSetIsolate() override;
@@ -56,6 +57,7 @@ class UIDartState : public tonic::DartState {
   std::string debug_name_;
   std::unique_ptr<Window> window_;
   RefPtr<FontSelector> font_selector_;
+  bool is_controller_state_;
 };
 
 }  // namespace blink

runtime/dart_controller.cc

@@ -54,8 +54,8 @@ DartController::~DartController() {
     Dart_EnterIsolate(ui_dart_state_->isolate());
     // Clear the message notify callback.
     Dart_SetMessageNotifyCallback(nullptr);
-    Dart_ShutdownIsolate();  // deletes ui_dart_state_
-    ui_dart_state_ = nullptr;
+    Dart_ShutdownIsolate();
+    delete ui_dart_state_;
   }
   if (platform_kernel_bytes) {
     free(platform_kernel_bytes);
@@ -204,6 +204,7 @@ void DartController::CreateIsolateFor(
   }
   FXL_CHECK(isolate) << error;
   ui_dart_state_ = state.release();
+  ui_dart_state_->set_is_controller_state(true);
   dart_state()->message_handler().Initialize(blink::Threads::UI());
 
   Dart_SetShouldPauseOnStart(Settings::Get().start_paused);
@@ -227,8 +228,4 @@ void DartController::CreateIsolateFor(
   Dart_ExitIsolate();
 }
 
-void DartController::IsolateShuttingDown() {
-  ui_dart_state_ = nullptr;
-}
-
 }  // namespace blink

runtime/dart_controller.h

@@ -40,8 +40,9 @@ class DartController {
  private:
   bool SendStartMessage(Dart_Handle root_library);
 
-  // The DartState associated with the main isolate.  This will be deleted
-  // during isolate shutdown.
+  // The DartState associated with the main isolate.  This is not deleted
+  // during isolate shutdown, instead it is deleted when the controller
+  // object is deleted.
   UIDartState* ui_dart_state_;
 
   // Kernel binary image of platform core libraries. This is copied and

runtime/dart_init.cc

@@ -132,11 +132,9 @@ void IsolateShutdownCallback(void* callback_data) {
     FXL_CHECK(LogIfError(sticky_error));
   }
   UIDartState* dart_state = static_cast<UIDartState*>(callback_data);
-  IsolateClient* isolate_client = dart_state->isolate_client();
-  if (isolate_client) {
-    isolate_client->WillShutDownIsolate(dart_state->isolate());
+  if ((dart_state != NULL) && !dart_state->is_controller_state()) {
+    delete dart_state;
   }
-  delete dart_state;
 }
 
 bool DartFileModifiedCallback(const char* source_url, int64_t since_ms) {
@@ -296,6 +294,7 @@ Dart_Isolate IsolateCreateCallback(const char* script_uri,
                                g_default_isolate_snapshot_instructions, nullptr,
                                dart_state, error);
   FXL_CHECK(isolate) << error;
+  dart_state->set_debug_name_prefix(script_uri);
   dart_state->SetIsolate(isolate);
   FXL_CHECK(!LogIfError(
       Dart_SetLibraryTagHandler(tonic::DartState::HandleLibraryTag)));

runtime/runtime_controller.cc

@@ -144,10 +144,6 @@ void RuntimeController::DidCreateSecondaryIsolate(Dart_Isolate isolate) {
   client_->DidCreateSecondaryIsolate(isolate);
 }
 
-void RuntimeController::WillShutDownIsolate(Dart_Isolate isolate) {
-  dart_controller_->IsolateShuttingDown();
-}
-
 Dart_Port RuntimeController::GetMainPort() {
   if (!dart_controller_) {
     return ILLEGAL_PORT;

runtime/runtime_controller.h

@@ -62,7 +62,6 @@ class RuntimeController : public WindowClient, public IsolateClient {
   void HandlePlatformMessage(fxl::RefPtr<PlatformMessage> message) override;
 
   void DidCreateSecondaryIsolate(Dart_Isolate isolate) override;
-  void WillShutDownIsolate(Dart_Isolate isolate) override;
 
   RuntimeDelegate* client_;
   std::string language_code_;