Skip to content

Commit

Permalink
[Windows] Add ID to views (flutter#50788)
Browse files Browse the repository at this point in the history
Adds an ID to a view:

1. `FlutterWindowsView` now has a ID
2. The `FlutterWindowsEngine::view(...)` accessor now requires a view ID parameter

This is a refactoring with no semantic changes.

Part of flutter/flutter#143765

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
  • Loading branch information
loic-sharma authored Feb 20, 2024
1 parent 1ae2c10 commit cb12a8c
Show file tree
Hide file tree
Showing 20 changed files with 94 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class FlutterWindowsViewSpy : public FlutterWindowsView {
public:
FlutterWindowsViewSpy(FlutterWindowsEngine* engine,
std::unique_ptr<WindowBindingHandler> handler)
: FlutterWindowsView(engine, std::move(handler)) {}
: FlutterWindowsView(kImplicitViewId, engine, std::move(handler)) {}

protected:
virtual std::shared_ptr<AccessibilityBridgeWindows>
Expand Down
5 changes: 4 additions & 1 deletion shell/platform/windows/compositor_opengl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "flutter/shell/platform/windows/compositor_opengl.h"

#include "GLES3/gl3.h"
#include "flutter/shell/platform/windows/flutter_windows_engine.h"
#include "flutter/shell/platform/windows/flutter_windows_view.h"

namespace flutter {
Expand Down Expand Up @@ -93,7 +94,9 @@ bool CompositorOpenGL::CollectBackingStore(const FlutterBackingStore* store) {

bool CompositorOpenGL::Present(const FlutterLayer** layers,
size_t layers_count) {
FlutterWindowsView* view = engine_->view();
// TODO(loicsharma): Remove implicit view assumption.
// https://github.com/flutter/flutter/issues/142845
FlutterWindowsView* view = engine_->view(kImplicitViewId);
if (!view) {
return false;
}
Expand Down
4 changes: 2 additions & 2 deletions shell/platform/windows/compositor_opengl_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ class CompositorOpenGLTest : public WindowsTest {
EXPECT_CALL(*window.get(), SetView).Times(1);
EXPECT_CALL(*window.get(), GetWindowHandle).WillRepeatedly(Return(nullptr));

view_ =
std::make_unique<FlutterWindowsView>(engine_.get(), std::move(window));
view_ = std::make_unique<FlutterWindowsView>(kImplicitViewId, engine_.get(),
std::move(window));

if (add_surface) {
auto surface = std::make_unique<egl::MockWindowSurface>();
Expand Down
10 changes: 7 additions & 3 deletions shell/platform/windows/compositor_software.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

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

#include "flutter/shell/platform/windows/flutter_windows_engine.h"
#include "flutter/shell/platform/windows/flutter_windows_view.h"

namespace flutter {
Expand Down Expand Up @@ -39,13 +40,16 @@ bool CompositorSoftware::CollectBackingStore(const FlutterBackingStore* store) {

bool CompositorSoftware::Present(const FlutterLayer** layers,
size_t layers_count) {
if (!engine_->view()) {
// TODO(loicsharma): Remove implicit view assumption.
// https://github.com/flutter/flutter/issues/142845
FlutterWindowsView* view = engine_->view(kImplicitViewId);
if (!view) {
return false;
}

// Clear the view if there are no layers to present.
if (layers_count == 0) {
return engine_->view()->ClearSoftwareBitmap();
return view->ClearSoftwareBitmap();
}

// TODO: Support compositing layers and platform views.
Expand All @@ -58,7 +62,7 @@ bool CompositorSoftware::Present(const FlutterLayer** layers,

const auto& backing_store = layers[0]->backing_store->software;

return engine_->view()->PresentSoftwareBitmap(
return view->PresentSoftwareBitmap(
backing_store.allocation, backing_store.row_bytes, backing_store.height);
}

Expand Down
2 changes: 1 addition & 1 deletion shell/platform/windows/compositor_software_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class MockFlutterWindowsView : public FlutterWindowsView {
public:
MockFlutterWindowsView(FlutterWindowsEngine* engine,
std::unique_ptr<WindowBindingHandler> window)
: FlutterWindowsView(engine, std::move(window)) {}
: FlutterWindowsView(kImplicitViewId, engine, std::move(window)) {}
virtual ~MockFlutterWindowsView() = default;

MOCK_METHOD(bool,
Expand Down
10 changes: 8 additions & 2 deletions shell/platform/windows/cursor_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,10 @@ void CursorHandler::HandleMethodCall(
return;
}
const auto& kind = std::get<std::string>(kind_iter->second);
FlutterWindowsView* view = engine_->view();

// TODO(loicsharma): Remove implicit view assumption.
// https://github.com/flutter/flutter/issues/142845
FlutterWindowsView* view = engine_->view(kImplicitViewId);
if (view == nullptr) {
result->Error(kCursorError,
"Cursor is not available in Windows headless mode");
Expand Down Expand Up @@ -164,7 +167,10 @@ void CursorHandler::HandleMethodCall(
return;
}
HCURSOR cursor = custom_cursors_[name];
FlutterWindowsView* view = engine_->view();

// TODO(loicsharma): Remove implicit view assumption.
// https://github.com/flutter/flutter/issues/142845
FlutterWindowsView* view = engine_->view(kImplicitViewId);
if (view == nullptr) {
result->Error(kCursorError,
"Cursor is not available in Windows headless mode");
Expand Down
4 changes: 2 additions & 2 deletions shell/platform/windows/cursor_handler_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ class CursorHandlerTest : public WindowsTest {

window_ = window.get();
engine_ = builder.Build();
view_ =
std::make_unique<FlutterWindowsView>(engine_.get(), std::move(window));
view_ = std::make_unique<FlutterWindowsView>(kImplicitViewId, engine_.get(),
std::move(window));

EngineModifier modifier{engine_.get()};
modifier.SetImplicitView(view_.get());
Expand Down
3 changes: 2 additions & 1 deletion shell/platform/windows/flutter_window_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ class MockFlutterWindowsView : public FlutterWindowsView {
public:
MockFlutterWindowsView(FlutterWindowsEngine* engine,
std::unique_ptr<WindowBindingHandler> window_binding)
: FlutterWindowsView(engine, std::move(window_binding)) {}
: FlutterWindowsView(kImplicitViewId, engine, std::move(window_binding)) {
}
~MockFlutterWindowsView() {}

MOCK_METHOD(void,
Expand Down
5 changes: 4 additions & 1 deletion shell/platform/windows/flutter_windows.cc
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,10 @@ bool FlutterDesktopEngineProcessExternalWindowMessage(

FlutterDesktopViewRef FlutterDesktopPluginRegistrarGetView(
FlutterDesktopPluginRegistrarRef registrar) {
return HandleForView(registrar->engine->view());
// TODO(loicsharma): Add |FlutterDesktopPluginRegistrarGetViewById| and
// deprecate this API as it makes single view assumptions.
// https://github.com/flutter/flutter/issues/143767
return HandleForView(registrar->engine->view(flutter::kImplicitViewId));
}

void FlutterDesktopPluginRegistrarRegisterTopLevelWindowProcDelegate(
Expand Down
17 changes: 14 additions & 3 deletions shell/platform/windows/flutter_windows_engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,10 @@ bool FlutterWindowsEngine::Run(std::string_view entrypoint) {
args.update_semantics_callback2 = [](const FlutterSemanticsUpdate2* update,
void* user_data) {
auto host = static_cast<FlutterWindowsEngine*>(user_data);
auto view = host->view();

// TODO(loicsharma): Remove implicit view assumption.
// https://github.com/flutter/flutter/issues/142845
auto view = host->view(kImplicitViewId);
if (!view) {
return;
}
Expand Down Expand Up @@ -485,8 +488,10 @@ bool FlutterWindowsEngine::Stop() {

std::unique_ptr<FlutterWindowsView> FlutterWindowsEngine::CreateView(
std::unique_ptr<WindowBindingHandler> window) {
auto view = std::make_unique<FlutterWindowsView>(this, std::move(window),
windows_proc_table_);
// TODO(loicsharma): Remove implicit view assumption.
// https://github.com/flutter/flutter/issues/142845
auto view = std::make_unique<FlutterWindowsView>(
kImplicitViewId, this, std::move(window), windows_proc_table_);

view_ = view.get();
InitializeKeyboard();
Expand Down Expand Up @@ -522,6 +527,12 @@ std::chrono::nanoseconds FlutterWindowsEngine::FrameInterval() {
return std::chrono::nanoseconds(interval);
}

FlutterWindowsView* FlutterWindowsEngine::view(FlutterViewId view_id) const {
FML_DCHECK(view_id == kImplicitViewId);

return view_;
}

// Returns the currently configured Plugin Registrar.
FlutterDesktopPluginRegistrarRef FlutterWindowsEngine::GetRegistrar() {
return plugin_registrar_.get();
Expand Down
8 changes: 7 additions & 1 deletion shell/platform/windows/flutter_windows_engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@

namespace flutter {

// The implicit view's ID.
//
// See:
// https://api.flutter.dev/flutter/dart-ui/PlatformDispatcher/implicitView.html
constexpr FlutterViewId kImplicitViewId = 0;

class FlutterWindowsView;

// Update the thread priority for the Windows engine.
Expand Down Expand Up @@ -117,7 +123,7 @@ class FlutterWindowsEngine {

// The view displaying this engine's content, if any. This will be null for
// headless engines.
FlutterWindowsView* view() { return view_; }
FlutterWindowsView* view(FlutterViewId view_id) const;

// Returns the currently configured Plugin Registrar.
FlutterDesktopPluginRegistrarRef GetRegistrar();
Expand Down
2 changes: 1 addition & 1 deletion shell/platform/windows/flutter_windows_engine_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ class MockFlutterWindowsView : public FlutterWindowsView {
public:
MockFlutterWindowsView(FlutterWindowsEngine* engine,
std::unique_ptr<WindowBindingHandler> wbh)
: FlutterWindowsView(engine, std::move(wbh)) {}
: FlutterWindowsView(kImplicitViewId, engine, std::move(wbh)) {}
~MockFlutterWindowsView() {}

MOCK_METHOD(void,
Expand Down
9 changes: 8 additions & 1 deletion shell/platform/windows/flutter_windows_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,13 @@ void UpdateVsync(const FlutterWindowsEngine& engine,
} // namespace

FlutterWindowsView::FlutterWindowsView(
FlutterViewId view_id,
FlutterWindowsEngine* engine,
std::unique_ptr<WindowBindingHandler> window_binding,
std::shared_ptr<WindowsProcTable> windows_proc_table)
: engine_(engine), windows_proc_table_(std::move(windows_proc_table)) {
: view_id_(view_id),
engine_(engine),
windows_proc_table_(std::move(windows_proc_table)) {
if (windows_proc_table_ == nullptr) {
windows_proc_table_ = std::make_shared<WindowsProcTable>();
}
Expand Down Expand Up @@ -652,6 +655,10 @@ bool FlutterWindowsView::PresentSoftwareBitmap(const void* allocation,
height);
}

FlutterViewId FlutterWindowsView::view_id() const {
return view_id_;
}

void FlutterWindowsView::CreateRenderSurface() {
FML_DCHECK(surface_ == nullptr);

Expand Down
10 changes: 10 additions & 0 deletions shell/platform/windows/flutter_windows_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,26 @@

namespace flutter {

// A unique identifier for a view.
using FlutterViewId = int64_t;

// An OS-windowing neutral abstration for a Flutter view that works
// with win32 HWNDs.
class FlutterWindowsView : public WindowBindingHandlerDelegate {
public:
// Creates a FlutterWindowsView with the given implementor of
// WindowBindingHandler.
FlutterWindowsView(
FlutterViewId view_id,
FlutterWindowsEngine* engine,
std::unique_ptr<WindowBindingHandler> window_binding,
std::shared_ptr<WindowsProcTable> windows_proc_table = nullptr);

virtual ~FlutterWindowsView();

// Get the view's unique identifier.
FlutterViewId view_id() const;

// Creates rendering surface for Flutter engine to draw into.
// Should be called before calling FlutterEngineRun using this view.
void CreateRenderSurface();
Expand Down Expand Up @@ -379,6 +386,9 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate {
// to prevent screen tearing.
bool NeedsVsync() const;

// The view's unique identifier.
FlutterViewId view_id_;

// The engine associated with this view.
FlutterWindowsEngine* engine_ = nullptr;

Expand Down
2 changes: 1 addition & 1 deletion shell/platform/windows/flutter_windows_view_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1029,7 +1029,7 @@ TEST(FlutterWindowsViewTest, WindowRepaintTests) {
std::unique_ptr<FlutterWindowsEngine> engine = GetTestEngine();
EngineModifier modifier(engine.get());

FlutterWindowsView view{engine.get(),
FlutterWindowsView view{kImplicitViewId, engine.get(),
std::make_unique<flutter::FlutterWindow>(100, 100)};

bool schedule_frame_called = false;
Expand Down
2 changes: 1 addition & 1 deletion shell/platform/windows/keyboard_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ class TestFlutterWindowsView : public FlutterWindowsView {
std::unique_ptr<WindowBindingHandler> window,
std::function<void(KeyCall)> on_key_call)
: on_key_call_(on_key_call),
FlutterWindowsView(engine, std::move(window)) {}
FlutterWindowsView(kImplicitViewId, engine, std::move(window)) {}

void OnText(const std::u16string& text) override {
on_key_call_(KeyCall{
Expand Down
12 changes: 9 additions & 3 deletions shell/platform/windows/platform_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,9 @@ PlatformHandler::~PlatformHandler() = default;
void PlatformHandler::GetPlainText(
std::unique_ptr<MethodResult<rapidjson::Document>> result,
std::string_view key) {
const FlutterWindowsView* view = engine_->view();
// TODO(loicsharma): Remove implicit view assumption.
// https://github.com/flutter/flutter/issues/142845
const FlutterWindowsView* view = engine_->view(kImplicitViewId);
if (view == nullptr) {
result->Error(kClipboardError,
"Clipboard is not available in Windows headless mode");
Expand Down Expand Up @@ -291,7 +293,9 @@ void PlatformHandler::GetPlainText(

void PlatformHandler::GetHasStrings(
std::unique_ptr<MethodResult<rapidjson::Document>> result) {
const FlutterWindowsView* view = engine_->view();
// TODO(loicsharma): Remove implicit view assumption.
// https://github.com/flutter/flutter/issues/142845
const FlutterWindowsView* view = engine_->view(kImplicitViewId);
if (view == nullptr) {
result->Error(kClipboardError,
"Clipboard is not available in Windows headless mode");
Expand Down Expand Up @@ -329,7 +333,9 @@ void PlatformHandler::GetHasStrings(
void PlatformHandler::SetPlainText(
const std::string& text,
std::unique_ptr<MethodResult<rapidjson::Document>> result) {
const FlutterWindowsView* view = engine_->view();
// TODO(loicsharma): Remove implicit view assumption.
// https://github.com/flutter/flutter/issues/142845
const FlutterWindowsView* view = engine_->view(kImplicitViewId);
if (view == nullptr) {
result->Error(kClipboardError,
"Clipboard is not available in Windows headless mode");
Expand Down
4 changes: 2 additions & 2 deletions shell/platform/windows/platform_handler_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,8 @@ class PlatformHandlerTest : public WindowsTest {
auto window = std::make_unique<NiceMock<MockWindowBindingHandler>>();

engine_ = builder.Build();
view_ =
std::make_unique<FlutterWindowsView>(engine_.get(), std::move(window));
view_ = std::make_unique<FlutterWindowsView>(kImplicitViewId, engine_.get(),
std::move(window));

EngineModifier modifier{engine_.get()};
modifier.SetImplicitView(view_.get());
Expand Down
12 changes: 9 additions & 3 deletions shell/platform/windows/text_input_plugin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,9 @@ void TextInputPlugin::HandleMethodCall(
if (method.compare(kShowMethod) == 0 || method.compare(kHideMethod) == 0) {
// These methods are no-ops.
} else if (method.compare(kClearClientMethod) == 0) {
FlutterWindowsView* view = engine_->view();
// TODO(loicsharma): Remove implicit view assumption.
// https://github.com/flutter/flutter/issues/142845
FlutterWindowsView* view = engine_->view(kImplicitViewId);
if (view == nullptr) {
result->Error(kInternalConsistencyError,
"Text input is not available in Windows headless mode");
Expand Down Expand Up @@ -326,7 +328,9 @@ void TextInputPlugin::HandleMethodCall(
TextRange(composing_base, composing_extent), cursor_offset);
}
} else if (method.compare(kSetMarkedTextRect) == 0) {
FlutterWindowsView* view = engine_->view();
// TODO(loicsharma): Remove implicit view assumption.
// https://github.com/flutter/flutter/issues/142845
FlutterWindowsView* view = engine_->view(kImplicitViewId);
if (view == nullptr) {
result->Error(kInternalConsistencyError,
"Text input is not available in Windows headless mode");
Expand Down Expand Up @@ -355,7 +359,9 @@ void TextInputPlugin::HandleMethodCall(
Rect transformed_rect = GetCursorRect();
view->OnCursorRectUpdated(transformed_rect);
} else if (method.compare(kSetEditableSizeAndTransform) == 0) {
FlutterWindowsView* view = engine_->view();
// TODO(loicsharma): Remove implicit view assumption.
// https://github.com/flutter/flutter/issues/142845
FlutterWindowsView* view = engine_->view(kImplicitViewId);
if (view == nullptr) {
result->Error(kInternalConsistencyError,
"Text input is not available in Windows headless mode");
Expand Down
2 changes: 1 addition & 1 deletion shell/platform/windows/text_input_plugin_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ class MockFlutterWindowsView : public FlutterWindowsView {
public:
MockFlutterWindowsView(FlutterWindowsEngine* engine,
std::unique_ptr<WindowBindingHandler> window)
: FlutterWindowsView(engine, std::move(window)) {}
: FlutterWindowsView(kImplicitViewId, engine, std::move(window)) {}
virtual ~MockFlutterWindowsView() = default;

MOCK_METHOD(void, OnCursorRectUpdated, (const Rect&), (override));
Expand Down

0 comments on commit cb12a8c

Please sign in to comment.