From 15f95f64b9d30a4bbf18f20f948b7a9a0e9201d8 Mon Sep 17 00:00:00 2001
From: Forrest Reiling <freiling@google.com>
Date: Wed, 11 Nov 2020 07:56:14 +0000
Subject: [PATCH] [fuchsia] shader warmup fixes

This change contains a couple of changes that should have been in
github.com/flutter/engine/commit/3105db8ee856ffef281d018774d21a6164c81236
but fell through the cracks

First one lifts the initialization of the flutter::RunConfiguration so that
the asset manager gets set on the persistant cache before the shader
warmup happens. I'm not sure how this didnt end up in the first PR I
think it got mangled during merge conflict resolution. no test coverage
for that code because its in the middle of a 400 line constructor

Second one fixes a race condition that the tests dont catch because the
tests are single threaded. This change restructures the test that missed
this bug so that it would have caught that bug and will catch comparable
bugs.
---
 shell/platform/fuchsia/flutter/engine.cc      | 12 +++++++-----
 .../fuchsia/flutter/tests/engine_unittests.cc | 19 +++++++++++++++----
 2 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/shell/platform/fuchsia/flutter/engine.cc b/shell/platform/fuchsia/flutter/engine.cc
index 4ef7e620615d5..8acdbd3cd78d8 100644
--- a/shell/platform/fuchsia/flutter/engine.cc
+++ b/shell/platform/fuchsia/flutter/engine.cc
@@ -201,6 +201,12 @@ Engine::Engine(Delegate& delegate,
         });
       };
 
+  // Launch the engine in the appropriate configuration.
+  // Note: this initializes the Asset Manager on the global PersistantCache
+  // so it must be called before WarmupSkps() is called below.
+  auto run_configuration = flutter::RunConfiguration::InferFromSettings(
+      settings, task_runners.GetIOTaskRunner());
+
   // Setup the callback that will instantiate the platform view.
   flutter::Shell::CreateCallback<flutter::PlatformView>
       on_create_platform_view = fml::MakeCopyable(
@@ -375,10 +381,6 @@ Engine::Engine(Delegate& delegate,
     };
   }
 
-  // Launch the engine in the appropriate configuration.
-  auto run_configuration = flutter::RunConfiguration::InferFromSettings(
-      shell_->GetSettings(), shell_->GetTaskRunners().GetIOTaskRunner());
-
   auto on_run_failure = [weak = weak_factory_.GetWeakPtr()]() {
     // The engine could have been killed by the caller right after the
     // constructor was called but before it could run on the UI thread.
@@ -647,7 +649,7 @@ void Engine::WarmupSkps(fml::BasicTaskRunner* concurrent_task_runner,
 
   // tell concurrent task runner to deserialize all skps available from
   // the asset manager
-  concurrent_task_runner->PostTask([&raster_task_runner, skp_warmup_surface,
+  concurrent_task_runner->PostTask([raster_task_runner, skp_warmup_surface,
                                     &surface_producer]() {
     TRACE_DURATION("flutter", "DeserializeSkps");
     std::vector<std::unique_ptr<fml::Mapping>> skp_mappings =
diff --git a/shell/platform/fuchsia/flutter/tests/engine_unittests.cc b/shell/platform/fuchsia/flutter/tests/engine_unittests.cc
index d3a78b697fec8..95cf266384c3c 100644
--- a/shell/platform/fuchsia/flutter/tests/engine_unittests.cc
+++ b/shell/platform/fuchsia/flutter/tests/engine_unittests.cc
@@ -34,14 +34,22 @@ class MockTaskRunner : public fml::BasicTaskRunner {
   virtual ~MockTaskRunner() {}
 
   void PostTask(const fml::closure& task) override {
-    task_count_++;
-    task();
+    outstanding_tasks_.push(task);
   }
 
   int GetTaskCount() { return task_count_; }
 
+  void Run() {
+    while (!outstanding_tasks_.empty()) {
+      outstanding_tasks_.front()();
+      outstanding_tasks_.pop();
+      task_count_++;
+    }
+  }
+
  private:
   int task_count_ = 0;
+  std::queue<const fml::closure> outstanding_tasks_;
 };
 
 class EngineTest : public ::testing::Test {
@@ -53,15 +61,16 @@ class EngineTest : public ::testing::Test {
 
     fuchsia::ui::scenic::SessionPtr session_ptr;
     scenic::Session session(std::move(session_ptr));
-    VulkanSurfaceProducer surface_producer(&session);
+    surface_producer_ = std::make_unique<VulkanSurfaceProducer>(&session);
 
     Engine::WarmupSkps(&concurrent_task_runner_, &raster_task_runner_,
-                       surface_producer);
+                       *surface_producer_);
   }
 
  protected:
   MockTaskRunner concurrent_task_runner_;
   MockTaskRunner raster_task_runner_;
+  std::unique_ptr<VulkanSurfaceProducer> surface_producer_;
 };
 
 TEST_F(EngineTest, SkpWarmup) {
@@ -100,6 +109,8 @@ TEST_F(EngineTest, SkpWarmup) {
   PersistentCache::GetCacheForProcess()->SetAssetManager(asset_manager);
 
   WarmupSkps();
+  concurrent_task_runner_.Run();
+  raster_task_runner_.Run();
 
   EXPECT_EQ(concurrent_task_runner_.GetTaskCount(), 1);
   EXPECT_EQ(raster_task_runner_.GetTaskCount(), 1);