From 23a36caa6f6caacbc6e03cd2f86590180d6400ea Mon Sep 17 00:00:00 2001
From: Jason Simmons <jason-simmons@users.noreply.github.com>
Date: Tue, 31 Jan 2017 17:54:34 -0800
Subject: [PATCH] Fix a race in PlatformView construction (flutter/engine#3380)

The PlatformView superclass constructor was posting a task to the UI thread
that adds the view to the shell's global list.  This could result in UI thread
operations seeing PlatformView instances that are not fully constructed and do
not yet have an engine.

This was happening in https://github.com/flutter/flutter/issues/7735
---
 engine/src/flutter/shell/common/platform_view.cc     | 12 ++++++++----
 engine/src/flutter/shell/common/platform_view.h      |  1 +
 .../shell/platform/android/platform_view_android.cc  |  2 ++
 .../platform/darwin/desktop/platform_view_mac.mm     |  2 ++
 .../shell/platform/darwin/ios/platform_view_ios.mm   |  2 ++
 .../shell/platform/linux/platform_view_glfw.cc       |  2 ++
 .../src/flutter/shell/testing/platform_view_test.cc  |  1 +
 7 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/engine/src/flutter/shell/common/platform_view.cc b/engine/src/flutter/shell/common/platform_view.cc
index 644d92160c..7f0839b116 100644
--- a/engine/src/flutter/shell/common/platform_view.cc
+++ b/engine/src/flutter/shell/common/platform_view.cc
@@ -18,10 +18,7 @@ namespace shell {
 PlatformView::PlatformView(std::unique_ptr<Rasterizer> rasterizer)
     : rasterizer_(std::move(rasterizer)),
       size_(SkISize::Make(0, 0)),
-      weak_factory_(this) {
-  blink::Threads::UI()->PostTask(
-      [self = GetWeakPtr()] { Shell::Shared().AddPlatformView(self); });
-}
+      weak_factory_(this) {}
 
 PlatformView::~PlatformView() {
   blink::Threads::UI()->PostTask([] { Shell::Shared().PurgePlatformViews(); });
@@ -37,6 +34,13 @@ void PlatformView::CreateEngine() {
   engine_.reset(new Engine(this));
 }
 
+// Add this to the shell's list of PlatformVIews.
+// Subclasses should call this after the object is fully constructed.
+void PlatformView::PostAddToShellTask() {
+  blink::Threads::UI()->PostTask(
+      [self = GetWeakPtr()] { Shell::Shared().AddPlatformView(self); });
+}
+
 void PlatformView::DispatchPlatformMessage(
     ftl::RefPtr<blink::PlatformMessage> message) {
   blink::Threads::UI()->PostTask(
diff --git a/engine/src/flutter/shell/common/platform_view.h b/engine/src/flutter/shell/common/platform_view.h
index 692fff9260..5bef2e661b 100644
--- a/engine/src/flutter/shell/common/platform_view.h
+++ b/engine/src/flutter/shell/common/platform_view.h
@@ -70,6 +70,7 @@ class PlatformView {
   explicit PlatformView(std::unique_ptr<Rasterizer> rasterizer);
 
   void CreateEngine();
+  void PostAddToShellTask();
 
   void SetupResourceContextOnIOThreadPerform(
       ftl::AutoResetWaitableEvent* event);
diff --git a/engine/src/flutter/shell/platform/android/platform_view_android.cc b/engine/src/flutter/shell/platform/android/platform_view_android.cc
index 00bcdbd29f..67a054ce0d 100644
--- a/engine/src/flutter/shell/platform/android/platform_view_android.cc
+++ b/engine/src/flutter/shell/platform/android/platform_view_android.cc
@@ -104,6 +104,8 @@ PlatformViewAndroid::PlatformViewAndroid()
   SetupResourceContextOnIOThread();
 
   UpdateThreadPriorities();
+
+  PostAddToShellTask();
 }
 
 PlatformViewAndroid::~PlatformViewAndroid() = default;
diff --git a/engine/src/flutter/shell/platform/darwin/desktop/platform_view_mac.mm b/engine/src/flutter/shell/platform/darwin/desktop/platform_view_mac.mm
index 271b4180f1..018133460a 100644
--- a/engine/src/flutter/shell/platform/darwin/desktop/platform_view_mac.mm
+++ b/engine/src/flutter/shell/platform/darwin/desktop/platform_view_mac.mm
@@ -34,6 +34,8 @@ PlatformViewMac::PlatformViewMac(NSOpenGLView* gl_view)
     shell::Shell::Shared().tracing_controller().set_traces_base_path(
         [[paths objectAtIndex:0] UTF8String]);
   }
+
+  PostAddToShellTask();
 }
 
 PlatformViewMac::~PlatformViewMac() = default;
diff --git a/engine/src/flutter/shell/platform/darwin/ios/platform_view_ios.mm b/engine/src/flutter/shell/platform/darwin/ios/platform_view_ios.mm
index b17001fd3b..96e187f56c 100644
--- a/engine/src/flutter/shell/platform/darwin/ios/platform_view_ios.mm
+++ b/engine/src/flutter/shell/platform/darwin/ios/platform_view_ios.mm
@@ -282,6 +282,8 @@ PlatformViewIOS::PlatformViewIOS(CAEAGLLayer* layer)
                                                        NSUserDomainMask, YES);
   shell::Shell::Shared().tracing_controller().set_traces_base_path(
       [paths.firstObject UTF8String]);
+
+  PostAddToShellTask();
 }
 
 PlatformViewIOS::~PlatformViewIOS() = default;
diff --git a/engine/src/flutter/shell/platform/linux/platform_view_glfw.cc b/engine/src/flutter/shell/platform/linux/platform_view_glfw.cc
index aaa93016c4..5fdeb860d9 100644
--- a/engine/src/flutter/shell/platform/linux/platform_view_glfw.cc
+++ b/engine/src/flutter/shell/platform/linux/platform_view_glfw.cc
@@ -49,6 +49,8 @@ PlatformViewGLFW::PlatformViewGLFW()
   });
 
   valid_ = true;
+
+  PostAddToShellTask();
 }
 
 PlatformViewGLFW::~PlatformViewGLFW() {
diff --git a/engine/src/flutter/shell/testing/platform_view_test.cc b/engine/src/flutter/shell/testing/platform_view_test.cc
index 1e5a718567..351416a9c2 100644
--- a/engine/src/flutter/shell/testing/platform_view_test.cc
+++ b/engine/src/flutter/shell/testing/platform_view_test.cc
@@ -12,6 +12,7 @@ namespace shell {
 PlatformViewTest::PlatformViewTest()
     : PlatformView(std::unique_ptr<Rasterizer>(new NullRasterizer())) {
   CreateEngine();
+  PostAddToShellTask();
 }
 
 PlatformViewTest::~PlatformViewTest() = default;