Skip to content

Commit

Permalink
[Windows] Fix top-level message procedure order (#50797)
Browse files Browse the repository at this point in the history
The Windows embedder registers "message procedures" to handle to top-level window events. These message procedures should be called in the order that they are registered.

For example, a plugin can override the embedder's app lifecycle behavior by registering a message procedure before the embedder's app lifecycle message procedure.

This did not always work as expected as the message procedures were ordered by their pointers instead of their insertion order.

Fixes flutter/flutter#137963

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
  • Loading branch information
loic-sharma authored Feb 21, 2024
1 parent 7a58dac commit fa50a0a
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#include <lib/fdio/directory.h>
#include <lib/zx/process.h>

#include <algorithm> // Foor std::remove_if
#include <algorithm> // For std::remove_if
#include <memory>
#include <string>
#include <utility>
Expand Down
22 changes: 10 additions & 12 deletions shell/platform/windows/flutter_windows_engine_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -784,9 +784,9 @@ TEST_F(FlutterWindowsEngineTest, TestExitCancel) {
}
}

// TODO(loicsharma): This test is passing incorrectly on the first
// WM_CLOSE message when instead it should pass on the second WM_CLOSE message.
// https://github.com/flutter/flutter/issues/137963
// Flutter consumes the first WM_CLOSE message to allow the app to cancel the
// exit. If the app does not cancel the exit, Flutter synthesizes a second
// WM_CLOSE message.
TEST_F(FlutterWindowsEngineTest, TestExitSecondCloseMessage) {
FlutterWindowsEngineBuilder builder{GetContext()};
builder.SetDartEntrypoint("exitTestExit");
Expand All @@ -802,18 +802,16 @@ TEST_F(FlutterWindowsEngineTest, TestExitSecondCloseMessage) {
modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };
auto handler = std::make_unique<MockWindowsLifecycleManager>(engine.get());
EXPECT_CALL(*handler, SetLifecycleState(AppLifecycleState::kResumed));
// TODO(loicsharma): These should be `EXPECT_CALL`s
// https://github.com/flutter/flutter/issues/137963
ON_CALL(*handler, IsLastWindowOfProcess).WillByDefault(Return(true));
ON_CALL(*handler, Quit)
.WillByDefault([handler_ptr = handler.get()](
std::optional<HWND> hwnd, std::optional<WPARAM> wparam,
std::optional<LPARAM> lparam, UINT exit_code) {
EXPECT_CALL(*handler, IsLastWindowOfProcess).WillOnce(Return(true));
EXPECT_CALL(*handler, Quit)
.WillOnce([handler_ptr = handler.get()](
std::optional<HWND> hwnd, std::optional<WPARAM> wparam,
std::optional<LPARAM> lparam, UINT exit_code) {
handler_ptr->WindowsLifecycleManager::Quit(hwnd, wparam, lparam,
exit_code);
});
ON_CALL(*handler, DispatchMessage)
.WillByDefault(
EXPECT_CALL(*handler, DispatchMessage)
.WillRepeatedly(
[&engine](HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam) {
engine->window_proc_delegate_manager()->OnTopLevelWindowProc(
hwnd, msg, wparam, lparam);
Expand Down
27 changes: 20 additions & 7 deletions shell/platform/windows/window_proc_delegate_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#include "flutter/shell/platform/windows/window_proc_delegate_manager.h"

#include <algorithm>

#include "flutter/shell/platform/embedder/embedder.h"

namespace flutter {
Expand All @@ -12,26 +14,37 @@ WindowProcDelegateManager::WindowProcDelegateManager() = default;
WindowProcDelegateManager::~WindowProcDelegateManager() = default;

void WindowProcDelegateManager::RegisterTopLevelWindowProcDelegate(
FlutterDesktopWindowProcCallback delegate,
FlutterDesktopWindowProcCallback callback,
void* user_data) {
top_level_window_proc_handlers_[delegate] = user_data;
UnregisterTopLevelWindowProcDelegate(callback);

delegates_.push_back(WindowProcDelegate{
.callback = callback,
.user_data = user_data,
});
}

void WindowProcDelegateManager::UnregisterTopLevelWindowProcDelegate(
FlutterDesktopWindowProcCallback delegate) {
top_level_window_proc_handlers_.erase(delegate);
FlutterDesktopWindowProcCallback callback) {
delegates_.erase(
std::remove_if(delegates_.begin(), delegates_.end(),
[&callback](const WindowProcDelegate& delegate) {
return delegate.callback == callback;
}),
delegates_.end());
}

std::optional<LRESULT> WindowProcDelegateManager::OnTopLevelWindowProc(
HWND hwnd,
UINT message,
WPARAM wparam,
LPARAM lparam) {
LPARAM lparam) const {
std::optional<LRESULT> result;
for (const auto& [handler, user_data] : top_level_window_proc_handlers_) {
for (const auto& delegate : delegates_) {
LPARAM handler_result;
// Stop as soon as any delegate indicates that it has handled the message.
if (handler(hwnd, message, wparam, lparam, user_data, &handler_result)) {
if (delegate.callback(hwnd, message, wparam, lparam, delegate.user_data,
&handler_result)) {
result = handler_result;
break;
}
Expand Down
25 changes: 15 additions & 10 deletions shell/platform/windows/window_proc_delegate_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@

#include <Windows.h>

#include <map>
#include <optional>
#include <vector>

#include "flutter/fml/macros.h"
#include "flutter/shell/platform/windows/public/flutter_windows.h"
Expand All @@ -22,30 +22,35 @@ class WindowProcDelegateManager {
explicit WindowProcDelegateManager();
~WindowProcDelegateManager();

// Adds |delegate| as a delegate to be called for |OnTopLevelWindowProc|.
// Adds |callback| as a delegate to be called for |OnTopLevelWindowProc|.
//
// Multiple calls with the same |delegate| will replace the previous
// Multiple calls with the same |callback| will replace the previous
// registration, even if |user_data| is different.
void RegisterTopLevelWindowProcDelegate(
FlutterDesktopWindowProcCallback delegate,
FlutterDesktopWindowProcCallback callback,
void* user_data);

// Unregisters |delegate| as a delate for |OnTopLevelWindowProc|.
// Unregisters |callback| as a delegate for |OnTopLevelWindowProc|.
void UnregisterTopLevelWindowProcDelegate(
FlutterDesktopWindowProcCallback delegate);
FlutterDesktopWindowProcCallback callback);

// Calls any registered WindowProc delegates.
// Calls any registered WindowProc delegates in the order they were
// registered.
//
// If a result is returned, then the message was handled in such a way that
// further handling should not be done.
std::optional<LRESULT> OnTopLevelWindowProc(HWND hwnd,
UINT message,
WPARAM wparam,
LPARAM lparam);
LPARAM lparam) const;

private:
std::map<FlutterDesktopWindowProcCallback, void*>
top_level_window_proc_handlers_;
struct WindowProcDelegate {
FlutterDesktopWindowProcCallback callback = nullptr;
void* user_data = nullptr;
};

std::vector<WindowProcDelegate> delegates_;

FML_DISALLOW_COPY_AND_ASSIGN(WindowProcDelegateManager);
};
Expand Down
35 changes: 35 additions & 0 deletions shell/platform/windows/window_proc_delegate_manager_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,41 @@ TEST(WindowProcDelegateManagerTest, RegisterMultiple) {
EXPECT_TRUE(called_b);
}

TEST(WindowProcDelegateManagerTest, Ordered) {
TestWindowProcDelegate delegate_1 = [](HWND hwnd, UINT message, WPARAM wparam,
LPARAM lparam) { return 1; };
TestWindowProcDelegate delegate_2 = [](HWND hwnd, UINT message, WPARAM wparam,
LPARAM lparam) { return 2; };

// Result should be 1 if delegate '1' is registered before delegate '2'.
{
WindowProcDelegateManager manager;
manager.RegisterTopLevelWindowProcDelegate(TestWindowProcCallback,
&delegate_1);
manager.RegisterTopLevelWindowProcDelegate(TestWindowProcCallback2,
&delegate_2);

std::optional<LRESULT> result =
manager.OnTopLevelWindowProc(nullptr, 0, 0, 0);

EXPECT_EQ(result, 1);
}

// Result should be 2 if delegate '2' is registered before delegate '1'.
{
WindowProcDelegateManager manager;
manager.RegisterTopLevelWindowProcDelegate(TestWindowProcCallback2,
&delegate_2);
manager.RegisterTopLevelWindowProcDelegate(TestWindowProcCallback,
&delegate_1);

std::optional<LRESULT> result =
manager.OnTopLevelWindowProc(nullptr, 0, 0, 0);

EXPECT_EQ(result, 2);
}
}

TEST(WindowProcDelegateManagerTest, ConflictingDelegates) {
WindowProcDelegateManager manager;

Expand Down

0 comments on commit fa50a0a

Please sign in to comment.