[Windows] Don't always stop engine on view destruction (flutter/engine#51681)

Currently destroying a view also shuts down the engine. This makes sense as in single-view world where the view also always owns the engine. However, in a multi-view world the views will share the engine. Destroying one view shouldn't necessarily shut down the engine unless that view owns the engine.

Part of https://github.com/flutter/flutter/issues/142845

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This commit is contained in:
Loïc Sharma
2024-03-28 17:14:10 -07:00
committed by GitHub
parent 115ff71db2
commit 675df6539b
10 changed files with 80 additions and 13 deletions

View File

@@ -41822,6 +41822,7 @@ ORIGIN: ../../../flutter/shell/platform/windows/flutter_windows_texture_registra
ORIGIN: ../../../flutter/shell/platform/windows/flutter_windows_texture_registrar.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/flutter_windows_view.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/flutter_windows_view.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/flutter_windows_view_controller.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/flutter_windows_view_controller.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/keyboard_handler_base.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/keyboard_key_channel_handler.cc + ../../../flutter/LICENSE
@@ -44730,6 +44731,7 @@ FILE: ../../../flutter/shell/platform/windows/flutter_windows_texture_registrar.
FILE: ../../../flutter/shell/platform/windows/flutter_windows_texture_registrar.h
FILE: ../../../flutter/shell/platform/windows/flutter_windows_view.cc
FILE: ../../../flutter/shell/platform/windows/flutter_windows_view.h
FILE: ../../../flutter/shell/platform/windows/flutter_windows_view_controller.cc
FILE: ../../../flutter/shell/platform/windows/flutter_windows_view_controller.h
FILE: ../../../flutter/shell/platform/windows/keyboard_handler_base.h
FILE: ../../../flutter/shell/platform/windows/keyboard_key_channel_handler.cc

View File

@@ -87,6 +87,7 @@ source_set("flutter_windows_source") {
"flutter_windows_texture_registrar.h",
"flutter_windows_view.cc",
"flutter_windows_view.h",
"flutter_windows_view_controller.cc",
"flutter_windows_view_controller.h",
"keyboard_handler_base.h",
"keyboard_key_channel_handler.cc",

View File

@@ -130,6 +130,7 @@ FlutterDesktopViewControllerRef FlutterDesktopEngineCreateViewController(
void FlutterDesktopViewControllerDestroy(FlutterDesktopViewControllerRef ref) {
auto controller = ViewControllerFromHandle(ref);
controller->Destroy();
delete controller;
}

View File

@@ -502,6 +502,24 @@ std::unique_ptr<FlutterWindowsView> FlutterWindowsEngine::CreateView(
return std::move(view);
}
void FlutterWindowsEngine::RemoveView(FlutterViewId view_id) {
FML_DCHECK(running());
FML_DCHECK(views_.find(view_id) != views_.end());
if (view_id == kImplicitViewId) {
// The engine and framework assume the implicit view always exists.
// Attempts to render to the implicit view will be ignored.
views_.erase(view_id);
return;
}
// TODO(loicsharma): Remove the view from the engine using the
// `FlutterEngineRemoveView` embedder API. Windows does not
// support views other than the implicit view yet.
// https://github.com/flutter/flutter/issues/144810
FML_UNREACHABLE();
}
void FlutterWindowsEngine::OnVsync(intptr_t baton) {
std::chrono::nanoseconds current_time =
std::chrono::nanoseconds(embedder_api_.GetCurrentTime());

View File

@@ -124,6 +124,9 @@ class FlutterWindowsEngine {
std::unique_ptr<FlutterWindowsView> CreateView(
std::unique_ptr<WindowBindingHandler> window);
// Remove a view. The engine will no longer render into it.
virtual void RemoveView(FlutterViewId view_id);
// Get a view that displays this engine's content.
//
// Returns null if the view does not exist.

View File

@@ -139,6 +139,9 @@ TEST_F(WindowsTest, EngineCanTransitionToHeadless) {
// The engine is back in headless mode now.
ASSERT_NE(engine, nullptr);
auto engine_ptr = reinterpret_cast<FlutterWindowsEngine*>(engine.get());
ASSERT_TRUE(engine_ptr->running());
}
// Verify that accessibility features are initialized when a view is created.

View File

@@ -105,10 +105,6 @@ FlutterWindowsView::~FlutterWindowsView() {
// Notify the engine the view's child window will no longer be visible.
engine_->OnWindowStateEvent(GetWindowHandle(), WindowStateEvent::kHide);
// The engine renders into the view's surface. The engine must be
// shutdown before the view's resources can be destroyed.
engine_->Stop();
DestroyRenderSurface();
}

View File

@@ -0,0 +1,30 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "flutter/shell/platform/windows/flutter_windows_view_controller.h"
namespace flutter {
FlutterWindowsViewController::~FlutterWindowsViewController() {
Destroy();
}
void FlutterWindowsViewController::Destroy() {
if (!view_) {
return;
}
// Prevent the engine from rendering into this view.
if (view_->GetEngine()->running()) {
auto view_id = view_->view_id();
view_->GetEngine()->RemoveView(view_id);
}
// Destroy the view, followed by the engine if it is owned by this controller.
view_.reset();
engine_.reset();
}
} // namespace flutter

View File

@@ -7,8 +7,9 @@
#include <memory>
#include "flutter_windows_engine.h"
#include "flutter_windows_view.h"
#include "flutter/fml/macros.h"
#include "flutter/shell/platform/windows/flutter_windows_engine.h"
#include "flutter/shell/platform/windows/flutter_windows_view.h"
namespace flutter {
@@ -19,6 +20,13 @@ class FlutterWindowsViewController {
std::unique_ptr<FlutterWindowsView> view)
: engine_(std::move(engine)), view_(std::move(view)) {}
~FlutterWindowsViewController();
// Destroy this view controller and its view.
//
// If this view controller owns the engine, the engine is also destroyed.
void Destroy();
FlutterWindowsEngine* engine() { return view_->GetEngine(); }
FlutterWindowsView* view() { return view_.get(); }
@@ -36,6 +44,8 @@ class FlutterWindowsViewController {
std::unique_ptr<FlutterWindowsEngine> engine_;
std::unique_ptr<FlutterWindowsView> view_;
FML_DISALLOW_COPY_AND_ASSIGN(FlutterWindowsViewController);
};
} // namespace flutter

View File

@@ -18,6 +18,7 @@
#include "flutter/shell/platform/windows/flutter_window.h"
#include "flutter/shell/platform/windows/flutter_windows_engine.h"
#include "flutter/shell/platform/windows/flutter_windows_texture_registrar.h"
#include "flutter/shell/platform/windows/flutter_windows_view_controller.h"
#include "flutter/shell/platform/windows/testing/egl/mock_context.h"
#include "flutter/shell/platform/windows/testing/egl/mock_manager.h"
#include "flutter/shell/platform/windows/testing/egl/mock_window_surface.h"
@@ -121,6 +122,7 @@ class MockFlutterWindowsEngine : public FlutterWindowsEngine {
MOCK_METHOD(bool, running, (), (const));
MOCK_METHOD(bool, Stop, (), ());
MOCK_METHOD(void, RemoveView, (FlutterViewId view_id), ());
MOCK_METHOD(bool, PostRasterThreadTask, (fml::closure), (const));
private:
@@ -241,6 +243,8 @@ TEST(FlutterWindowsViewTest, Shutdown) {
std::make_unique<NiceMock<MockWindowBindingHandler>>();
auto egl_manager = std::make_unique<egl::MockManager>();
auto surface = std::make_unique<egl::MockWindowSurface>();
auto engine_ptr = engine.get();
auto surface_ptr = surface.get();
EngineModifier modifier{engine.get()};
@@ -250,12 +254,16 @@ TEST(FlutterWindowsViewTest, Shutdown) {
std::unique_ptr<FlutterWindowsView> view =
engine->CreateView(std::move(window_binding_handler));
auto view_id = view->view_id();
ViewModifier view_modifier{view.get()};
view_modifier.SetSurface(std::move(surface));
// The engine must be stopped before the surface can be destroyed.
FlutterWindowsViewController controller{std::move(engine), std::move(view)};
// The view must be removed before the surface can be destroyed.
InSequence s;
EXPECT_CALL(*engine.get(), Stop).Times(1);
EXPECT_CALL(*engine_ptr, running).WillOnce(Return(true));
EXPECT_CALL(*engine_ptr, RemoveView(view_id)).Times(1);
EXPECT_CALL(*surface_ptr, Destroy).Times(1);
}
}
@@ -1392,7 +1400,6 @@ TEST(FlutterWindowsViewTest, DisablesVSyncAtStartup) {
EXPECT_CALL(*surface_ptr, SetVSyncEnabled(false)).WillOnce(Return(true));
EXPECT_CALL(render_context, ClearCurrent).WillOnce(Return(true));
EXPECT_CALL(*engine.get(), Stop).Times(1);
EXPECT_CALL(*surface_ptr, Destroy).Times(1);
EngineModifier modifier{engine.get()};
@@ -1430,7 +1437,6 @@ TEST(FlutterWindowsViewTest, EnablesVSyncAtStartup) {
EXPECT_CALL(*surface_ptr, SetVSyncEnabled(true)).WillOnce(Return(true));
EXPECT_CALL(render_context, ClearCurrent).WillOnce(Return(true));
EXPECT_CALL(*engine.get(), Stop).Times(1);
EXPECT_CALL(*surface_ptr, Destroy).Times(1);
EngineModifier modifier{engine.get()};
@@ -1471,7 +1477,6 @@ TEST(FlutterWindowsViewTest, DisablesVSyncAfterStartup) {
EXPECT_CALL(*surface_ptr, MakeCurrent).WillOnce(Return(true));
EXPECT_CALL(*surface_ptr, SetVSyncEnabled(false)).WillOnce(Return(true));
EXPECT_CALL(render_context, ClearCurrent).WillOnce(Return(true));
EXPECT_CALL(*engine.get(), Stop).Times(1);
EXPECT_CALL(*surface_ptr, Destroy).Times(1);
EngineModifier modifier{engine.get()};
@@ -1514,7 +1519,6 @@ TEST(FlutterWindowsViewTest, EnablesVSyncAfterStartup) {
EXPECT_CALL(*surface_ptr, MakeCurrent).WillOnce(Return(true));
EXPECT_CALL(*surface_ptr, SetVSyncEnabled(true)).WillOnce(Return(true));
EXPECT_CALL(render_context, ClearCurrent).WillOnce(Return(true));
EXPECT_CALL(*engine.get(), Stop).Times(1);
EXPECT_CALL(*surface_ptr, Destroy).Times(1);
EngineModifier modifier{engine.get()};
@@ -1563,7 +1567,6 @@ TEST(FlutterWindowsViewTest, UpdatesVSyncOnDwmUpdates) {
EXPECT_CALL(*surface_ptr, SetVSyncEnabled(false)).WillOnce(Return(true));
EXPECT_CALL(render_context, ClearCurrent).WillOnce(Return(true));
EXPECT_CALL(*engine.get(), Stop).Times(1);
EXPECT_CALL(*surface_ptr, Destroy).Times(1);
EngineModifier engine_modifier{engine.get()};