Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Add Platform View Manager to Windows shell #50598

Merged
merged 29 commits into from
Feb 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
500e60c
Define structs, class
yaakovschectman Feb 8, 2024
8dd4543
Define PV type registration runner API function
yaakovschectman Feb 8, 2024
ae0d049
Bare minimum PlatformViewHandler
yaakovschectman Feb 8, 2024
5f14af0
Compositor owns platform view manager
yaakovschectman Feb 8, 2024
200f13a
Set up for mocking
yaakovschectman Feb 8, 2024
ed90810
Add unit test
yaakovschectman Feb 13, 2024
5d410ff
Formatting
yaakovschectman Feb 13, 2024
484c8c1
License fix, await future
yaakovschectman Feb 13, 2024
b8fadd7
PR Feedback
yaakovschectman Feb 13, 2024
1d203f4
_internal header file
yaakovschectman Feb 14, 2024
b9acc1a
Wait to add manager to compositors
yaakovschectman Feb 14, 2024
35eb688
Rename fixture function
yaakovschectman Feb 14, 2024
659ebd6
Reorder parameters
yaakovschectman Feb 14, 2024
db2851b
Use constants
yaakovschectman Feb 14, 2024
db48ddd
Reorder params in test
yaakovschectman Feb 14, 2024
f87ed96
Format
yaakovschectman Feb 14, 2024
42f976e
License
yaakovschectman Feb 14, 2024
284b309
Break up PlatformViewManager
yaakovschectman Feb 16, 2024
e000930
PR Feedback
yaakovschectman Feb 16, 2024
676868f
Formatting
yaakovschectman Feb 16, 2024
6e6ef7e
PR Feedback
yaakovschectman Feb 16, 2024
22a9e46
Update shell/platform/windows/flutter_windows_engine_unittests.cc
yaakovschectman Feb 21, 2024
95a8498
Update shell/platform/windows/platform_view_manager.h
yaakovschectman Feb 21, 2024
1fa97f3
Update shell/platform/windows/public/flutter_windows.h
yaakovschectman Feb 21, 2024
5453b29
Update shell/platform/windows/flutter_windows_engine_unittests.cc
yaakovschectman Feb 21, 2024
dfa6b73
PR Feedback
yaakovschectman Feb 21, 2024
cde468e
Merge branch 'main' into win_pv_i
yaakovschectman Feb 21, 2024
585ca11
Include internal header
yaakovschectman Feb 22, 2024
54d91a3
Merge branch 'main' into win_pv_i
yaakovschectman Feb 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -11628,6 +11628,7 @@ ORIGIN: ../../../flutter/shell/platform/windows/flutter_window.h + ../../../flut
ORIGIN: ../../../flutter/shell/platform/windows/flutter_windows.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/flutter_windows_engine.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/flutter_windows_engine.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/flutter_windows_internal.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/flutter_windows_texture_registrar.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/flutter_windows_texture_registrar.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/flutter_windows_view.cc + ../../../flutter/LICENSE
Expand All @@ -11646,6 +11647,10 @@ ORIGIN: ../../../flutter/shell/platform/windows/keyboard_utils.cc + ../../../flu
ORIGIN: ../../../flutter/shell/platform/windows/keyboard_utils.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/platform_handler.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/platform_handler.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/platform_view_manager.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/platform_view_manager.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/platform_view_plugin.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/platform_view_plugin.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/public/flutter_windows.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/sequential_id_generator.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/sequential_id_generator.h + ../../../flutter/LICENSE
Expand Down Expand Up @@ -14490,6 +14495,7 @@ FILE: ../../../flutter/shell/platform/windows/flutter_window.h
FILE: ../../../flutter/shell/platform/windows/flutter_windows.cc
FILE: ../../../flutter/shell/platform/windows/flutter_windows_engine.cc
FILE: ../../../flutter/shell/platform/windows/flutter_windows_engine.h
FILE: ../../../flutter/shell/platform/windows/flutter_windows_internal.h
FILE: ../../../flutter/shell/platform/windows/flutter_windows_texture_registrar.cc
FILE: ../../../flutter/shell/platform/windows/flutter_windows_texture_registrar.h
FILE: ../../../flutter/shell/platform/windows/flutter_windows_view.cc
Expand All @@ -14508,6 +14514,10 @@ FILE: ../../../flutter/shell/platform/windows/keyboard_utils.cc
FILE: ../../../flutter/shell/platform/windows/keyboard_utils.h
FILE: ../../../flutter/shell/platform/windows/platform_handler.cc
FILE: ../../../flutter/shell/platform/windows/platform_handler.h
FILE: ../../../flutter/shell/platform/windows/platform_view_manager.cc
FILE: ../../../flutter/shell/platform/windows/platform_view_manager.h
FILE: ../../../flutter/shell/platform/windows/platform_view_plugin.cc
FILE: ../../../flutter/shell/platform/windows/platform_view_plugin.h
FILE: ../../../flutter/shell/platform/windows/public/flutter_windows.h
FILE: ../../../flutter/shell/platform/windows/sequential_id_generator.cc
FILE: ../../../flutter/shell/platform/windows/sequential_id_generator.h
Expand Down
8 changes: 7 additions & 1 deletion shell/platform/windows/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import("//flutter/shell/platform/glfw/config.gni")
import("//flutter/testing/testing.gni")

_public_headers = [ "public/flutter_windows.h" ]
_internal_headers = [ "flutter_windows_internal.h" ]

config("relative_angle_headers") {
include_dirs = [ "//third_party/angle/include" ]
Expand All @@ -24,7 +25,7 @@ config("relative_flutter_windows_headers") {
# The headers are a separate source set since the client wrapper is allowed
# to depend on the public headers, but none of the rest of the code.
source_set("flutter_windows_headers") {
public = _public_headers
public = _public_headers + _internal_headers

public_deps = [ "//flutter/shell/platform/common:common_cpp_library_headers" ]

Expand Down Expand Up @@ -98,6 +99,10 @@ source_set("flutter_windows_source") {
"keyboard_utils.h",
"platform_handler.cc",
"platform_handler.h",
"platform_view_manager.cc",
"platform_view_manager.h",
"platform_view_plugin.cc",
"platform_view_plugin.h",
"sequential_id_generator.cc",
"sequential_id_generator.h",
"settings_plugin.cc",
Expand Down Expand Up @@ -218,6 +223,7 @@ executable("flutter_windows_unittests") {
"testing/flutter_windows_engine_builder.cc",
"testing/flutter_windows_engine_builder.h",
"testing/mock_direct_manipulation.h",
"testing/mock_platform_view_manager.h",
"testing/mock_text_input_manager.cc",
"testing/mock_text_input_manager.h",
"testing/mock_window.cc",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,16 @@ bool FlutterDesktopEngineProcessExternalWindowMessage(
return false;
}

void FlutterDesktopEngineRegisterPlatformViewType(
FlutterDesktopEngineRef engine,
const char* view_type_name,
FlutterPlatformViewTypeEntry view_type) {
if (s_stub_implementation) {
s_stub_implementation->EngineRegisterPlatformViewType(view_type_name,
view_type);
}
}

FlutterDesktopViewRef FlutterDesktopPluginRegistrarGetView(
FlutterDesktopPluginRegistrarRef controller) {
// The stub ignores this, so just return an arbitrary non-zero value.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include <memory>

#include "flutter/shell/platform/windows/flutter_windows_internal.h"
#include "flutter/shell/platform/windows/public/flutter_windows.h"

namespace flutter {
Expand Down Expand Up @@ -72,6 +73,11 @@ class StubFlutterWindowsApi {
// Called for FlutterDesktopEngineReloadSystemFonts.
virtual void EngineReloadSystemFonts() {}

// Called for FlutterDesktopEngineRegisterPlatformViewType.
virtual void EngineRegisterPlatformViewType(
const char* view_type_name,
FlutterPlatformViewTypeEntry view_type) {}

// Called for FlutterDesktopViewGetHWND.
virtual HWND ViewGetHWND() { return reinterpret_cast<HWND>(1); }

Expand Down
33 changes: 32 additions & 1 deletion shell/platform/windows/fixtures/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import 'dart:async';
import 'dart:io' as io;
import 'dart:typed_data' show ByteData, Uint8List;
import 'dart:typed_data' show ByteData, Endian, Uint8List;
import 'dart:ui' as ui;
import 'dart:convert';

Expand Down Expand Up @@ -159,6 +159,37 @@ void enableLifecycleToFrom() async {
});
}

@pragma('vm:entry-point')
void sendCreatePlatformViewMethod() async {
// The platform view method channel uses the standard method codec.
// See https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/services/message_codecs.dart#L262
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use a permalink here. The line number will be wrong as the code continues to evolve

// for the implementation of the encoding and magic number identifiers.
const int valueString = 7;
const int valueMap = 13;
const int valueInt32 = 3;
const String method = 'create';
const String typeKey = 'viewType';
const String typeValue = 'type';
const String idKey = 'id';
final List<int> data = <int>[
// Method name
valueString, method.length, ...utf8.encode(method),
// Method arguments: {'type': 'type':, 'id': 0}
valueMap, 2,
valueString, typeKey.length, ...utf8.encode(typeKey),
valueString, typeValue.length, ...utf8.encode(typeValue),
valueString, idKey.length, ...utf8.encode(idKey),
valueInt32, 0, 0, 0, 0,
];

final Completer<ByteData?> completed = Completer<ByteData?>();
final ByteData bytes = ByteData.sublistView(Uint8List.fromList(data));
ui.PlatformDispatcher.instance.sendPlatformMessage('flutter/platform_views', bytes, (ByteData? response) {
completed.complete(response);
});
await completed.future;
}

@pragma('vm:entry-point')
void customEntrypoint() {}

Expand Down
8 changes: 8 additions & 0 deletions shell/platform/windows/flutter_windows.cc
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,14 @@ bool FlutterDesktopEngineProcessExternalWindowMessage(
return lresult.has_value();
}

void FlutterDesktopEngineRegisterPlatformViewType(
FlutterDesktopEngineRef engine,
const char* view_type_name,
FlutterPlatformViewTypeEntry view_type) {
// TODO(schectman): forward to platform view manager.
// https://github.com/flutter/flutter/issues/143375
}

FlutterDesktopViewRef FlutterDesktopPluginRegistrarGetView(
FlutterDesktopPluginRegistrarRef registrar) {
// TODO(loicsharma): Add |FlutterDesktopPluginRegistrarGetViewById| and
Expand Down
6 changes: 6 additions & 0 deletions shell/platform/windows/flutter_windows_engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -383,11 +383,17 @@ bool FlutterWindowsEngine::Run(std::string_view entrypoint) {

args.custom_task_runners = &custom_task_runners;

if (!platform_view_plugin_) {
platform_view_plugin_ = std::make_unique<PlatformViewPlugin>(
messenger_wrapper_.get(), task_runner_.get());
}
if (egl_manager_) {
auto resolver = [](const char* name) -> void* {
return reinterpret_cast<void*>(::eglGetProcAddress(name));
};

// TODO(schectman) Pass the platform view manager to the compositor
// constructors: https://github.com/flutter/flutter/issues/143375
compositor_ = std::make_unique<CompositorOpenGL>(this, resolver);
} else {
compositor_ = std::make_unique<CompositorSoftware>(this);
Expand Down
5 changes: 5 additions & 0 deletions shell/platform/windows/flutter_windows_engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "flutter/shell/platform/windows/keyboard_handler_base.h"
#include "flutter/shell/platform/windows/keyboard_key_embedder_handler.h"
#include "flutter/shell/platform/windows/platform_handler.h"
#include "flutter/shell/platform/windows/platform_view_plugin.h"
#include "flutter/shell/platform/windows/public/flutter_windows.h"
#include "flutter/shell/platform/windows/settings_plugin.h"
#include "flutter/shell/platform/windows/task_runner.h"
Expand Down Expand Up @@ -144,6 +145,8 @@ class FlutterWindowsEngine {

TaskRunner* task_runner() { return task_runner_.get(); }

BinaryMessenger* messenger_wrapper() { return messenger_wrapper_.get(); }

FlutterWindowsTextureRegistrar* texture_registrar() {
return texture_registrar_.get();
}
Expand Down Expand Up @@ -434,6 +437,8 @@ class FlutterWindowsEngine {

std::shared_ptr<egl::ProcTable> gl_;

std::unique_ptr<PlatformViewPlugin> platform_view_plugin_;

FML_DISALLOW_COPY_AND_ASSIGN(FlutterWindowsEngine);
};

Expand Down
26 changes: 26 additions & 0 deletions shell/platform/windows/flutter_windows_engine_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "flutter/shell/platform/windows/testing/egl/mock_manager.h"
#include "flutter/shell/platform/windows/testing/engine_modifier.h"
#include "flutter/shell/platform/windows/testing/flutter_windows_engine_builder.h"
#include "flutter/shell/platform/windows/testing/mock_platform_view_manager.h"
#include "flutter/shell/platform/windows/testing/mock_window_binding_handler.h"
#include "flutter/shell/platform/windows/testing/mock_windows_proc_table.h"
#include "flutter/shell/platform/windows/testing/test_keyboard.h"
Expand Down Expand Up @@ -1171,5 +1172,30 @@ TEST_F(FlutterWindowsEngineTest, ChannelListenedTo) {
}
}

TEST_F(FlutterWindowsEngineTest, ReceivePlatformViewMessage) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my understanding, this file is more for engine unit tests. Since this launches & runs a full app, should we move this to the integration tests in flutter_windows_unittests.cc?

I don't feel strongly about this, feel free to keep as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that this test is quite similar in most regards to the handful of unit tests immediately preceding it in this file. Would you disagree?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's probably fine to land as-is, but I do think we should probably do a followup evaluation of which tests belong where. We're definitely not super consistent and to be honest, I'm not sure we've ever drawn a really clear line (not just in the Windows embedder but also on macOS).

FlutterWindowsEngineBuilder builder{GetContext()};
builder.SetDartEntrypoint("sendCreatePlatformViewMethod");
auto engine = builder.Build();

EngineModifier modifier{engine.get()};
modifier.embedder_api().RunsAOTCompiledDartCode = []() { return false; };

bool received_call = false;

auto manager = std::make_unique<MockPlatformViewManager>(engine.get());
EXPECT_CALL(*manager, AddPlatformView)
.WillOnce([&](PlatformViewId id, std::string_view type_name) {
received_call = true;
return true;
});
modifier.SetPlatformViewPlugin(std::move(manager));

engine->Run();

while (!received_call) {
engine->task_runner()->ProcessTasks();
}
}

} // namespace testing
} // namespace flutter
47 changes: 47 additions & 0 deletions shell/platform/windows/flutter_windows_internal.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// 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.

#ifndef FLUTTER_SHELL_PLATFORM_WINDOWS_FLUTTER_WINDOWS_INTERNAL_H_
#define FLUTTER_SHELL_PLATFORM_WINDOWS_FLUTTER_WINDOWS_INTERNAL_H_

#include "flutter/shell/platform/windows/public/flutter_windows.h"

#if defined(__cplusplus)
extern "C" {
#endif

// Declare functions that are currently in-progress and shall be exposed to the
// public facing API upon completion.

typedef int64_t PlatformViewId;

typedef struct {
size_t struct_size;
HWND parent_window;
const char* platform_view_type;
// user_data may hold any necessary additional information for creating a new
// platform view. For example, an instance of FlutterWindow.
void* user_data;
PlatformViewId platform_view_id;
} FlutterPlatformViewCreationParameters;

typedef HWND (*FlutterPlatformViewFactory)(
const FlutterPlatformViewCreationParameters*);

typedef struct {
size_t struct_size;
FlutterPlatformViewFactory factory;
void* user_data; // Arbitrary user data supplied to the creation struct.
} FlutterPlatformViewTypeEntry;

FLUTTER_EXPORT void FlutterDesktopEngineRegisterPlatformViewType(
FlutterDesktopEngineRef engine,
const char* view_type_name,
FlutterPlatformViewTypeEntry view_type);

#if defined(__cplusplus)
}
#endif

#endif // FLUTTER_SHELL_PLATFORM_WINDOWS_FLUTTER_WINDOWS_INTERNAL_H_
84 changes: 84 additions & 0 deletions shell/platform/windows/platform_view_manager.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
// 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/platform_view_manager.h"

#include "flutter/shell/platform/common/client_wrapper/include/flutter/standard_method_codec.h"

namespace flutter {

namespace {
constexpr char kChannelName[] = "flutter/platform_views";
constexpr char kCreateMethod[] = "create";
constexpr char kFocusMethod[] = "focus";
constexpr char kViewTypeParameter[] = "viewType";
constexpr char kIdParameter[] = "id";
constexpr char kDirectionParameter[] = "direction";
constexpr char kFocusParameter[] = "focus";
} // namespace

PlatformViewManager::PlatformViewManager(BinaryMessenger* binary_messenger)
: channel_(std::make_unique<MethodChannel<EncodableValue>>(
binary_messenger,
kChannelName,
&StandardMethodCodec::GetInstance())) {
channel_->SetMethodCallHandler(
[this](const MethodCall<EncodableValue>& call,
std::unique_ptr<MethodResult<EncodableValue>> result) {
const auto& args = std::get<EncodableMap>(*call.arguments());
if (call.method_name() == kCreateMethod) {
const auto& type_itr = args.find(EncodableValue(kViewTypeParameter));
const auto& id_itr = args.find(EncodableValue(kIdParameter));
if (type_itr == args.end()) {
result->Error("AddPlatformView", "Parameter viewType is required");
return;
}
if (id_itr == args.end()) {
result->Error("AddPlatformView", "Parameter id is required");
return;
}
const auto& type = std::get<std::string>(type_itr->second);
const auto& id = std::get<std::int32_t>(id_itr->second);
if (AddPlatformView(id, type)) {
result->Success();
} else {
result->Error("AddPlatformView", "Failed to add platform view");
}
return;
} else if (call.method_name() == kFocusMethod) {
const auto& id_itr = args.find(EncodableValue(kIdParameter));
const auto& direction_itr =
args.find(EncodableValue(kDirectionParameter));
const auto& focus_itr = args.find(EncodableValue(kFocusParameter));
if (id_itr == args.end()) {
result->Error("FocusPlatformView", "Parameter id is required");
return;
}
if (direction_itr == args.end()) {
result->Error("FocusPlatformView",
"Parameter direction is required");
return;
}
if (focus_itr == args.end()) {
result->Error("FocusPlatformView", "Parameter focus is required");
return;
}
const auto& id = std::get<std::int32_t>(id_itr->second);
const auto& direction = std::get<std::int32_t>(direction_itr->second);
const auto& focus = std::get<bool>(focus_itr->second);
if (FocusPlatformView(
id, static_cast<FocusChangeDirection>(direction), focus)) {
result->Success();
} else {
result->Error("FocusPlatformView", "Failed to focus platform view");
}
return;
}
result->NotImplemented();
});
}

PlatformViewManager::~PlatformViewManager() {}

} // namespace flutter
Loading