Avoid to freeze the system if hot reloading during debug (#3833)

B3rn475 · jason-simmons · commit 8ba522eeae35 · 2017-07-12T10:25:42.000-07:00
diff --git a/shell/common/engine.cc b/shell/common/engine.cc
@@ -474,7 +474,7 @@ void Engine::Render(std::unique_ptr<flow::LayerTree> layer_tree) {
 
 void Engine::UpdateSemantics(std::vector<blink::SemanticsNode> update) {
   blink::Threads::Platform()->PostTask(ftl::MakeCopyable(
-      [ platform_view = platform_view_, update = std::move(update) ]() mutable {
+      [ platform_view = std::shared_ptr<PlatformView>{platform_view_}, update = std::move(update) ]() mutable {
         if (platform_view)
           platform_view->UpdateSemantics(std::move(update));
       }));
@@ -487,7 +487,7 @@ void Engine::HandlePlatformMessage(
     return;
   }
   blink::Threads::Platform()->PostTask([
-    platform_view = platform_view_, message = std::move(message)
+    platform_view = std::shared_ptr<PlatformView>{platform_view_}, message = std::move(message)
   ]() mutable {
     if (platform_view)
       platform_view->HandlePlatformMessage(std::move(message));
diff --git a/shell/common/engine.h b/shell/common/engine.h
@@ -95,7 +95,7 @@ class Engine : public blink::RuntimeDelegate {
   void HandleAssetPlatformMessage(ftl::RefPtr<blink::PlatformMessage> message);
   bool GetAssetAsBuffer(const std::string& name, std::vector<uint8_t>* data);
 
-  ftl::WeakPtr<PlatformView> platform_view_;
+  std::weak_ptr<PlatformView> platform_view_;
   std::unique_ptr<Animator> animator_;
   std::unique_ptr<blink::RuntimeController> runtime_;
   tonic::DartErrorHandleType load_script_error_;
diff --git a/shell/common/platform_view.cc b/shell/common/platform_view.cc
@@ -18,8 +18,7 @@ namespace shell {
 
 PlatformView::PlatformView(std::unique_ptr<Rasterizer> rasterizer)
     : rasterizer_(std::move(rasterizer)),
-      size_(SkISize::Make(0, 0)),
-      weak_factory_(this) {}
+      size_(SkISize::Make(0, 0)){}
 
 PlatformView::~PlatformView() {
   blink::Threads::UI()->PostTask([] { Shell::Shared().PurgePlatformViews(); });
@@ -39,7 +38,7 @@ void PlatformView::CreateEngine() {
 // Subclasses should call this after the object is fully constructed.
 void PlatformView::PostAddToShellTask() {
   blink::Threads::UI()->PostTask(
-      [self = GetWeakPtr()] { Shell::Shared().AddPlatformView(self); });
+      [self = shared_from_this()] { Shell::Shared().AddPlatformView(self); });
 }
 
 void PlatformView::DispatchPlatformMessage(
@@ -118,8 +117,8 @@ void PlatformView::NotifyDestroyed() {
   latch.Wait();
 }
 
-ftl::WeakPtr<PlatformView> PlatformView::GetWeakPtr() {
-  return weak_factory_.GetWeakPtr();
+std::weak_ptr<PlatformView> PlatformView::GetWeakPtr() {
+  return shared_from_this();
 }
 
 VsyncWaiter* PlatformView::GetVsyncWaiter() {
diff --git a/shell/common/platform_view.h b/shell/common/platform_view.h
@@ -22,7 +22,7 @@ namespace shell {
 
 class Rasterizer;
 
-class PlatformView {
+class PlatformView : public std::enable_shared_from_this<PlatformView> {
  public:
   struct SurfaceConfig {
     uint8_t red_bits = 8;
@@ -48,7 +48,7 @@ class PlatformView {
 
   void NotifyDestroyed();
 
-  ftl::WeakPtr<PlatformView> GetWeakPtr();
+  std::weak_ptr<PlatformView> GetWeakPtr();
 
   // The VsyncWaiter will live at least as long as the PlatformView.
   virtual VsyncWaiter* GetVsyncWaiter();
@@ -81,8 +81,7 @@ class PlatformView {
   std::unique_ptr<VsyncWaiter> vsync_waiter_;
   SkISize size_;
 
- private:
-  ftl::WeakPtrFactory<PlatformView> weak_factory_;
+private:
 
   FTL_DISALLOW_COPY_AND_ASSIGN(PlatformView);
 };
diff --git a/shell/common/shell.cc b/shell/common/shell.cc
@@ -33,8 +33,8 @@ bool IsInvalid(const ftl::WeakPtr<Rasterizer>& rasterizer) {
   return !rasterizer;
 }
 
-bool IsViewInvalid(const ftl::WeakPtr<PlatformView>& platform_view) {
-  return !platform_view;
+bool IsViewInvalid(const std::weak_ptr<PlatformView>& platform_view) {
+  return !(platform_view.expired());
 }
 
 template <typename T>
@@ -242,58 +242,41 @@ void Shell::GetRasterizers(std::vector<ftl::WeakPtr<Rasterizer>>* rasterizers) {
   *rasterizers = rasterizers_;
 }
 
-void Shell::AddPlatformView(const ftl::WeakPtr<PlatformView>& platform_view) {
-  FTL_DCHECK(ui_thread_checker_ &&
-             ui_thread_checker_->IsCreationThreadCurrent());
+void Shell::AddPlatformView(const std::shared_ptr<PlatformView>& platform_view) {
+  std::lock_guard<std::mutex> lk(platform_views_mutex_);
   if (platform_view) {
     platform_views_.push_back(platform_view);
   }
 }
 
 void Shell::PurgePlatformViews() {
-  FTL_DCHECK(ui_thread_checker_ &&
-             ui_thread_checker_->IsCreationThreadCurrent());
+  std::lock_guard<std::mutex> lk(platform_views_mutex_);
   platform_views_.erase(std::remove_if(platform_views_.begin(),
                                        platform_views_.end(), IsViewInvalid),
                         platform_views_.end());
 }
 
 void Shell::GetPlatformViews(
-    std::vector<ftl::WeakPtr<PlatformView>>* platform_views) {
-  FTL_DCHECK(ui_thread_checker_ &&
-             ui_thread_checker_->IsCreationThreadCurrent());
+    std::vector<std::weak_ptr<PlatformView>>* platform_views) {
+  std::lock_guard<std::mutex> lk(platform_views_mutex_);
   *platform_views = platform_views_;
 }
 
 void Shell::WaitForPlatformViewIds(
     std::vector<PlatformViewInfo>* platform_view_ids) {
-  ftl::AutoResetWaitableEvent latch;
-
-  blink::Threads::UI()->PostTask([this, platform_view_ids, &latch]() {
-    WaitForPlatformViewsIdsUIThread(platform_view_ids, &latch);
-  });
-
-  latch.Wait();
-}
-
-void Shell::WaitForPlatformViewsIdsUIThread(
-    std::vector<PlatformViewInfo>* platform_view_ids,
-    ftl::AutoResetWaitableEvent* latch) {
-  std::vector<ftl::WeakPtr<PlatformView>> platform_views;
-  GetPlatformViews(&platform_views);
-  for (auto it = platform_views.begin(); it != platform_views.end(); it++) {
-    PlatformView* view = it->get();
+  std::lock_guard<std::mutex> lk(platform_views_mutex_);
+  for (auto it = platform_views_.begin(); it != platform_views_.end(); it++) {
+    std::shared_ptr <PlatformView> view{*it};
     if (!view) {
       // Skip dead views.
       continue;
     }
     PlatformViewInfo info;
-    info.view_id = reinterpret_cast<uintptr_t>(view);
+    info.view_id = reinterpret_cast<uintptr_t>(view.get());
     info.isolate_id = view->engine().GetUIIsolateMainPort();
     info.isolate_name = view->engine().GetUIIsolateName();
     platform_view_ids->push_back(info);
   }
-  latch->Signal();
 }
 
 void Shell::RunInPlatformView(uintptr_t view_id,
@@ -334,8 +317,9 @@ void Shell::RunInPlatformViewUIThread(uintptr_t view_id,
   *view_existed = false;
 
   for (auto it = platform_views_.begin(); it != platform_views_.end(); it++) {
-    PlatformView* view = it->get();
-    if (reinterpret_cast<uintptr_t>(view) == view_id) {
+    std::shared_ptr<PlatformView> view{*it};
+    if (!view) continue;
+    if (reinterpret_cast<uintptr_t>(view.get()) == view_id) {
       *view_existed = true;
       view->RunFromSource(assets_directory, main, packages);
       *dart_isolate_id = view->engine().GetUIIsolateMainPort();
diff --git a/shell/common/shell.h b/shell/common/shell.h
@@ -15,6 +15,8 @@
 #include "lib/ftl/synchronization/waitable_event.h"
 #include "lib/ftl/tasks/task_runner.h"
 
+#include <mutex>
+
 namespace shell {
 
 class PlatformView;
@@ -43,10 +45,10 @@ class Shell {
   // List of PlatformViews.
 
   // These APIs must only be accessed on UI thread.
-  void AddPlatformView(const ftl::WeakPtr<PlatformView>& platform_view);
+  void AddPlatformView(const std::shared_ptr<PlatformView>& platform_view);
   void PurgePlatformViews();
   void GetPlatformViews(
-      std::vector<ftl::WeakPtr<PlatformView>>* platform_views);
+      std::vector<std::weak_ptr<PlatformView>>* platform_views);
 
   struct PlatformViewInfo {
     uintptr_t view_id;
@@ -76,10 +78,6 @@ class Shell {
   void InitGpuThread();
   void InitUIThread();
 
-  void WaitForPlatformViewsIdsUIThread(
-      std::vector<PlatformViewInfo>* platform_views,
-      ftl::AutoResetWaitableEvent* latch);
-
   void RunInPlatformViewUIThread(uintptr_t view_id,
                                  const std::string& main,
                                  const std::string& packages,
@@ -101,7 +99,9 @@ class Shell {
   TracingController tracing_controller_;
 
   std::vector<ftl::WeakPtr<Rasterizer>> rasterizers_;
-  std::vector<ftl::WeakPtr<PlatformView>> platform_views_;
+  std::vector<std::weak_ptr<PlatformView>> platform_views_;
+
+  std::mutex platform_views_mutex_;
 
   FTL_DISALLOW_COPY_AND_ASSIGN(Shell);
 };
diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc
@@ -37,9 +37,10 @@ class PlatformMessageResponseAndroid : public blink::PlatformMessageResponse {
     ftl::RefPtr<PlatformMessageResponseAndroid> self(this);
     blink::Threads::Platform()->PostTask(
         ftl::MakeCopyable([ self, data = std::move(data) ]() mutable {
-          if (!self->view_)
+          std::shared_ptr<PlatformView> view{self->view_};
+          if (!view)
             return;
-          static_cast<PlatformViewAndroid*>(self->view_.get())
+          static_cast<PlatformViewAndroid*>(view.get())
               ->HandlePlatformMessageResponse(self->response_id_,
                                               std::move(data));
         }));
@@ -48,20 +49,21 @@ class PlatformMessageResponseAndroid : public blink::PlatformMessageResponse {
   void CompleteEmpty() override {
     ftl::RefPtr<PlatformMessageResponseAndroid> self(this);
     blink::Threads::Platform()->PostTask(ftl::MakeCopyable([self]() mutable {
-      if (!self->view_)
+      std::shared_ptr<PlatformView> view{self->view_};
+      if (!view)
         return;
-      static_cast<PlatformViewAndroid*>(self->view_.get())
+      static_cast<PlatformViewAndroid*>(view.get())
           ->HandlePlatformMessageEmptyResponse(self->response_id_);
     }));
   }
 
  private:
   PlatformMessageResponseAndroid(int response_id,
-                                 ftl::WeakPtr<PlatformView> view)
+                                 std::weak_ptr<PlatformView> view)
       : response_id_(response_id), view_(view) {}
 
   int response_id_;
-  ftl::WeakPtr<PlatformView> view_;
+  std::weak_ptr<PlatformView> view_;
 };
 
 static std::unique_ptr<AndroidSurface> InitializePlatformSurfaceGL() {
@@ -125,6 +127,11 @@ static std::unique_ptr<AndroidSurface> InitializePlatformSurface() {
 PlatformViewAndroid::PlatformViewAndroid()
     : PlatformView(std::make_unique<GPURasterizer>(nullptr)),
       android_surface_(InitializePlatformSurface()) {
+}
+
+PlatformViewAndroid::~PlatformViewAndroid() = default;
+
+void PlatformViewAndroid::Attach() {
   CreateEngine();
 
   // Eagerly setup the IO thread context. We have already setup the surface.
@@ -135,11 +142,8 @@ PlatformViewAndroid::PlatformViewAndroid()
   PostAddToShellTask();
 }
 
-PlatformViewAndroid::~PlatformViewAndroid() = default;
-
 void PlatformViewAndroid::Detach() {
   ReleaseSurface();
-  delete this;
 }
 
 void PlatformViewAndroid::SurfaceCreated(JNIEnv* env,
diff --git a/shell/platform/android/platform_view_android.h b/shell/platform/android/platform_view_android.h
@@ -28,6 +28,8 @@ class PlatformViewAndroid : public PlatformView {
 
   ~PlatformViewAndroid() override;
 
+  void Attach();
+
   void Detach();
 
   void SurfaceCreated(JNIEnv* env, jobject jsurface, jint backgroundColor);
diff --git a/shell/platform/android/platform_view_android_jni.cc b/shell/platform/android/platform_view_android_jni.cc
@@ -11,7 +11,7 @@
 #include "lib/ftl/arraysize.h"
 #include "lib/ftl/logging.h"
 
-#define PLATFORM_VIEW reinterpret_cast<PlatformViewAndroid*>(platform_view)
+#define PLATFORM_VIEW (*reinterpret_cast<std::shared_ptr<PlatformViewAndroid>*>(platform_view))
 
 namespace shell {
 
@@ -52,15 +52,18 @@ void FlutterViewUpdateSemantics(JNIEnv* env,
 // Called By Java
 
 static jlong Attach(JNIEnv* env, jclass clazz, jobject flutterView) {
-  PlatformViewAndroid* view = new PlatformViewAndroid();
+  auto view = new PlatformViewAndroid();
+  auto storage = new std::shared_ptr<PlatformViewAndroid>(view);
   // Create a weak reference to the flutterView Java object so that we can make
   // calls into it later.
+  view->Attach();
   view->set_flutter_view(fml::jni::JavaObjectWeakGlobalRef(env, flutterView));
-  return reinterpret_cast<jlong>(view);
+  return reinterpret_cast<jlong>(storage);
 }
 
 static void Detach(JNIEnv* env, jobject jcaller, jlong platform_view) {
-  return PLATFORM_VIEW->Detach();
+  PLATFORM_VIEW->Detach();
+  delete &PLATFORM_VIEW;
 }
 
 static jstring GetObservatoryUri(JNIEnv* env, jclass clazz) {
diff --git a/shell/platform/darwin/desktop/flutter_window.mm b/shell/platform/darwin/desktop/flutter_window.mm
@@ -56,6 +56,7 @@ - (void)setupPlatformView {
   FTL_DCHECK(_platformView == nullptr) << "The platform view must not already be set.";
 
   _platformView.reset(new shell::PlatformViewMac(self.renderSurface));
+  _platformView->Attach();
   _platformView->SetupResourceContextOnIOThread();
   _platformView->NotifyCreated(std::make_unique<shell::GPUSurfaceGL>(_platformView.get()));
 }
diff --git a/shell/platform/darwin/desktop/platform_view_mac.h b/shell/platform/darwin/desktop/platform_view_mac.h
@@ -21,6 +21,8 @@ class PlatformViewMac : public PlatformView, public GPUSurfaceGLDelegate {
 
   ~PlatformViewMac() override;
 
+  void Attach();
+
   void SetupAndLoadDart();
 
   bool GLContextMakeCurrent() override;
diff --git a/shell/platform/darwin/desktop/platform_view_mac.mm b/shell/platform/darwin/desktop/platform_view_mac.mm
@@ -24,12 +24,15 @@
       opengl_view_([gl_view retain]),
       resource_loading_context_([[NSOpenGLContext alloc] initWithFormat:gl_view.pixelFormat
                                                            shareContext:gl_view.openGLContext]) {
-  CreateEngine();
-  PostAddToShellTask();
 }
 
 PlatformViewMac::~PlatformViewMac() = default;
 
+void PlatformViewMac::Attach() {
+  CreateEngine();
+  PostAddToShellTask();
+}
+
 void PlatformViewMac::SetupAndLoadDart() {
   if (AttemptLaunchFromCommandLineSwitches(&engine())) {
     // This attempts launching from an FLX bundle that does not contain a
diff --git a/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm b/shell/platform/darwin/ios/framework/Source/FlutterViewController.mm
@@ -120,6 +120,7 @@ - (void)performCommonViewControllerInitialization {
   _statusBarStyle = UIStatusBarStyleDefault;
   _platformView =
       std::make_unique<shell::PlatformViewIOS>(reinterpret_cast<CAEAGLLayer*>(self.view.layer));
+  _platformView->Attach();
   _platformView->SetupResourceContextOnIOThread();
 
   _localizationChannel.reset([[FlutterMethodChannel alloc]
diff --git a/shell/platform/darwin/ios/platform_view_ios.h b/shell/platform/darwin/ios/platform_view_ios.h
@@ -25,6 +25,8 @@ class PlatformViewIOS : public PlatformView {
 
   ~PlatformViewIOS() override;
 
+  void Attach();
+
   void NotifyCreated();
 
   void ToggleAccessibility(UIView* view, bool enabled);
diff --git a/shell/platform/darwin/ios/platform_view_ios.mm b/shell/platform/darwin/ios/platform_view_ios.mm
@@ -21,12 +21,15 @@
     : PlatformView(std::make_unique<GPURasterizer>(std::make_unique<ProcessInfoMac>())),
       ios_surface_(IOSSurface::Create(surface_config_, layer)),
       weak_factory_(this) {
-  CreateEngine();
-  PostAddToShellTask();
 }
 
 PlatformViewIOS::~PlatformViewIOS() = default;
 
+void PlatformViewIOS::Attach() {
+  CreateEngine();
+  PostAddToShellTask();
+}
+
 void PlatformViewIOS::NotifyCreated() {
   PlatformView::NotifyCreated(ios_surface_->CreateGPUSurface());
 }
diff --git a/shell/testing/platform_view_test.cc b/shell/testing/platform_view_test.cc
diff --git a/shell/testing/platform_view_test.h b/shell/testing/platform_view_test.h
diff --git a/shell/testing/test_runner.cc b/shell/testing/test_runner.cc

Original file line number	Diff line number	Diff line change
`@@ -56,6 +56,7 @@ - (void)setupPlatformView {`
`56`	`56`	`FTL_DCHECK(_platformView == nullptr) << "The platform view must not already be set.";`
`57`	`57`
`58`	`58`	`_platformView.reset(new shell::PlatformViewMac(self.renderSurface));`
	`59`	`+ _platformView->Attach();`
`59`	`60`	`_platformView->SetupResourceContextOnIOThread();`
`60`	`61`	`_platformView->NotifyCreated(std::make_unique<shell::GPUSurfaceGL>(_platformView.get()));`
`61`	`62`	`}`