Don't pause Animator when lifecycle state paused (#30969)

ColdPaleLight · web-flow · commit 46bea73d2e04 · 2022-01-27T15:55:07.000-08:00
diff --git a/shell/common/animator.cc b/shell/common/animator.cc
@@ -45,25 +45,6 @@ Animator::Animator(Delegate& delegate,
 
 Animator::~Animator() = default;
 
-void Animator::Stop() {
-  paused_ = true;
-}
-
-void Animator::Start() {
-  if (!paused_) {
-    return;
-  }
-
-  paused_ = false;
-  RequestFrame();
-}
-
-// Indicate that screen dimensions will be changing in order to force rendering
-// of an updated frame even if the animator is currently paused.
-void Animator::SetDimensionChangePending() {
-  dimension_change_pending_ = true;
-}
-
 void Animator::EnqueueTraceFlowId(uint64_t trace_flow_id) {
   fml::TaskRunner::RunNowOrPostTask(
       task_runners_.GetUITaskRunner(),
@@ -179,10 +160,6 @@ void Animator::BeginFrame(
 
 void Animator::Render(std::unique_ptr<flutter::LayerTree> layer_tree) {
   has_rendered_ = true;
-  if (dimension_change_pending_ &&
-      layer_tree->frame_size() != last_layer_tree_size_) {
-    dimension_change_pending_ = false;
-  }
   last_layer_tree_size_ = layer_tree->frame_size();
 
   if (!frame_timings_recorder_) {
@@ -232,9 +209,6 @@ void Animator::RequestFrame(bool regenerate_layer_tree) {
   if (regenerate_layer_tree) {
     regenerate_layer_tree_ = true;
   }
-  if (paused_ && !dimension_change_pending_) {
-    return;
-  }
 
   if (!pending_frame_semaphore_.TryWait()) {
     // Multiple calls to Animator::RequestFrame will still result in a
diff --git a/shell/common/animator.h b/shell/common/animator.h
@@ -74,12 +74,6 @@ class Animator final {
   void ScheduleSecondaryVsyncCallback(uintptr_t id,
                                       const fml::closure& callback);
 
-  void Start();
-
-  void Stop();
-
-  void SetDimensionChangePending();
-
   // Enqueue |trace_flow_id| into |trace_flow_ids_|.  The flow event will be
   // ended at either the next frame, or the next vsync interval with no active
   // active rendering.
@@ -112,11 +106,9 @@ class Animator final {
   std::shared_ptr<LayerTreePipeline> layer_tree_pipeline_;
   fml::Semaphore pending_frame_semaphore_;
   LayerTreePipeline::ProducerContinuation producer_continuation_;
-  bool paused_ = true;
   bool regenerate_layer_tree_ = false;
   bool frame_scheduled_ = false;
   int notify_idle_task_id_ = 0;
-  bool dimension_change_pending_ = false;
   SkISize last_layer_tree_size_ = {0, 0};
   std::deque<uint64_t> trace_flow_ids_;
   bool has_rendered_ = false;
diff --git a/shell/common/animator_unittests.cc b/shell/common/animator_unittests.cc
@@ -92,9 +92,6 @@ TEST_F(ShellTest, VSyncTargetTime) {
   fml::TaskRunner::RunNowOrPostTask(shell->GetTaskRunners().GetUITaskRunner(),
                                     [engine = shell->GetEngine()]() {
                                       if (engine) {
-                                        // Engine needs a surface for frames to
-                                        // be scheduled.
-                                        engine->OnOutputSurfaceCreated();
                                         // this implies we can re-use the last
                                         // frame to trigger begin frame rather
                                         // than re-generating the layer tree.
@@ -116,30 +113,6 @@ TEST_F(ShellTest, VSyncTargetTime) {
   ASSERT_FALSE(DartVMRef::IsInstanceRunning());
 }
 
-TEST_F(ShellTest, AnimatorStartsPaused) {
-  // Create all te prerequisites for a shell.
-  ASSERT_FALSE(DartVMRef::IsInstanceRunning());
-  auto settings = CreateSettingsForFixture();
-  TaskRunners task_runners = GetTaskRunnersForFixture();
-
-  auto shell = CreateShell(std::move(settings), task_runners,
-                           /* simulate_vsync=*/true);
-  ASSERT_TRUE(DartVMRef::IsInstanceRunning());
-
-  auto configuration = RunConfiguration::InferFromSettings(settings);
-  ASSERT_TRUE(configuration.IsValid());
-
-  configuration.SetEntrypoint("emptyMain");
-
-  RunEngine(shell.get(), std::move(configuration));
-
-  ASSERT_FALSE(IsAnimatorRunning(shell.get()));
-
-  // teardown.
-  DestroyShell(std::move(shell), std::move(task_runners));
-  ASSERT_FALSE(DartVMRef::IsInstanceRunning());
-}
-
 TEST_F(ShellTest, AnimatorDoesNotNotifyIdleBeforeRender) {
   FakeAnimatorDelegate delegate;
   TaskRunners task_runners = {
@@ -176,7 +149,6 @@ TEST_F(ShellTest, AnimatorDoesNotNotifyIdleBeforeRender) {
   // Validate it has not notified idle and start it. This will request a frame.
   task_runners.GetUITaskRunner()->PostTask([&] {
     ASSERT_FALSE(delegate.notify_idle_called_);
-    animator->Start();
     // Immediately request a frame saying it can reuse the last layer tree to
     // avoid more calls to BeginFrame by the animator.
     animator->RequestFrame(false);
@@ -211,8 +183,6 @@ TEST_F(ShellTest, AnimatorDoesNotNotifyIdleBeforeRender) {
   // Now it should notify idle. Make sure it is destroyed on the UI thread.
   ASSERT_TRUE(delegate.notify_idle_called_);
 
-  // Stop and do one more flush so we can safely clean up on the UI thread.
-  animator->Stop();
   task_runners.GetPlatformTaskRunner()->PostTask(flush_vsync_task);
   latch.Wait();
 
diff --git a/shell/common/engine.cc b/shell/common/engine.cc
@@ -50,8 +50,6 @@ Engine::Engine(
       settings_(std::move(settings)),
       animator_(std::move(animator)),
       runtime_controller_(std::move(runtime_controller)),
-      activity_running_(true),
-      have_surface_(false),
       font_collection_(font_collection),
       image_decoder_(task_runners, image_decoder_task_runner, io_manager),
       task_runners_(std::move(task_runners)),
@@ -280,30 +278,11 @@ tonic::DartErrorHandleType Engine::GetUIIsolateLastError() {
   return runtime_controller_->GetLastError();
 }
 
-void Engine::OnOutputSurfaceCreated() {
-  have_surface_ = true;
-  ScheduleFrame();
-}
-
-void Engine::OnOutputSurfaceDestroyed() {
-  have_surface_ = false;
-  StopAnimator();
-}
-
 void Engine::SetViewportMetrics(const ViewportMetrics& metrics) {
-  bool dimensions_changed =
-      viewport_metrics_.physical_height != metrics.physical_height ||
-      viewport_metrics_.physical_width != metrics.physical_width ||
-      viewport_metrics_.device_pixel_ratio != metrics.device_pixel_ratio;
   viewport_metrics_ = metrics;
   runtime_controller_->SetViewportMetrics(viewport_metrics_);
   if (animator_) {
-    if (dimensions_changed) {
-      animator_->SetDimensionChangePending();
-    }
-    if (have_surface_) {
-      ScheduleFrame();
-    }
+    ScheduleFrame();
   }
 }
 
@@ -339,20 +318,12 @@ bool Engine::HandleLifecyclePlatformMessage(PlatformMessage* message) {
   const auto& data = message->data();
   std::string state(reinterpret_cast<const char*>(data.GetMapping()),
                     data.GetSize());
-  if (state == "AppLifecycleState.paused" ||
-      state == "AppLifecycleState.detached") {
-    activity_running_ = false;
-    StopAnimator();
-  } else if (state == "AppLifecycleState.resumed" ||
-             state == "AppLifecycleState.inactive") {
-    activity_running_ = true;
-    StartAnimatorIfPossible();
-  }
 
   // Always schedule a frame when the app does become active as per API
   // recommendation
   // https://developer.apple.com/documentation/uikit/uiapplicationdelegate/1622956-applicationdidbecomeactive?language=objc
-  if (state == "AppLifecycleState.resumed" && have_surface_) {
+  if (state == "AppLifecycleState.resumed" ||
+      state == "AppLifecycleState.inactive") {
     ScheduleFrame();
   }
   runtime_controller_->SetLifecycleState(state);
@@ -455,16 +426,6 @@ void Engine::SetAccessibilityFeatures(int32_t flags) {
   runtime_controller_->SetAccessibilityFeatures(flags);
 }
 
-void Engine::StopAnimator() {
-  animator_->Stop();
-}
-
-void Engine::StartAnimatorIfPossible() {
-  if (activity_running_ && have_surface_) {
-    animator_->Start();
-  }
-}
-
 std::string Engine::DefaultRouteName() {
   if (!initial_route_.empty()) {
     return initial_route_;
@@ -473,7 +434,6 @@ std::string Engine::DefaultRouteName() {
 }
 
 void Engine::ScheduleFrame(bool regenerate_layer_tree) {
-  StartAnimatorIfPossible();
   animator_->RequestFrame(regenerate_layer_tree);
 }
 
diff --git a/shell/common/engine.h b/shell/common/engine.h
@@ -658,36 +658,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
   ///
   std::optional<uint32_t> GetUIIsolateReturnCode();
 
-  //----------------------------------------------------------------------------
-  /// @brief      Indicates to the Flutter application that it has obtained a
-  ///             rendering surface. This is a good opportunity for the engine
-  ///             to start servicing any outstanding frame requests from the
-  ///             Flutter applications. Flutter application that have no
-  ///             rendering concerns may never get a rendering surface. In such
-  ///             cases, while their root isolate can perform as normal, any
-  ///             frame requests made by them will never be serviced and layer
-  ///             trees produced outside of frame workloads will be dropped.
-  ///
-  ///             Very close to when this call is made, the application can
-  ///             expect the updated viewport metrics. Rendering only begins
-  ///             when the Flutter application gets an output surface and a
-  ///             valid set of viewport metrics.
-  ///
-  /// @see        `OnOutputSurfaceDestroyed`
-  ///
-  void OnOutputSurfaceCreated();
-
-  //----------------------------------------------------------------------------
-  /// @brief      Indicates to the Flutter application that a previously
-  ///             acquired rendering surface has been lost. Further frame
-  ///             requests will no longer be serviced and any layer tree
-  ///             submitted for rendering will be dropped. If/when a new surface
-  ///             is acquired, a new layer tree must be generated.
-  ///
-  /// @see        `OnOutputSurfaceCreated`
-  ///
-  void OnOutputSurfaceDestroyed();
-
   //----------------------------------------------------------------------------
   /// @brief      Updates the viewport metrics for the currently running Flutter
   ///             application. The viewport metrics detail the size of the
@@ -932,10 +902,6 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
 
   void SetNeedsReportTimings(bool value) override;
 
-  void StopAnimator();
-
-  void StartAnimatorIfPossible();
-
   bool HandleLifecyclePlatformMessage(PlatformMessage* message);
 
   bool HandleNavigationPlatformMessage(
diff --git a/shell/common/shell.cc b/shell/common/shell.cc
@@ -762,12 +762,9 @@ void Shell::OnPlatformViewCreated(std::unique_ptr<Surface> surface) {
         waiting_for_first_frame.store(true);
       });
 
-  // TODO(91717): This probably isn't necessary. The engine should be able to
-  // handle things here via normal lifecycle messages.
-  // https://github.com/flutter/flutter/issues/91717
   auto ui_task = [engine = engine_->GetWeakPtr()] {
     if (engine) {
-      engine->OnOutputSurfaceCreated();
+      engine->ScheduleFrame();
     }
   };
 
@@ -863,24 +860,11 @@ void Shell::OnPlatformViewDestroyed() {
       rasterizer->EnableThreadMergerIfNeeded();
       rasterizer->Teardown();
     }
-    // Step 3: Tell the IO thread to complete its remaining work.
+    // Step 2: Tell the IO thread to complete its remaining work.
     fml::TaskRunner::RunNowOrPostTask(io_task_runner, io_task);
   };
 
-  // TODO(91717): This probably isn't necessary. The engine should be able to
-  // handle things here via normal lifecycle messages.
-  // https://github.com/flutter/flutter/issues/91717
-  auto ui_task = [engine = engine_->GetWeakPtr()]() {
-    if (engine) {
-      engine->OnOutputSurfaceDestroyed();
-    }
-  };
-
-  // Step 1: 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);
-
-  // Step 2: Post a task to the Raster thread (possibly this thread) to tell the
+  // Step 1: Post a task to the Raster thread (possibly this thread) to tell the
   // rasterizer the output surface is going away.
   fml::TaskRunner::RunNowOrPostTask(task_runners_.GetRasterTaskRunner(),
                                     raster_task);
diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc
@@ -385,23 +385,6 @@ void ShellTest::DestroyShell(std::unique_ptr<Shell> shell,
   latch.Wait();
 }
 
-bool ShellTest::IsAnimatorRunning(Shell* shell) {
-  fml::AutoResetWaitableEvent latch;
-  bool running = false;
-  if (!shell) {
-    return running;
-  }
-  fml::TaskRunner::RunNowOrPostTask(
-      shell->GetTaskRunners().GetUITaskRunner(), [shell, &running, &latch]() {
-        if (shell && shell->engine_ && shell->engine_->animator_) {
-          running = !shell->engine_->animator_->paused_;
-        }
-        latch.Signal();
-      });
-  latch.Wait();
-  return running;
-}
-
 size_t ShellTest::GetLiveTrackedPathCount(
     std::shared_ptr<VolatilePathTracker> tracker) {
   return std::count_if(
