[Windows] Add/remove view failures should not hang (#52164)

If the embedder API's `FlutterEngineAddView` and `FlutterEngineRemoveView` don't return `kSuccess`, their callbacks won't be invoked. See: [flutter.dev/go/multi-view-embedder-apis](https://flutter.dev/go/multi-view-embedder-apis).

Previously, the Windows embedder would hang in this scenario as it blocks until the callbacks are invoked. Now, the Windows embedder only blocks if these embedder APIs return `kSuccess`.

Kudos to @dkwingsmt for catching this!

Part of flutter/flutter#144810
Part of flutter/flutter#142845

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

Author: loic-sharma

shell/platform/windows/flutter_windows_engine.cc

@@ -532,7 +532,15 @@ std::unique_ptr<FlutterWindowsView> FlutterWindowsEngine::CreateView(
       captures->latch.Signal();
     };
 
-    embedder_api_.AddView(engine_, &info);
+    FlutterEngineResult result = embedder_api_.AddView(engine_, &info);
+    if (result != kSuccess) {
+      FML_LOG(ERROR)
+          << "Starting the add view operation failed. FlutterEngineAddView "
+             "returned an unexpected result: "
+          << result << ". This indicates a bug in the Windows embedder.";
+      FML_DCHECK(false);
+      return nullptr;
+    }
 
     // Block the platform thread until the engine has added the view.
     // TODO(loicsharma): This blocks the platform thread eagerly and can
@@ -573,15 +581,24 @@ void FlutterWindowsEngine::RemoveView(FlutterViewId view_id) {
     info.view_id = view_id;
     info.user_data = &captures;
     info.remove_view_callback = [](const FlutterRemoveViewResult* result) {
-      // This is invoked on the raster thread, the same thread that the present
-      // callback is invoked. If |FlutterRemoveViewResult.removed| is `true`,
-      // the engine guarantees the view won't be presented.
+      // This is invoked on an engine thread. If
+      // |FlutterRemoveViewResult.removed| is `true`, the engine guarantees the
+      // view won't be presented.
       Captures* captures = reinterpret_cast<Captures*>(result->user_data);
       captures->removed = result->removed;
       captures->latch.Signal();
     };
 
-    embedder_api_.RemoveView(engine_, &info);
+    FlutterEngineResult result = embedder_api_.RemoveView(engine_, &info);
+    if (result != kSuccess) {
+      FML_LOG(ERROR) << "Starting the remove view operation failed. "
+                        "FlutterEngineRemoveView "
+                        "returned an unexpected result: "
+                     << result
+                     << ". This indicates a bug in the Windows embedder.";
+      FML_DCHECK(false);
+      return;
+    }
 
     // Block the platform thread until the engine has removed the view.
     // TODO(loicsharma): This blocks the platform thread eagerly and can

shell/platform/windows/flutter_windows_engine_unittests.cc

@@ -1265,5 +1265,53 @@ TEST_F(FlutterWindowsEngineTest, ReceivePlatformViewMessage) {
   }
 }
 
+TEST_F(FlutterWindowsEngineTest, AddViewFailureDoesNotHang) {
+  FlutterWindowsEngineBuilder builder{GetContext()};
+  auto engine = builder.Build();
+
+  EngineModifier modifier{engine.get()};
+
+  modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };
+  modifier.embedder_api().AddView = MOCK_ENGINE_PROC(
+      AddView,
+      [](FLUTTER_API_SYMBOL(FlutterEngine) engine,
+         const FlutterAddViewInfo* info) { return kInternalInconsistency; });
+
+  ASSERT_TRUE(engine->Run());
+
+  // Create the first view. This is the implicit view and isn't added to the
+  // engine.
+  auto implicit_window = std::make_unique<NiceMock<MockWindowBindingHandler>>();
+
+  std::unique_ptr<FlutterWindowsView> implicit_view =
+      engine->CreateView(std::move(implicit_window));
+
+  EXPECT_TRUE(implicit_view);
+
+  // Create a second view. The embedder attempts to add it to the engine.
+  auto second_window = std::make_unique<NiceMock<MockWindowBindingHandler>>();
+
+  EXPECT_DEBUG_DEATH(engine->CreateView(std::move(second_window)),
+                     "FlutterEngineAddView returned an unexpected result");
+}
+
+TEST_F(FlutterWindowsEngineTest, RemoveViewFailureDoesNotHang) {
+  FlutterWindowsEngineBuilder builder{GetContext()};
+  builder.SetDartEntrypoint("sendCreatePlatformViewMethod");
+  auto engine = builder.Build();
+
+  EngineModifier modifier{engine.get()};
+
+  modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };
+  modifier.embedder_api().RemoveView = MOCK_ENGINE_PROC(
+      RemoveView,
+      [](FLUTTER_API_SYMBOL(FlutterEngine) engine,
+         const FlutterRemoveViewInfo* info) { return kInternalInconsistency; });
+
+  ASSERT_TRUE(engine->Run());
+  EXPECT_DEBUG_DEATH(engine->RemoveView(123),
+                     "FlutterEngineRemoveView returned an unexpected result");
+}
+
 }  // namespace testing
 }  // namespace flutter