Skip to content

Commit

Permalink
[fuchsia] Replace deprecated AddLocalChild (flutter#38788)
Browse files Browse the repository at this point in the history 
`AddLocalChild(LocalComponent*)` is deprecated.

It was replaced by:

`AddLocalChild(LocalComponentFactory)`, which is a type alias for a
lambda that returns a `std::unique_ptr<LocalComponentImpl>`.

This change addresses problems that arised due to object lifetime
management of the components, and allows RealmBuilder to model the
component lifecycle of local components in a way that's more consistent
with other components.

The RealmBuilder-built `Realm` now owns the lifetime of the local
components, instead of the client, and those objects are valid until
the `Realm` is destroyed.

Bug: fxbug.dev/109804
  • Loading branch information
@richkadel
richkadel authored and Ricardo Amador committed Jan 25, 2023
1 parent 9eb31b2 commit 6abf987
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 43 deletions.
29 changes: 15 additions & 14 deletions shell/platform/fuchsia/flutter/tests/integration/mouse-input/mouse-input-test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ namespace {
// Types imported for the realm_builder library.
using component_testing::ChildRef;
using component_testing::ConfigValue;
using component_testing::LocalComponent;
using component_testing::LocalComponentHandles;
using component_testing::LocalComponentImpl;
using component_testing::ParentRef;
using component_testing::Protocol;
using component_testing::Realm;
Expand Down Expand Up @@ -109,7 +108,7 @@ int ButtonsToInt(
// us know what position and button press state the mouse cursor has.
class MouseInputListenerServer
: public fuchsia::ui::test::input::MouseInputListener,
public LocalComponent {
public LocalComponentImpl {
public:
explicit MouseInputListenerServer(async_dispatcher_t* dispatcher)
: dispatcher_(dispatcher) {}
Expand All @@ -121,19 +120,18 @@ class MouseInputListenerServer
events_.push(std::move(request));
}

// |MockComponent::Start|
// |MockComponent::OnStart|
// When the component framework requests for this component to start, this
// method will be invoked by the realm_builder library.
void Start(std::unique_ptr<LocalComponentHandles> mock_handles) override {
void OnStart() override {
FML_LOG(INFO) << "Starting MouseInputServer";
ASSERT_EQ(ZX_OK, mock_handles->outgoing()->AddPublicService(
ASSERT_EQ(ZX_OK, outgoing()->AddPublicService(
fidl::InterfaceRequestHandler<
fuchsia::ui::test::input::MouseInputListener>(
[this](auto request) {
bindings_.AddBinding(this, std::move(request),
dispatcher_);
})));
mock_handles_.emplace_back(std::move(mock_handles));
}

size_t SizeOfEvents() const { return events_.size(); }
Expand All @@ -156,7 +154,6 @@ class MouseInputListenerServer
// Not owned.
async_dispatcher_t* dispatcher_ = nullptr;
fidl::BindingSet<fuchsia::ui::test::input::MouseInputListener> bindings_;
std::vector<std::unique_ptr<LocalComponentHandles>> mock_handles_;
std::queue<
fuchsia::ui::test::input::MouseInputListenerReportMouseInputRequest>
events_;
Expand Down Expand Up @@ -191,7 +188,7 @@ class MouseInputTest : public PortableUITest,
}

MouseInputListenerServer* mouse_input_listener() {
return mouse_input_listener_.get();
return mouse_input_listener_;
}

// Helper method for checking the test.mouse.MouseInputListener response from
Expand Down Expand Up @@ -254,13 +251,17 @@ class MouseInputTest : public PortableUITest,
private:
void ExtendRealm() override {
FML_LOG(INFO) << "Extending realm";
mouse_input_listener_ =
std::make_unique<MouseInputListenerServer>(dispatcher());

// Key part of service setup: have this test component vend the
// |MouseInputListener| service in the constructed realm.
realm_builder()->AddLocalChild(kMouseInputListener,
mouse_input_listener_.get());
auto mouse_input_listener =
std::make_unique<MouseInputListenerServer>(dispatcher());
mouse_input_listener_ = mouse_input_listener.get();
realm_builder()->AddLocalChild(
kMouseInputListener,
[mouse_input_listener = std::move(mouse_input_listener)]() mutable {
return std::move(mouse_input_listener);
});

realm_builder()->AddChild(kMouseInputView, kMouseInputViewUrl,
component_testing::ChildOptions{
Expand All @@ -281,7 +282,7 @@ class MouseInputTest : public PortableUITest,

ParamType GetTestUIStackUrl() override { return GetParam(); };

std::unique_ptr<MouseInputListenerServer> mouse_input_listener_;
MouseInputListenerServer* mouse_input_listener_;

fuchsia::ui::scenic::ScenicPtr scenic_;
uint32_t display_width_ = 0;
Expand Down
25 changes: 13 additions & 12 deletions shell/platform/fuchsia/flutter/tests/integration/text-input/text-input-test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ namespace {

// Types imported for the realm_builder library.
using component_testing::ChildRef;
using component_testing::LocalComponent;
using component_testing::LocalComponentHandles;
using component_testing::LocalComponentImpl;
using component_testing::ParentRef;
using component_testing::Protocol;
using component_testing::RealmBuilder;
Expand Down Expand Up @@ -76,7 +75,7 @@ constexpr auto kTestUIStackUrl =
/// additions of characters, not just the end result.
class KeyboardInputListenerServer
: public fuchsia::ui::test::input::KeyboardInputListener,
public LocalComponent {
public LocalComponentImpl {
public:
explicit KeyboardInputListenerServer(async_dispatcher_t* dispatcher)
: dispatcher_(dispatcher) {}
Expand All @@ -90,10 +89,9 @@ class KeyboardInputListenerServer
}

/// Starts this server.
void Start(std::unique_ptr<LocalComponentHandles> local_handles) override {
void OnStart() override {
FML_LOG(INFO) << "Starting KeyboardInputListenerServer";
local_handles_ = std::move(local_handles);
ASSERT_EQ(ZX_OK, local_handles_->outgoing()->AddPublicService(
ASSERT_EQ(ZX_OK, outgoing()->AddPublicService(
bindings_.GetHandler(this, dispatcher_)));
}

Expand Down Expand Up @@ -124,7 +122,6 @@ class KeyboardInputListenerServer

private:
async_dispatcher_t* dispatcher_ = nullptr;
std::unique_ptr<LocalComponentHandles> local_handles_;
fidl::BindingSet<fuchsia::ui::test::input::KeyboardInputListener> bindings_;
std::vector<std::string> response_list_;
bool ready_ = false;
Expand Down Expand Up @@ -169,18 +166,22 @@ class TextInputTest : public PortableUITest,

std::string GetTestUIStackUrl() override { return GetParam(); };

std::unique_ptr<KeyboardInputListenerServer> keyboard_input_listener_server_;
KeyboardInputListenerServer* keyboard_input_listener_server_;

private:
void ExtendRealm() override {
FML_LOG(INFO) << "Extending realm";
// Key part of service setup: have this test component vend the
// |KeyboardInputListener| service in the constructed realm.
keyboard_input_listener_server_ =
auto keyboard_input_listener_server =
std::make_unique<KeyboardInputListenerServer>(dispatcher());

realm_builder()->AddLocalChild(kKeyboardInputListener,
keyboard_input_listener_server_.get());
keyboard_input_listener_server_ = keyboard_input_listener_server.get();
realm_builder()->AddLocalChild(
kKeyboardInputListener,
[keyboard_input_listener_server =
std::move(keyboard_input_listener_server)]() mutable {
return std::move(keyboard_input_listener_server);
});

// Add text-input-view to the Realm
realm_builder()->AddChild(kTextInputView, kTextInputViewUrl,
Expand Down
39 changes: 22 additions & 17 deletions shell/platform/fuchsia/flutter/tests/integration/touch-input/touch-input-test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,7 @@ namespace {
using component_testing::ChildRef;
using component_testing::ConfigValue;
using component_testing::DirectoryContents;
using component_testing::LocalComponent;
using component_testing::LocalComponentHandles;
using component_testing::LocalComponentImpl;
using component_testing::ParentRef;
using component_testing::Protocol;
using component_testing::Realm;
Expand Down Expand Up @@ -151,17 +150,17 @@ bool CompareDouble(double f0, double f1, double epsilon) {
}

// This component implements the TouchInput protocol
// and the interface for a RealmBuilder LocalComponent. A LocalComponent
// and the interface for a RealmBuilder LocalComponentImpl. A LocalComponentImpl
// is a component that is implemented here in the test, as opposed to
// elsewhere in the system. When it's inserted to the realm, it will act
// like a proper component. This is accomplished, in part, because the
// realm_builder library creates the necessary plumbing. It creates a manifest
// for the component and routes all capabilities to and from it.
// LocalComponent:
// LocalComponentImpl:
// https://fuchsia.dev/fuchsia-src/development/testing/components/realm_builder#mock-components
class TouchInputListenerServer
: public fuchsia::ui::test::input::TouchInputListener,
public LocalComponent {
public LocalComponentImpl {
public:
explicit TouchInputListenerServer(async_dispatcher_t* dispatcher)
: dispatcher_(dispatcher) {}
Expand All @@ -174,21 +173,20 @@ class TouchInputListenerServer
events_received_.push_back(std::move(request));
}

// |LocalComponent::Start|
// |LocalComponentImpl::OnStart|
// When the component framework requests for this component to start, this
// method will be invoked by the realm_builder library.
void Start(std::unique_ptr<LocalComponentHandles> local_handles) override {
void OnStart() override {
FML_LOG(INFO) << "Starting TouchInputListenerServer";
// When this component starts, add a binding to the
// protocol to this component's outgoing directory.
ASSERT_EQ(ZX_OK, local_handles->outgoing()->AddPublicService(
ASSERT_EQ(ZX_OK, outgoing()->AddPublicService(
fidl::InterfaceRequestHandler<
fuchsia::ui::test::input::TouchInputListener>(
[this](auto request) {
bindings_.AddBinding(this, std::move(request),
dispatcher_);
})));
local_handles_.emplace_back(std::move(local_handles));
}

const std::vector<
Expand All @@ -199,7 +197,6 @@ class TouchInputListenerServer

private:
async_dispatcher_t* dispatcher_ = nullptr;
std::vector<std::unique_ptr<LocalComponentHandles>> local_handles_;
fidl::BindingSet<fuchsia::ui::test::input::TouchInputListener> bindings_;
std::vector<
fuchsia::ui::test::input::TouchInputListenerReportTouchInputRequest>
Expand Down Expand Up @@ -290,7 +287,7 @@ class FlutterTapTestBase : public PortableUITest,

ParamType GetTestUIStackUrl() override { return GetParam(); };

std::unique_ptr<TouchInputListenerServer> touch_input_listener_server_;
TouchInputListenerServer* touch_input_listener_server_;
};

class FlutterTapTest : public FlutterTapTestBase {
Expand All @@ -299,10 +296,14 @@ class FlutterTapTest : public FlutterTapTestBase {
FML_LOG(INFO) << "Extending realm";
// Key part of service setup: have this test component vend the
// |TouchInputListener| service in the constructed realm.
touch_input_listener_server_ =
auto touch_input_listener_server =
std::make_unique<TouchInputListenerServer>(dispatcher());
realm_builder()->AddLocalChild(kMockTouchInputListener,
touch_input_listener_server_.get());
touch_input_listener_server_ = touch_input_listener_server.get();
realm_builder()->AddLocalChild(
kMockTouchInputListener, [touch_input_listener_server = std::move(
touch_input_listener_server)]() mutable {
return std::move(touch_input_listener_server);
});

// Add touch-input-view to the Realm
realm_builder()->AddChild(kTouchInputView, kTouchInputViewUrl,
Expand Down Expand Up @@ -381,10 +382,14 @@ class FlutterEmbedTapTest : public FlutterTapTestBase {
FML_LOG(INFO) << "Extending realm";
// Key part of service setup: have this test component vend the
// |TouchInputListener| service in the constructed realm.
touch_input_listener_server_ =
auto touch_input_listener_server =
std::make_unique<TouchInputListenerServer>(dispatcher());
realm_builder()->AddLocalChild(kMockTouchInputListener,
touch_input_listener_server_.get());
touch_input_listener_server_ = touch_input_listener_server.get();
realm_builder()->AddLocalChild(
kMockTouchInputListener, [touch_input_listener_server = std::move(
touch_input_listener_server)]() mutable {
return std::move(touch_input_listener_server);
});

// Add touch-input-view to the Realm
realm_builder()->AddChild(kTouchInputView, kTouchInputViewUrl,
Expand Down

0 comments on commit 6abf987

Please sign in to comment.