Fix weak pointer use violations in shell and platform view. (#8046)

Author: chinmaygarde

shell/common/platform_view.cc

@@ -60,18 +60,29 @@ 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() {
   delegate_.OnPlatformViewDestroyed();
 }
 
 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;
 }
 

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:

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();
 }
 

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 =