Merge only gpu and platform threads for platform views, fix deadlock. (…

…flutter#8045)

The reason we didn't merge just the gpu and platform threads from the get go was a deadlock in Shell:OnPlatformViewCreated and Shell:OnPlatformViewDestroyed.

The deadlock was caused by the platform thread starting a thread-hopping flow that ends ends up with the gpu thread releasing a latch that the platform thread is waiting on just after starting the cross-thread dance.
If the platform and gpu threads are the same, that last task that is posted to the gpu thread will never get executed as the gpu/platform thread is blocked on a latch.

This works around the deadlock by having a special case in the code for the scenario where the gpu and platform threads are the same.

Fixes: flutter/flutter#23974

shell/common/shell.cc

@@ -437,16 +437,35 @@ void Shell::OnPlatformViewCreated(std::unique_ptr<Surface> surface) {
     latch.Signal();
   });
 
+  // The normal flow exectued by this method is that the platform thread is
+  // starting the sequence and waiting on the latch. Later the UI thread posts
+  // gpu_task to the GPU thread which signals the latch. If the GPU the and
+  // platform threads are the same this results in a deadlock as the gpu_task
+  // will never be posted to the plaform/gpu thread that is blocked on a latch.
+  // To avoid the described deadlock, if the gpu and the platform threads are
+  // the same, should_post_gpu_task will be false, and then instead of posting a
+  // task to the gpu thread, the ui thread just signals the latch and the
+  // platform/gpu thread follows with executing gpu_task.
+  bool should_post_gpu_task =
+      task_runners_.GetGPUTaskRunner() != task_runners_.GetPlatformTaskRunner();
+
   auto ui_task = [engine = engine_->GetWeakPtr(),                      //
                   gpu_task_runner = task_runners_.GetGPUTaskRunner(),  //
-                  gpu_task                                             //
+                  gpu_task, should_post_gpu_task,
+                  &latch  //
   ] {
     if (engine) {
       engine->OnOutputSurfaceCreated();
     }
     // Step 2: Next, tell the GPU thread that it should create a surface for its
     // rasterizer.
-    fml::TaskRunner::RunNowOrPostTask(gpu_task_runner, gpu_task);
+    if (should_post_gpu_task) {
+      fml::TaskRunner::RunNowOrPostTask(gpu_task_runner, gpu_task);
+    } else {
+      // See comment on should_post_gpu_task, in this case we just unblock
+      // the platform thread.
+      latch.Signal();
+    }
   };
 
   // Threading: Capture platform view by raw pointer and not the weak pointer.
@@ -471,6 +490,12 @@ void Shell::OnPlatformViewCreated(std::unique_ptr<Surface> surface) {
   fml::TaskRunner::RunNowOrPostTask(task_runners_.GetIOTaskRunner(), io_task);
 
   latch.Wait();
+  if (!should_post_gpu_task) {
+    // See comment on should_post_gpu_task, in this case the gpu_task
+    // wasn't executed, and we just run it here as the platform thread
+    // is the GPU thread.
+    gpu_task();
+  }
 }
 
 // |shell::PlatformView::Delegate|
@@ -504,21 +529,46 @@ void Shell::OnPlatformViewDestroyed() {
     fml::TaskRunner::RunNowOrPostTask(io_task_runner, io_task);
   };
 
+  // The normal flow exectued by this method is that the platform thread is
+  // starting the sequence and waiting on the latch. Later the UI thread posts
+  // gpu_task to the GPU thread triggers signaling the latch(on the IO thread).
+  // If the GPU the and platform threads are the same this results in a deadlock
+  // as the gpu_task will never be posted to the plaform/gpu thread that is
+  // blocked on a latch.  To avoid the described deadlock, if the gpu and the
+  // platform threads are the same, should_post_gpu_task will be false, and then
+  // instead of posting a task to the gpu thread, the ui thread just signals the
+  // latch and the platform/gpu thread follows with executing gpu_task.
+  bool should_post_gpu_task =
+      task_runners_.GetGPUTaskRunner() != task_runners_.GetPlatformTaskRunner();
+
   auto ui_task = [engine = engine_->GetWeakPtr(),
-                  gpu_task_runner = task_runners_.GetGPUTaskRunner(),
-                  gpu_task]() {
+                  gpu_task_runner = task_runners_.GetGPUTaskRunner(), gpu_task,
+                  should_post_gpu_task, &latch]() {
     if (engine) {
       engine->OnOutputSurfaceDestroyed();
     }
     // Step 1: Next, tell the GPU thread that its rasterizer should suspend
     // access to the underlying surface.
-    fml::TaskRunner::RunNowOrPostTask(gpu_task_runner, gpu_task);
+    if (should_post_gpu_task) {
+      fml::TaskRunner::RunNowOrPostTask(gpu_task_runner, gpu_task);
+    } else {
+      // See comment on should_post_gpu_task, in this case we just unblock
+      // the platform thread.
+      latch.Signal();
+    }
   };
 
   // Step 0: Post a task onto the UI thread to tell the engine that its output
   // surface is about to go away.
   fml::TaskRunner::RunNowOrPostTask(task_runners_.GetUITaskRunner(), ui_task);
   latch.Wait();
+  if (!should_post_gpu_task) {
+    // See comment on should_post_gpu_task, in this case the gpu_task
+    // wasn't executed, and we just run it here as the platform thread
+    // is the GPU thread.
+    gpu_task();
+    latch.Wait();
+  }
 }
 
 // |shell::PlatformView::Delegate|

shell/common/shell_unittests.cc

@@ -193,7 +193,7 @@ TEST(ShellTest, InitializeWithMultipleThreadButCallingThreadAsPlatformThread) {
 
 // Reported in Bug: Engine deadlocks when gpu and platforms threads are the same
 // #21398 (https://github.com/flutter/flutter/issues/21398)
-TEST(ShellTest, DISABLED_InitializeWithGPUAndPlatformThreadsTheSame) {
+TEST(ShellTest, InitializeWithGPUAndPlatformThreadsTheSame) {
   blink::Settings settings = {};
   settings.task_observer_add = [](intptr_t, fml::closure) {};
   settings.task_observer_remove = [](intptr_t) {};

shell/platform/darwin/ios/framework/Source/FlutterEngine.mm

@@ -355,16 +355,16 @@ - (BOOL)createShell:(NSString*)entrypoint libraryURI:(NSString*)libraryURI {
     // Embedded views requires the gpu and the platform views to be the same.
     // The plan is to eventually dynamically merge the threads when there's a
     // platform view in the layer tree.
-    // For now we run in a single threaded configuration.
-    // TODO(amirh/chinmaygarde): merge only the gpu and platform threads.
-    // https://github.com/flutter/flutter/issues/23974
+    // For now we use a fixed thread configuration with the same thread used as the
+    // gpu and platform task runner.
     // TODO(amirh/chinmaygarde): remove this, and dynamically change the thread configuration.
     // https://github.com/flutter/flutter/issues/23975
+
     blink::TaskRunners task_runners(threadLabel.UTF8String,                          // label
                                     fml::MessageLoop::GetCurrent().GetTaskRunner(),  // platform
                                     fml::MessageLoop::GetCurrent().GetTaskRunner(),  // gpu
-                                    fml::MessageLoop::GetCurrent().GetTaskRunner(),  // ui
-                                    fml::MessageLoop::GetCurrent().GetTaskRunner()   // io
+                                    _threadHost.ui_thread->GetTaskRunner(),          // ui
+                                    _threadHost.io_thread->GetTaskRunner()           // io
     );
     // Create the shell. This is a blocking operation.
     _shell = shell::Shell::Create(std::move(task_runners),  // task runners