Remove the dummy rasterizer delegate now that flutter_runner is in tree, and cleanup ctor params (flutter#20486)

dnfield · web-flow · commit 6d44559b7dbf · 2020-08-13T14:46:01.000-07:00
diff --git a/shell/common/animator_unittests.cc b/shell/common/animator_unittests.cc
@@ -58,11 +58,7 @@ TEST_F(ShellTest, VSyncTargetTime) {
               std::move(create_vsync_waiter),
               ShellTestPlatformView::BackendType::kDefaultBackend, nullptr);
         },
-        [](Shell& shell) {
-          return std::make_unique<Rasterizer>(
-              shell, shell.GetTaskRunners(),
-              shell.GetIsGpuDisabledSyncSwitch());
-        });
+        [](Shell& shell) { return std::make_unique<Rasterizer>(shell); });
     ASSERT_TRUE(DartVMRef::IsInstanceRunning());
 
     auto configuration = RunConfiguration::InferFromSettings(settings);
diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc
@@ -25,38 +25,18 @@ namespace flutter {
 // used within this interval.
 static constexpr std::chrono::milliseconds kSkiaCleanupExpiration(15000);
 
-// TODO(dnfield): Remove this once internal embedders have caught up.
-static Rasterizer::DummyDelegate dummy_delegate_;
-Rasterizer::Rasterizer(
-    TaskRunners task_runners,
-    std::unique_ptr<flutter::CompositorContext> compositor_context,
-    std::shared_ptr<fml::SyncSwitch> is_gpu_disabled_sync_switch)
-    : Rasterizer(dummy_delegate_,
-                 std::move(task_runners),
-                 std::move(compositor_context),
-                 is_gpu_disabled_sync_switch) {}
-
-Rasterizer::Rasterizer(
-    Delegate& delegate,
-    TaskRunners task_runners,
-    std::shared_ptr<fml::SyncSwitch> is_gpu_disabled_sync_switch)
+Rasterizer::Rasterizer(Delegate& delegate)
     : Rasterizer(delegate,
-                 std::move(task_runners),
                  std::make_unique<flutter::CompositorContext>(
-                     delegate.GetFrameBudget()),
-                 is_gpu_disabled_sync_switch) {}
+                     delegate.GetFrameBudget())) {}
 
 Rasterizer::Rasterizer(
     Delegate& delegate,
-    TaskRunners task_runners,
-    std::unique_ptr<flutter::CompositorContext> compositor_context,
-    std::shared_ptr<fml::SyncSwitch> is_gpu_disabled_sync_switch)
+    std::unique_ptr<flutter::CompositorContext> compositor_context)
     : delegate_(delegate),
-      task_runners_(std::move(task_runners)),
       compositor_context_(std::move(compositor_context)),
       user_override_resource_cache_bytes_(false),
-      weak_factory_(this),
-      is_gpu_disabled_sync_switch_(is_gpu_disabled_sync_switch) {
+      weak_factory_(this) {
   FML_DCHECK(compositor_context_);
 }
 
@@ -83,8 +63,9 @@ void Rasterizer::Setup(std::unique_ptr<Surface> surface) {
   // support for raster thread merger for Fuchsia.
   if (surface_->GetExternalViewEmbedder()) {
     const auto platform_id =
-        task_runners_.GetPlatformTaskRunner()->GetTaskQueueId();
-    const auto gpu_id = task_runners_.GetRasterTaskRunner()->GetTaskQueueId();
+        delegate_.GetTaskRunners().GetPlatformTaskRunner()->GetTaskQueueId();
+    const auto gpu_id =
+        delegate_.GetTaskRunners().GetRasterTaskRunner()->GetTaskQueueId();
     raster_thread_merger_ =
         fml::MakeRefCounted<fml::RasterThreadMerger>(platform_id, gpu_id);
   }
@@ -134,7 +115,9 @@ void Rasterizer::Draw(fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline) {
     // we yield and let this frame be serviced on the right thread.
     return;
   }
-  FML_DCHECK(task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread());
+  FML_DCHECK(delegate_.GetTaskRunners()
+                 .GetRasterTaskRunner()
+                 ->RunsTasksOnCurrentThread());
 
   RasterStatus raster_status = RasterStatus::kFailed;
   Pipeline<flutter::LayerTree>::Consumer consumer =
@@ -168,7 +151,7 @@ void Rasterizer::Draw(fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline) {
   // between successive tries.
   switch (consume_result) {
     case PipelineConsumeResult::MoreAvailable: {
-      task_runners_.GetRasterTaskRunner()->PostTask(
+      delegate_.GetTaskRunners().GetRasterTaskRunner()->PostTask(
           [weak_this = weak_factory_.GetWeakPtr(), pipeline]() {
             if (weak_this) {
               weak_this->Draw(pipeline);
@@ -226,7 +209,7 @@ sk_sp<SkImage> Rasterizer::DoMakeRasterSnapshot(
     sk_sp<SkSurface> surface = SkSurface::MakeRaster(image_info);
     result = DrawSnapshot(surface, draw_callback);
   } else {
-    is_gpu_disabled_sync_switch_->Execute(
+    delegate_.GetIsGpuDisabledSyncSwitch()->Execute(
         fml::SyncSwitch::Handlers()
             .SetIfTrue([&] {
               sk_sp<SkSurface> surface = SkSurface::MakeRaster(image_info);
@@ -282,7 +265,9 @@ sk_sp<SkImage> Rasterizer::ConvertToRasterImage(sk_sp<SkImage> image) {
 
 RasterStatus Rasterizer::DoDraw(
     std::unique_ptr<flutter::LayerTree> layer_tree) {
-  FML_DCHECK(task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread());
+  FML_DCHECK(delegate_.GetTaskRunners()
+                 .GetRasterTaskRunner()
+                 ->RunsTasksOnCurrentThread());
 
   if (!layer_tree || !surface_) {
     return RasterStatus::kFailed;
diff --git a/shell/common/rasterizer.h b/shell/common/rasterizer.h
@@ -74,82 +74,41 @@ class Rasterizer final : public SnapshotDelegate {
     /// Target time for the latest frame. See also `Shell::OnAnimatorBeginFrame`
     /// for when this time gets updated.
     virtual fml::TimePoint GetLatestFrameTargetTime() const = 0;
-  };
 
-  // TODO(dnfield): remove once embedders have caught up.
-  class DummyDelegate : public Delegate {
-    void OnFrameRasterized(const FrameTiming&) override {}
-    fml::Milliseconds GetFrameBudget() override {
-      return fml::kDefaultFrameBudget;
-    }
-    // Returning a time in the past so we don't add additional trace
-    // events when exceeding the frame budget for other embedders.
-    fml::TimePoint GetLatestFrameTargetTime() const override {
-      return fml::TimePoint::FromEpochDelta(fml::TimeDelta::Zero());
-    }
-  };
+    /// Task runners used by the shell.
+    virtual const TaskRunners& GetTaskRunners() const = 0;
 
-  //----------------------------------------------------------------------------
-  /// @brief      Creates a new instance of a rasterizer. Rasterizers may only
-  ///             be created on the GPU task runner. Rasterizers are currently
-  ///             only created by the shell. Usually, the shell also sets itself
-  ///             up as the rasterizer delegate. But, this constructor sets up a
-  ///             dummy rasterizer delegate.
-  ///
-  //  TODO(chinmaygarde): The rasterizer does not use the task runners for
-  //  anything other than thread checks. Remove the same as an argument.
-  ///
-  /// @param[in]  task_runners        The task runners used by the shell.
-  /// @param[in]  compositor_context  The compositor context used to hold all
-  ///                                 the GPU state used by the rasterizer.
-  /// @param[in]  is_gpu_disabled_sync_switch
-  ///    A `SyncSwitch` for handling disabling of the GPU (typically happens
-  ///    when an app is backgrounded)
-  ///
-  Rasterizer(TaskRunners task_runners,
-             std::unique_ptr<flutter::CompositorContext> compositor_context,
-             std::shared_ptr<fml::SyncSwitch> is_gpu_disabled_sync_switch);
+    /// Accessor for the shell's GPU sync switch, which determines whether GPU
+    /// operations are allowed on the current thread.
+    ///
+    /// For example, on some platforms when the application is backgrounded it
+    /// is critical that GPU operations are not processed.
+    virtual std::shared_ptr<fml::SyncSwitch> GetIsGpuDisabledSyncSwitch()
+        const = 0;
+  };
 
   //----------------------------------------------------------------------------
   /// @brief      Creates a new instance of a rasterizer. Rasterizers may only
   ///             be created on the GPU task runner. Rasterizers are currently
   ///             only created by the shell (which also sets itself up as the
   ///             rasterizer delegate).
   ///
-  //  TODO(chinmaygarde): The rasterizer does not use the task runners for
-  //  anything other than thread checks. Remove the same as an argument.
-  ///
   /// @param[in]  delegate            The rasterizer delegate.
-  /// @param[in]  task_runners        The task runners used by the shell.
-  /// @param[in]  is_gpu_disabled_sync_switch
-  ///    A `SyncSwitch` for handling disabling of the GPU (typically happens
-  ///    when an app is backgrounded)
   ///
-  Rasterizer(Delegate& delegate,
-             TaskRunners task_runners,
-             std::shared_ptr<fml::SyncSwitch> is_gpu_disabled_sync_switch);
+  Rasterizer(Delegate& delegate);
 
   //----------------------------------------------------------------------------
   /// @brief      Creates a new instance of a rasterizer. Rasterizers may only
   ///             be created on the GPU task runner. Rasterizers are currently
   ///             only created by the shell (which also sets itself up as the
   ///             rasterizer delegate).
   ///
-  //  TODO(chinmaygarde): The rasterizer does not use the task runners for
-  //  anything other than thread checks. Remove the same as an argument.
-  ///
   /// @param[in]  delegate            The rasterizer delegate.
-  /// @param[in]  task_runners        The task runners used by the shell.
   /// @param[in]  compositor_context  The compositor context used to hold all
   ///                                 the GPU state used by the rasterizer.
-  /// @param[in]  is_gpu_disabled_sync_switch
-  ///    A `SyncSwitch` for handling disabling of the GPU (typically happens
-  ///    when an app is backgrounded)
   ///
   Rasterizer(Delegate& delegate,
-             TaskRunners task_runners,
-             std::unique_ptr<flutter::CompositorContext> compositor_context,
-             std::shared_ptr<fml::SyncSwitch> is_gpu_disabled_sync_switch);
+             std::unique_ptr<flutter::CompositorContext> compositor_context);
 
   //----------------------------------------------------------------------------
   /// @brief      Destroys the rasterizer. This must happen on the GPU task
@@ -432,7 +391,6 @@ class Rasterizer final : public SnapshotDelegate {
 
  private:
   Delegate& delegate_;
-  TaskRunners task_runners_;
   std::unique_ptr<Surface> surface_;
   std::unique_ptr<flutter::CompositorContext> compositor_context_;
   // This is the last successfully rasterized layer tree.
@@ -446,7 +404,6 @@ class Rasterizer final : public SnapshotDelegate {
   std::optional<size_t> max_cache_bytes_;
   fml::TaskRunnerAffineWeakPtrFactory<Rasterizer> weak_factory_;
   fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger_;
-  std::shared_ptr<fml::SyncSwitch> is_gpu_disabled_sync_switch_;
 
   // |SnapshotDelegate|
   sk_sp<SkImage> MakeRasterSnapshot(sk_sp<SkPicture> picture,
diff --git a/shell/common/shell.h b/shell/common/shell.h
@@ -248,7 +248,7 @@ class Shell final : public PlatformView::Delegate,
   ///
   /// @return     The task runners current in use by the shell.
   ///
-  const TaskRunners& GetTaskRunners() const;
+  const TaskRunners& GetTaskRunners() const override;
 
   //----------------------------------------------------------------------------
   /// @brief      Rasterizers may only be accessed on the GPU task runner.
@@ -352,7 +352,7 @@ class Shell final : public PlatformView::Delegate,
 
   //----------------------------------------------------------------------------
   /// @brief     Accessor for the disable GPU SyncSwitch
-  std::shared_ptr<fml::SyncSwitch> GetIsGpuDisabledSyncSwitch() const;
+  std::shared_ptr<fml::SyncSwitch> GetIsGpuDisabledSyncSwitch() const override;
 
   //----------------------------------------------------------------------------
   /// @brief      Get a pointer to the Dart VM used by this running shell
diff --git a/shell/common/shell_benchmarks.cc b/shell/common/shell_benchmarks.cc
@@ -58,11 +58,7 @@ static void StartupAndShutdownShell(benchmark::State& state,
         [](Shell& shell) {
           return std::make_unique<PlatformView>(shell, shell.GetTaskRunners());
         },
-        [](Shell& shell) {
-          return std::make_unique<Rasterizer>(
-              shell, shell.GetTaskRunners(),
-              shell.GetIsGpuDisabledSyncSwitch());
-        });
+        [](Shell& shell) { return std::make_unique<Rasterizer>(shell); });
   }
 
   FML_CHECK(shell);
diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc
@@ -268,10 +268,7 @@ std::unique_ptr<Shell> ShellTest::CreateShell(
             ShellTestPlatformView::BackendType::kDefaultBackend,
             shell_test_external_view_embedder);
       },
-      [](Shell& shell) {
-        return std::make_unique<Rasterizer>(shell, shell.GetTaskRunners(),
-                                            shell.GetIsGpuDisabledSyncSwitch());
-      });
+      [](Shell& shell) { return std::make_unique<Rasterizer>(shell); });
 }
 void ShellTest::DestroyShell(std::unique_ptr<Shell> shell) {
   DestroyShell(std::move(shell), GetTaskRunnersForFixture());
diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc
@@ -149,10 +149,7 @@ TEST_F(ShellTest,
             },
             ShellTestPlatformView::BackendType::kDefaultBackend, nullptr);
       },
-      [](Shell& shell) {
-        return std::make_unique<Rasterizer>(shell, shell.GetTaskRunners(),
-                                            shell.GetIsGpuDisabledSyncSwitch());
-      });
+      [](Shell& shell) { return std::make_unique<Rasterizer>(shell); });
   ASSERT_TRUE(ValidateShell(shell.get()));
   ASSERT_TRUE(DartVMRef::IsInstanceRunning());
   DestroyShell(std::move(shell), std::move(task_runners));
diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc
@@ -81,8 +81,7 @@ AndroidShellHolder::AndroidShellHolder(
       };
 
   Shell::CreateCallback<Rasterizer> on_create_rasterizer = [](Shell& shell) {
-    return std::make_unique<Rasterizer>(shell, shell.GetTaskRunners(),
-                                        shell.GetIsGpuDisabledSyncSwitch());
+    return std::make_unique<Rasterizer>(shell);
   };
 
   // The current thread will be used as the platform thread. Ensure that the
diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm
@@ -506,10 +506,7 @@ - (BOOL)createShell:(NSString*)entrypoint
       };
 
   flutter::Shell::CreateCallback<flutter::Rasterizer> on_create_rasterizer =
-      [](flutter::Shell& shell) {
-        return std::make_unique<flutter::Rasterizer>(shell, shell.GetTaskRunners(),
-                                                     shell.GetIsGpuDisabledSyncSwitch());
-      };
+      [](flutter::Shell& shell) { return std::make_unique<flutter::Rasterizer>(shell); };
 
   if (flutter::IsIosEmbeddedViewsPreviewEnabled()) {
     // Embedded views requires the gpu and the platform views to be the same.
diff --git a/shell/platform/embedder/embedder.cc b/shell/platform/embedder/embedder.cc
@@ -991,8 +991,7 @@ FlutterEngineResult FlutterEngineInitialize(size_t version,
 
   flutter::Shell::CreateCallback<flutter::Rasterizer> on_create_rasterizer =
       [](flutter::Shell& shell) {
-        return std::make_unique<flutter::Rasterizer>(
-            shell, shell.GetTaskRunners(), shell.GetIsGpuDisabledSyncSwitch());
+        return std::make_unique<flutter::Rasterizer>(shell);
       };
 
   // TODO(chinmaygarde): This is the wrong spot for this. It belongs in the
diff --git a/shell/platform/fuchsia/flutter/engine.cc b/shell/platform/fuchsia/flutter/engine.cc
@@ -230,9 +230,7 @@ Engine::Engine(Delegate& delegate,
         }
 
         return std::make_unique<flutter::Rasterizer>(
-            /*task_runners=*/shell.GetTaskRunners(),
-            /*compositor_context=*/std::move(compositor_context),
-            /*is_gpu_disabled_sync_switch=*/shell.GetIsGpuDisabledSyncSwitch());
+            shell, std::move(compositor_context));
       });
 
   UpdateNativeThreadLabelNames(thread_label_, task_runners);
diff --git a/shell/testing/tester_main.cc b/shell/testing/tester_main.cc
@@ -140,8 +140,7 @@ int RunTester(const flutter::Settings& settings,
       };
 
   Shell::CreateCallback<Rasterizer> on_create_rasterizer = [](Shell& shell) {
-    return std::make_unique<Rasterizer>(shell, shell.GetTaskRunners(),
-                                        shell.GetIsGpuDisabledSyncSwitch());
+    return std::make_unique<Rasterizer>(shell);
   };
 
   auto shell = Shell::Create(task_runners,             //
