From ba310270c1764f51d75b998007a3847daaba1fad Mon Sep 17 00:00:00 2001
From: Chinmay Garde <chinmaygarde@google.com>
Date: Tue, 5 Mar 2019 14:04:15 -0800
Subject: [PATCH] Fix weak pointer use violations in shell and platform view.

---
 shell/common/platform_view.cc | 17 ++++++++++++++---
 shell/common/platform_view.h  |  2 ++
 shell/common/shell.cc         | 25 +++++++++++++------------
 shell/common/surface.h        |  2 +-
 4 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/shell/common/platform_view.cc b/shell/common/platform_view.cc
index 7c7c1dc258aad..cd909814c66b8 100644
--- a/shell/common/platform_view.cc
+++ b/shell/common/platform_view.cc
@@ -60,7 +60,20 @@ void PlatformView::SetViewportMetrics(const blink::ViewportMetrics& metrics) {
 }
 
 void PlatformView::NotifyCreated() {
-  delegate_.OnPlatformViewCreated(CreateRenderingSurface());
+  std::unique_ptr<Surface> surface;
+
+  // Threading: We want to use the platform view on the non-platform thread.
+  // Using the weak pointer is illegal. But, we are going to introduce a latch
+  // so that the platform view is not collected till the surface is obtained.
+  auto* platform_view = this;
+  fml::ManualResetWaitableEvent latch;
+  fml::TaskRunner::RunNowOrPostTask(
+      task_runners_.GetGPUTaskRunner(), [platform_view, &surface, &latch]() {
+        surface = platform_view->CreateRenderingSurface();
+        latch.Signal();
+      });
+  latch.Wait();
+  delegate_.OnPlatformViewCreated(std::move(surface));
 }
 
 void PlatformView::NotifyDestroyed() {
@@ -68,10 +81,8 @@ void PlatformView::NotifyDestroyed() {
 }
 
 sk_sp<GrContext> PlatformView::CreateResourceContext() const {
-#ifndef OS_FUCHSIA
   FML_DLOG(WARNING) << "This platform does not setup the resource "
                        "context on the IO thread for async texture uploads.";
-#endif  // OS_FUCHSIA
   return nullptr;
 }
 
diff --git a/shell/common/platform_view.h b/shell/common/platform_view.h
index ed3b6e999a3ea..2ed5091b46895 100644
--- a/shell/common/platform_view.h
+++ b/shell/common/platform_view.h
@@ -123,6 +123,8 @@ class PlatformView {
   SkISize size_;
   fml::WeakPtrFactory<PlatformView> weak_factory_;
 
+  // Unlike all other methods on the platform view, this is called on the GPU
+  // task runner.
   virtual std::unique_ptr<Surface> CreateRenderingSurface();
 
  private:
diff --git a/shell/common/shell.cc b/shell/common/shell.cc
index 4a501d3af7d31..ef177eacabcc3 100644
--- a/shell/common/shell.cc
+++ b/shell/common/shell.cc
@@ -446,26 +446,27 @@ void Shell::OnPlatformViewCreated(std::unique_ptr<Surface> surface) {
     fml::TaskRunner::RunNowOrPostTask(gpu_task_runner, gpu_task);
   };
 
-  auto io_task = [io_manager = io_manager_->GetWeakPtr(),
-                  platform_view = platform_view_->GetWeakPtr(),
+  // Threading: Capture platform view by raw pointer and not the weak pointer.
+  // We are going to use the pointer on the IO thread which is not safe with a
+  // weak pointer. However, we are preventing the platform view from being
+  // collected by using a latch.
+  auto* platform_view = platform_view_.get();
+
+  FML_DCHECK(platform_view);
+
+  auto io_task = [io_manager = io_manager_->GetWeakPtr(), platform_view,
                   ui_task_runner = task_runners_.GetUITaskRunner(), ui_task] {
-    if (io_manager) {
+    if (io_manager && !io_manager->GetResourceContext()) {
       io_manager->NotifyResourceContextAvailable(
-          platform_view ? platform_view->CreateResourceContext() : nullptr);
+          platform_view->CreateResourceContext());
     }
     // Step 1: Next, post a task on the UI thread to tell the engine that it has
     // an output surface.
     fml::TaskRunner::RunNowOrPostTask(ui_task_runner, ui_task);
   };
 
-  // Step 0: If the IOManager doesn't already have a ResourceContext, tell the
-  // IO thread that the PlatformView can make one for it now.
-  // Otherwise, jump right to step 1 on the UI thread.
-  if (!io_manager_->GetResourceContext()) {
-    fml::TaskRunner::RunNowOrPostTask(task_runners_.GetIOTaskRunner(), io_task);
-  } else {
-    fml::TaskRunner::RunNowOrPostTask(task_runners_.GetUITaskRunner(), ui_task);
-  }
+  fml::TaskRunner::RunNowOrPostTask(task_runners_.GetIOTaskRunner(), io_task);
+
   latch.Wait();
 }
 
diff --git a/shell/common/surface.h b/shell/common/surface.h
index d82453a00dd29..dc37c803d9ce9 100644
--- a/shell/common/surface.h
+++ b/shell/common/surface.h
@@ -15,7 +15,7 @@
 namespace shell {
 
 /// Represents a Frame that has been fully configured for the underlying client
-/// rendering API. A frame may only be sumitted once.
+/// rendering API. A frame may only be submitted once.
 class SurfaceFrame {
  public:
   using SubmitCallback =