[Windows] Make lifecycle manager updates atomic (#164872)

Required for multi-window.

On windows the `LifecycleManager` currently sends the lifecycle event as
soon as windows message is processed. This however causes problems when
changing focus between application windows.

When switching focus from HWND1 to HWND2, HWND1 first gets unfocused,
followed by HWND2 getting focused. After HWND1 gets unfocused,
`LifecycleManager` immediately notifies the framework that the
application is inactive, which is wrong as the application never went
into inactive state, followed by subsequent call to put the application
in resumed state when HWND2 is focused. Because this happens very
quickly, sometimes focus manager gets into inconsistent state.

To resolve this `LifecycleManager` should gather the all the changes
while window sends the messages and then notify the framework atomically
in next run loop turn.

This PR also simplifies the logic in `LifecycleManager` through which
the application state is derived from window states.

This PR removes engine forcing `resumed` lifecycle state at startup. I'm
not entirely sure what the point of this was - the state can and should
be determined solely from window states, this just seems to muddy the
state logic. Also it happens before the framework is even listening to
state changes.

The mutex in `WindowsLifecycleManager` is removed. Not sure why it was
there.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x ] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

---------

Co-authored-by: Matthew Kosarek <matt.kosarek@canonical.com>
Co-authored-by: Harlen Batagelo <hbatagelo@gmail.com>
Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>

Author: 4 people

engine/src/flutter/shell/platform/windows/flutter_windows_engine.cc

@@ -489,7 +489,6 @@ bool FlutterWindowsEngine::Run(std::string_view entrypoint) {
                                     displays.data(), displays.size());
 
   SendSystemLocales();
-  SetLifecycleState(flutter::AppLifecycleState::kResumed);
 
   settings_plugin_->StartWatching();
   settings_plugin_->SendSettings();
@@ -802,12 +801,6 @@ void FlutterWindowsEngine::SetNextFrameCallback(fml::closure callback) {
       this);
 }
 
-void FlutterWindowsEngine::SetLifecycleState(flutter::AppLifecycleState state) {
-  if (lifecycle_manager_) {
-    lifecycle_manager_->SetLifecycleState(state);
-  }
-}
-
 void FlutterWindowsEngine::SendSystemLocales() {
   std::vector<LanguageInfo> languages =
       GetPreferredLanguageInfo(*windows_proc_table_);

engine/src/flutter/shell/platform/windows/flutter_windows_engine.h

@@ -347,9 +347,6 @@ class FlutterWindowsEngine {
   // system changes.
   void SendSystemLocales();
 
-  // Sends the current lifecycle state to the framework.
-  void SetLifecycleState(flutter::AppLifecycleState state);
-
   // Create the keyboard & text input sub-systems.
   //
   // This requires that a view is attached to the engine.

engine/src/flutter/shell/platform/windows/flutter_windows_engine_unittests.cc

@@ -800,7 +800,6 @@ TEST_F(FlutterWindowsEngineTest, TestExit) {
   modifier.SetImplicitView(&view);
   modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };
   auto handler = std::make_unique<MockWindowsLifecycleManager>(engine.get());
-  EXPECT_CALL(*handler, SetLifecycleState(AppLifecycleState::kResumed));
   EXPECT_CALL(*handler, Quit)
       .WillOnce([&finished](std::optional<HWND> hwnd,
                             std::optional<WPARAM> wparam,
@@ -837,7 +836,6 @@ TEST_F(FlutterWindowsEngineTest, TestExitCancel) {
   modifier.SetImplicitView(&view);
   modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };
   auto handler = std::make_unique<MockWindowsLifecycleManager>(engine.get());
-  EXPECT_CALL(*handler, SetLifecycleState(AppLifecycleState::kResumed));
   EXPECT_CALL(*handler, IsLastWindowOfProcess).WillRepeatedly(Return(true));
   EXPECT_CALL(*handler, Quit).Times(0);
   modifier.SetLifecycleManager(std::move(handler));
@@ -885,7 +883,6 @@ TEST_F(FlutterWindowsEngineTest, TestExitSecondCloseMessage) {
   modifier.SetImplicitView(&view);
   modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };
   auto handler = std::make_unique<MockWindowsLifecycleManager>(engine.get());
-  EXPECT_CALL(*handler, SetLifecycleState(AppLifecycleState::kResumed));
   EXPECT_CALL(*handler, IsLastWindowOfProcess).WillOnce(Return(true));
   EXPECT_CALL(*handler, Quit)
       .WillOnce([handler_ptr = handler.get()](
@@ -945,7 +942,6 @@ TEST_F(FlutterWindowsEngineTest, TestExitCloseMultiWindow) {
   modifier.SetImplicitView(&view);
   modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };
   auto handler = std::make_unique<MockWindowsLifecycleManager>(engine.get());
-  EXPECT_CALL(*handler, SetLifecycleState(AppLifecycleState::kResumed));
   EXPECT_CALL(*handler, IsLastWindowOfProcess).WillOnce([&finished]() {
     finished = true;
     return false;
@@ -1023,24 +1019,6 @@ TEST_F(FlutterWindowsEngineTest, ApplicationLifecycleExternalWindow) {
   engine->lifecycle_manager()->ExternalWindowMessage(0, WM_CLOSE, 0, 0);
 }
 
-TEST_F(FlutterWindowsEngineTest, AppStartsInResumedState) {
-  FlutterWindowsEngineBuilder builder{GetContext()};
-
-  auto engine = builder.Build();
-  auto window_binding_handler =
-      std::make_unique<::testing::NiceMock<MockWindowBindingHandler>>();
-  MockFlutterWindowsView view(engine.get(), std::move(window_binding_handler));
-
-  EngineModifier modifier(engine.get());
-  modifier.SetImplicitView(&view);
-  modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };
-  auto handler = std::make_unique<MockWindowsLifecycleManager>(engine.get());
-  EXPECT_CALL(*handler, SetLifecycleState(AppLifecycleState::kResumed))
-      .Times(1);
-  modifier.SetLifecycleManager(std::move(handler));
-  engine->Run();
-}
-
 TEST_F(FlutterWindowsEngineTest, LifecycleStateTransition) {
   FlutterWindowsEngineBuilder builder{GetContext()};
 
@@ -1056,16 +1034,41 @@ TEST_F(FlutterWindowsEngineTest, LifecycleStateTransition) {
 
   engine->window_proc_delegate_manager()->OnTopLevelWindowProc(
       (HWND)1, WM_SIZE, SIZE_RESTORED, 0);
+
+  while (engine->lifecycle_manager()->IsUpdateStateScheduled()) {
+    PumpMessage();
+  }
+
+  EXPECT_EQ(engine->lifecycle_manager()->GetLifecycleState(),
+            AppLifecycleState::kInactive);
+
+  engine->lifecycle_manager()->OnWindowStateEvent((HWND)1,
+                                                  WindowStateEvent::kFocus);
+
+  while (engine->lifecycle_manager()->IsUpdateStateScheduled()) {
+    PumpMessage();
+  }
+
   EXPECT_EQ(engine->lifecycle_manager()->GetLifecycleState(),
             AppLifecycleState::kResumed);
 
   engine->window_proc_delegate_manager()->OnTopLevelWindowProc(
       (HWND)1, WM_SIZE, SIZE_MINIMIZED, 0);
+
+  while (engine->lifecycle_manager()->IsUpdateStateScheduled()) {
+    PumpMessage();
+  }
+
   EXPECT_EQ(engine->lifecycle_manager()->GetLifecycleState(),
             AppLifecycleState::kHidden);
 
   engine->window_proc_delegate_manager()->OnTopLevelWindowProc(
       (HWND)1, WM_SIZE, SIZE_RESTORED, 0);
+
+  while (engine->lifecycle_manager()->IsUpdateStateScheduled()) {
+    PumpMessage();
+  }
+
   EXPECT_EQ(engine->lifecycle_manager()->GetLifecycleState(),
             AppLifecycleState::kInactive);
 }
@@ -1090,6 +1093,10 @@ TEST_F(FlutterWindowsEngineTest, ExternalWindowMessage) {
   engine->ProcessExternalWindowMessage(reinterpret_cast<HWND>(1), WM_SHOWWINDOW,
                                        FALSE, NULL);
 
+  while (engine->lifecycle_manager()->IsUpdateStateScheduled()) {
+    PumpMessage();
+  }
+
   EXPECT_EQ(engine->lifecycle_manager()->GetLifecycleState(),
             AppLifecycleState::kHidden);
 }
@@ -1117,12 +1124,20 @@ TEST_F(FlutterWindowsEngineTest, InnerWindowHidden) {
   view.OnWindowStateEvent(inner, WindowStateEvent::kShow);
   view.OnWindowStateEvent(inner, WindowStateEvent::kFocus);
 
+  while (engine->lifecycle_manager()->IsUpdateStateScheduled()) {
+    PumpMessage();
+  }
+
   EXPECT_EQ(engine->lifecycle_manager()->GetLifecycleState(),
             AppLifecycleState::kResumed);
 
   // Hide Flutter window, but not top level window.
   view.OnWindowStateEvent(inner, WindowStateEvent::kHide);
 
+  while (engine->lifecycle_manager()->IsUpdateStateScheduled()) {
+    PumpMessage();
+  }
+
   // The top-level window is still visible, so we ought not enter hidden state.
   EXPECT_EQ(engine->lifecycle_manager()->GetLifecycleState(),
             AppLifecycleState::kInactive);
@@ -1244,7 +1259,6 @@ TEST_F(FlutterWindowsEngineTest, ChannelListenedTo) {
 
   bool lifecycle_began = false;
   auto handler = std::make_unique<MockWindowsLifecycleManager>(engine.get());
-  EXPECT_CALL(*handler, SetLifecycleState).Times(1);
   handler->begin_processing_callback = [&]() { lifecycle_began = true; };
   modifier.SetLifecycleManager(std::move(handler));
 

engine/src/flutter/shell/platform/windows/flutter_windows_unittests.cc

@@ -535,7 +535,7 @@ TEST_F(WindowsTest, Lifecycle) {
   modifier.SetLifecycleManager(std::move(lifecycle_manager));
 
   EXPECT_CALL(*lifecycle_manager_ptr,
-              SetLifecycleState(AppLifecycleState::kResumed))
+              SetLifecycleState(AppLifecycleState::kInactive))
       .WillOnce([lifecycle_manager_ptr](AppLifecycleState state) {
         lifecycle_manager_ptr->WindowsLifecycleManager::SetLifecycleState(
             state);
@@ -548,10 +548,12 @@ TEST_F(WindowsTest, Lifecycle) {
             state);
       });
 
+  FlutterDesktopViewControllerProperties properties = {0, 0};
+
   // Create a controller. This launches the engine and sets the app lifecycle
   // to the "resumed" state.
   ViewControllerPtr controller{
-      FlutterDesktopViewControllerCreate(0, 0, engine.release())};
+      FlutterDesktopEngineCreateViewController(engine.get(), &properties)};
 
   FlutterDesktopViewRef view =
       FlutterDesktopViewControllerGetView(controller.get());
@@ -565,6 +567,17 @@ TEST_F(WindowsTest, Lifecycle) {
   // "hidden" app lifecycle event.
   ::MoveWindow(hwnd, /* X */ 0, /* Y */ 0, /* nWidth*/ 100, /* nHeight*/ 100,
                /* bRepaint*/ false);
+
+  while (lifecycle_manager_ptr->IsUpdateStateScheduled()) {
+    PumpMessage();
+  }
+
+  // Resets the view, simulating the window being hidden.
+  controller.reset();
+
+  while (lifecycle_manager_ptr->IsUpdateStateScheduled()) {
+    PumpMessage();
+  }
 }
 
 TEST_F(WindowsTest, GetKeyboardStateHeadless) {

engine/src/flutter/shell/platform/windows/windows_lifecycle_manager.cc

@@ -205,49 +205,48 @@ void WindowsLifecycleManager::SetLifecycleState(AppLifecycleState state) {
   }
 }
 
+void WindowsLifecycleManager::UpdateState() {
+  AppLifecycleState new_state = AppLifecycleState::kResumed;
+  if (visible_windows_.empty()) {
+    new_state = AppLifecycleState::kHidden;
+  } else if (focused_windows_.empty()) {
+    new_state = AppLifecycleState::kInactive;
+  }
+  SetLifecycleState(new_state);
+}
+
 void WindowsLifecycleManager::OnWindowStateEvent(HWND hwnd,
                                                  WindowStateEvent event) {
-  // Synthesize an unfocus event when a focused window is hidden.
-  if (event == WindowStateEvent::kHide &&
-      focused_windows_.find(hwnd) != focused_windows_.end()) {
-    OnWindowStateEvent(hwnd, WindowStateEvent::kUnfocus);
+  // Instead of updating the state immediately, remember all
+  // changes to individual window and then update the state in next run loop
+  // turn. Otherwise the application would be temporarily deactivated when
+  // switching focus between windows for example.
+  if (!update_state_scheduled_) {
+    update_state_scheduled_ = true;
+    // Task runner will be destroyed together with engine so it is safe
+    // to keep reference to it.
+    engine_->task_runner()->PostTask([this]() {
+      update_state_scheduled_ = false;
+      UpdateState();
+    });
   }
 
-  std::lock_guard guard(state_update_lock_);
   switch (event) {
     case WindowStateEvent::kShow: {
-      bool first_shown_window = visible_windows_.empty();
-      auto pair = visible_windows_.insert(hwnd);
-      if (first_shown_window && pair.second &&
-          state_ == AppLifecycleState::kHidden) {
-        SetLifecycleState(AppLifecycleState::kInactive);
-      }
+      visible_windows_.insert(hwnd);
       break;
     }
     case WindowStateEvent::kHide: {
-      bool present = visible_windows_.erase(hwnd);
-      bool empty = visible_windows_.empty();
-      if (present && empty &&
-          (state_ == AppLifecycleState::kResumed ||
-           state_ == AppLifecycleState::kInactive)) {
-        SetLifecycleState(AppLifecycleState::kHidden);
-      }
+      visible_windows_.erase(hwnd);
+      focused_windows_.erase(hwnd);
       break;
     }
     case WindowStateEvent::kFocus: {
-      bool first_focused_window = focused_windows_.empty();
-      auto pair = focused_windows_.insert(hwnd);
-      if (first_focused_window && pair.second &&
-          state_ == AppLifecycleState::kInactive) {
-        SetLifecycleState(AppLifecycleState::kResumed);
-      }
+      focused_windows_.insert(hwnd);
       break;
     }
     case WindowStateEvent::kUnfocus: {
-      if (focused_windows_.erase(hwnd) && focused_windows_.empty() &&
-          state_ == AppLifecycleState::kResumed) {
-        SetLifecycleState(AppLifecycleState::kInactive);
-      }
+      focused_windows_.erase(hwnd);
       break;
     }
   }

engine/src/flutter/shell/platform/windows/windows_lifecycle_manager.h

@@ -63,17 +63,16 @@ class WindowsLifecycleManager {
   // message to the framework notifying it of the state change.
   virtual void SetLifecycleState(AppLifecycleState state);
 
-  // Respond to a change in window state. Transitions as follows:
-  // When the only visible window is hidden, transition from resumed or
-  // inactive to hidden.
-  // When the only focused window is unfocused, transition from resumed to
-  // inactive.
-  // When a window is focused, transition from inactive to resumed.
-  // When a window is shown, transition from hidden to inactive.
+  // Respond to a change in window state.
+  // Saves the state for the HWND and schedules UpdateState to be called
+  // if it is not already scheduled.
   virtual void OnWindowStateEvent(HWND hwnd, WindowStateEvent event);
 
   AppLifecycleState GetLifecycleState() { return state_; }
 
+  // Used in tests to wait until the state is updated.
+  bool IsUpdateStateScheduled() const { return update_state_scheduled_; }
+
   // Called by the engine when a non-Flutter window receives an event that may
   // alter the lifecycle state. The logic for external windows must differ from
   // that used for FlutterWindow instances, because:
@@ -114,12 +113,20 @@ class WindowsLifecycleManager {
   bool process_exit_ = false;
 
   std::set<HWND> visible_windows_;
-
   std::set<HWND> focused_windows_;
 
-  std::mutex state_update_lock_;
+  // Transitions the application state. If any windows are focused,
+  // the application is considered resumed. If no windows are focused
+  // but there are visible windows, application is considered inactive.
+  // Otherwise, if there are no visible window, application is considered
+  // hidden.
+  void UpdateState();
+
+  // Whether update state is scheduled to be called in next run loop turn.
+  // This is needed to provide atomic updates of the state.
+  bool update_state_scheduled_ = false;
 
-  flutter::AppLifecycleState state_;
+  AppLifecycleState state_ = AppLifecycleState::kDetached;
 };
 
 }  // namespace flutter