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

[fuchsia] Create CF v2 Flutter runner. #29142

Merged
merged 2 commits into from
Oct 14, 2021
Merged

[fuchsia] Create CF v2 Flutter runner. #29142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
45a11a7
[fuchsia] Create CF v2 Flutter runner.
akbiggs Oct 8, 2021
0fc678e
[fuchsia] Address review feedback.
akbiggs Oct 14, 2021
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
8 changes: 5 additions & 3 deletions ci/licenses_golden/licenses_flutter
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1413,9 +1413,11 @@ FILE: ../../../flutter/shell/platform/fuchsia/dart_runner/vmservice/meta/vmservi
FILE: ../../../flutter/shell/platform/fuchsia/flutter/accessibility_bridge.cc
FILE: ../../../flutter/shell/platform/fuchsia/flutter/accessibility_bridge.h
FILE: ../../../flutter/shell/platform/fuchsia/flutter/accessibility_bridge_unittest.cc
FILE: ../../../flutter/shell/platform/fuchsia/flutter/component.cc
FILE: ../../../flutter/shell/platform/fuchsia/flutter/component.h
FILE: ../../../flutter/shell/platform/fuchsia/flutter/component_unittest.cc
FILE: ../../../flutter/shell/platform/fuchsia/flutter/component_v1.cc
FILE: ../../../flutter/shell/platform/fuchsia/flutter/component_v1.h
FILE: ../../../flutter/shell/platform/fuchsia/flutter/component_v1_unittest.cc
FILE: ../../../flutter/shell/platform/fuchsia/flutter/component_v2.cc
FILE: ../../../flutter/shell/platform/fuchsia/flutter/component_v2.h
FILE: ../../../flutter/shell/platform/fuchsia/flutter/engine.cc
FILE: ../../../flutter/shell/platform/fuchsia/flutter/engine.h
FILE: ../../../flutter/shell/platform/fuchsia/flutter/file_in_namespace_buffer.cc
Expand Down
7 changes: 7 additions & 0 deletions shell/platform/fuchsia/dart_runner/dart_component_controller_v2.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,13 @@ void DartComponentControllerV2::Run() {
loop_->Run();

if (binding_.is_bound()) {
// TODO(fxb/79871): This is likely a bug. We're taking the return_code
Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, v2 doesn't have a good way to communicate the return code like a v1 component does. This is something we will need for other things outside of the runner so we should work with the CF team to figure out a plan. I think there will likely be some other lifecycle type service we need to start interacting with.

In the meantime. Can we just do a ZX_ERR_INTERNAL if the return code is not zero and ZX_OK otherwise?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, will handle in a follow-up PR.

Copy link

Choose a reason for hiding this comment

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

This is a feature request that has come up before, it and sounds reasonable. We could probably have the ComponentController dispatch a FIDL event with the return code. In the meantime, converting to INTERNAL SGTM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opened fxb/86666 for handling that in CF. Thanks for the feedback Gary.

// of the process (a uint32_t) and implicitly converting it into a
// zx_status_t that is used as the epitaph. The uint32_t return code is
// unlikely to correspond to the epitaph status that is expected to close
// the connection (with the exception of 0 == ZX_OK). For the documentation
// of what epitaph status we should choose, see
// https://cs.opensource.google/fuchsia/fuchsia/+/main:sdk/fidl/fuchsia.component.runner/component_runner.fidl;l=118;drc=e3b39f2b57e720770773b857feca4f770ee0619e
binding_.Close(return_code_);
}
}
Expand Down
9 changes: 6 additions & 3 deletions shell/platform/fuchsia/flutter/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,10 @@ template("runner_sources") {
sources = [
"accessibility_bridge.cc",
"accessibility_bridge.h",
"component.cc",
"component.h",
"component_v1.cc",
"component_v1.h",
"component_v2.cc",
"component_v2.h",
"engine.cc",
"engine.h",
"file_in_namespace_buffer.cc",
Expand Down Expand Up @@ -138,6 +140,7 @@ template("runner_sources") {

deps = [
"$fuchsia_sdk_root/fidl:fuchsia.accessibility.semantics",
"$fuchsia_sdk_root/fidl:fuchsia.component.runner",
"$fuchsia_sdk_root/fidl:fuchsia.fonts",
"$fuchsia_sdk_root/fidl:fuchsia.images",
"$fuchsia_sdk_root/fidl:fuchsia.intl",
Expand Down Expand Up @@ -465,7 +468,7 @@ executable("flutter_runner_unittests") {

sources = [
"accessibility_bridge_unittest.cc",
"component_unittest.cc",
"component_v1_unittest.cc",
"flutter_runner_fakes.h",
"focus_delegate_unittests.cc",
"fuchsia_intl_unittest.cc",
Expand Down
44 changes: 22 additions & 22 deletions shell/platform/fuchsia/flutter/component.cc → .../platform/fuchsia/flutter/component_v1.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "component.h"
#include "component_v1.h"

#include <dlfcn.h>
#include <fuchsia/mem/cpp/fidl.h>
Expand Down Expand Up @@ -59,7 +59,7 @@ std::string DebugLabelForUrl(const std::string& url) {

} // namespace

void Component::ParseProgramMetadata(
void ComponentV1::ParseProgramMetadata(
const fidl::VectorPtr<fuchsia::sys::ProgramMetadata>& program_metadata,
std::string* data_path,
std::string* assets_path) {
Expand All @@ -80,21 +80,21 @@ void Component::ParseProgramMetadata(
}
}

ActiveComponent Component::Create(
ActiveComponentV1 ComponentV1::Create(
TerminationCallback termination_callback,
fuchsia::sys::Package package,
fuchsia::sys::StartupInfo startup_info,
std::shared_ptr<sys::ServiceDirectory> runner_incoming_services,
fidl::InterfaceRequest<fuchsia::sys::ComponentController> controller) {
auto thread = std::make_unique<fml::Thread>();
std::unique_ptr<Component> component;
std::unique_ptr<ComponentV1> component;

fml::AutoResetWaitableEvent latch;
thread->GetTaskRunner()->PostTask([&]() mutable {
component.reset(new Component(std::move(termination_callback),
std::move(package), std::move(startup_info),
runner_incoming_services,
std::move(controller)));
component.reset(new ComponentV1(std::move(termination_callback),
std::move(package), std::move(startup_info),
runner_incoming_services,
std::move(controller)));
latch.Signal();
});

Expand All @@ -103,7 +103,7 @@ ActiveComponent Component::Create(
.component = std::move(component)};
}

Component::Component(
ComponentV1::ComponentV1(
TerminationCallback termination_callback,
fuchsia::sys::Package package,
fuchsia::sys::StartupInfo startup_info,
Expand Down Expand Up @@ -215,9 +215,9 @@ Component::Component(
[this](zx_status_t status, std::unique_ptr<fuchsia::io::NodeInfo> info) {
cloned_directory_ptr_.Unbind();
if (status != ZX_OK) {
FML_LOG(ERROR) << "could not bind out directory for flutter app("
<< debug_label_
<< "): " << zx_status_get_string(status);
FML_LOG(ERROR)
<< "could not bind out directory for flutter component("
<< debug_label_ << "): " << zx_status_get_string(status);
return;
}
const char* other_dirs[] = {"debug", "ctrl", "diagnostics"};
Expand Down Expand Up @@ -433,7 +433,7 @@ Component::Component(
// happening on the UI thread. If the Component dtor and thread
// termination happen (on the platform thread) between the previous
// line and the next line, a crash will occur since we'll be posting
// to a dead thread. See Runner::OnComponentTerminate() in
// to a dead thread. See Runner::OnComponentV1Terminate() in
// runner.cc.
platform_task_runner->PostTask([weak, runner_incoming_services,
component_url, error, stack_trace]() {
Expand All @@ -458,13 +458,13 @@ Component::Component(
};
}

Component::~Component() = default;
ComponentV1::~ComponentV1() = default;

const std::string& Component::GetDebugLabel() const {
const std::string& ComponentV1::GetDebugLabel() const {
return debug_label_;
}

void Component::Kill() {
void ComponentV1::Kill() {
component_controller_.events().OnTerminated(
last_return_code_.second, fuchsia::sys::TerminationReason::EXITED);

Expand All @@ -473,11 +473,11 @@ void Component::Kill() {
// collected.
}

void Component::Detach() {
void ComponentV1::Detach() {
component_controller_.set_error_handler(nullptr);
}

void Component::OnEngineTerminate(const Engine* shell_holder) {
void ComponentV1::OnEngineTerminate(const Engine* shell_holder) {
auto found = std::find_if(shell_holders_.begin(), shell_holders_.end(),
[shell_holder](const auto& holder) {
return holder.get() == shell_holder;
Expand All @@ -504,7 +504,7 @@ void Component::OnEngineTerminate(const Engine* shell_holder) {
}
}

void Component::CreateView(
void ComponentV1::CreateView(
zx::eventpair token,
fidl::InterfaceRequest<fuchsia::sys::ServiceProvider> /*incoming_services*/,
fidl::InterfaceHandle<
Expand All @@ -514,7 +514,7 @@ void Component::CreateView(
std::move(view_ref_pair.view_ref));
}

void Component::CreateViewWithViewRef(
void ComponentV1::CreateViewWithViewRef(
zx::eventpair view_token,
fuchsia::ui::views::ViewRefControl control_ref,
fuchsia::ui::views::ViewRef view_ref) {
Expand Down Expand Up @@ -542,7 +542,7 @@ void Component::CreateViewWithViewRef(
));
}

void Component::CreateView2(fuchsia::ui::app::CreateView2Args view_args) {
void ComponentV1::CreateView2(fuchsia::ui::app::CreateView2Args view_args) {
if (!svc_) {
FML_DLOG(ERROR)
<< "Component incoming services was invalid when attempting to "
Expand All @@ -566,7 +566,7 @@ void Component::CreateView2(fuchsia::ui::app::CreateView2Args view_args) {
}

#if !defined(DART_PRODUCT)
void Component::WriteProfileToTrace() const {
void ComponentV1::WriteProfileToTrace() const {
for (const auto& engine : shell_holders_) {
engine->WriteProfileToTrace();
}
Expand Down
36 changes: 18 additions & 18 deletions shell/platform/fuchsia/flutter/component.h → ...l/platform/fuchsia/flutter/component_v1.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef FLUTTER_SHELL_PLATFORM_FUCHSIA_COMPONENT_H_
#define FLUTTER_SHELL_PLATFORM_FUCHSIA_COMPONENT_H_
#ifndef FLUTTER_SHELL_PLATFORM_FUCHSIA_COMPONENT_V1_H_
#define FLUTTER_SHELL_PLATFORM_FUCHSIA_COMPONENT_V1_H_

#include <array>
#include <memory>
Expand All @@ -30,35 +30,35 @@

namespace flutter_runner {

class Component;
class ComponentV1;

struct ActiveComponent {
struct ActiveComponentV1 {
std::unique_ptr<fml::Thread> platform_thread;
std::unique_ptr<Component> component;
std::unique_ptr<ComponentV1> component;

ActiveComponent& operator=(ActiveComponent&& other) noexcept {
ActiveComponentV1& operator=(ActiveComponentV1&& other) noexcept {
if (this != &other) {
this->platform_thread.reset(other.platform_thread.release());
this->component.reset(other.component.release());
}
return *this;
}

~ActiveComponent() = default;
~ActiveComponentV1() = default;
};

// Represents an instance of a Flutter component that contains one of more
// Represents an instance of a CF v1 Flutter component that contains one or more
// Flutter engine instances.
class Component final : public Engine::Delegate,
public fuchsia::sys::ComponentController,
public fuchsia::ui::app::ViewProvider {
class ComponentV1 final : public Engine::Delegate,
public fuchsia::sys::ComponentController,
public fuchsia::ui::app::ViewProvider {
public:
using TerminationCallback = fit::function<void(const Component*)>;
using TerminationCallback = fit::function<void(const ComponentV1*)>;

// Creates a dedicated thread to run the component and creates the
// component on it. The component can be accessed only on this thread.
// This is a synchronous operation.
static ActiveComponent Create(
static ActiveComponentV1 Create(
TerminationCallback termination_callback,
fuchsia::sys::Package package,
fuchsia::sys::StartupInfo startup_info,
Expand All @@ -67,7 +67,7 @@ class Component final : public Engine::Delegate,

// Must be called on the same thread returned from the create call. The thread
// may be collected after.
~Component();
~ComponentV1();

static void ParseProgramMetadata(
const fidl::VectorPtr<fuchsia::sys::ProgramMetadata>& program_metadata,
Expand Down Expand Up @@ -101,9 +101,9 @@ class Component final : public Engine::Delegate,
fml::RefPtr<flutter::DartSnapshot> isolate_snapshot_;
std::set<std::unique_ptr<Engine>> shell_holders_;
std::pair<bool, uint32_t> last_return_code_;
fml::WeakPtrFactory<Component> weak_factory_;
fml::WeakPtrFactory<ComponentV1> weak_factory_;

Component(
ComponentV1(
TerminationCallback termination_callback,
fuchsia::sys::Package package,
fuchsia::sys::StartupInfo startup_info,
Expand Down Expand Up @@ -134,9 +134,9 @@ class Component final : public Engine::Delegate,
// |flutter::Engine::Delegate|
void OnEngineTerminate(const Engine* holder) override;

FML_DISALLOW_COPY_AND_ASSIGN(Component);
FML_DISALLOW_COPY_AND_ASSIGN(ComponentV1);
};

} // namespace flutter_runner

#endif // FLUTTER_SHELL_PLATFORM_FUCHSIA_COMPONENT_H_
#endif // FLUTTER_SHELL_PLATFORM_FUCHSIA_COMPONENT_V1_H_
12 changes: 6 additions & 6 deletions ...orm/fuchsia/flutter/component_unittest.cc → .../fuchsia/flutter/component_v1_unittest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,36 +2,36 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "flutter/shell/platform/fuchsia/flutter/component.h"
#include "component_v1.h"

#include <gtest/gtest.h>

namespace flutter_runner {
namespace {

TEST(Component, ParseProgramMetadata) {
TEST(ComponentV1, ParseProgramMetadata) {
std::string data_path;
std::string assets_path;

// The ProgramMetadata field may be null. We should parse this as if no
// fields were specified.
Component::ParseProgramMetadata(nullptr, &data_path, &assets_path);
ComponentV1::ParseProgramMetadata(nullptr, &data_path, &assets_path);

EXPECT_EQ(data_path, "");
EXPECT_EQ(assets_path, "");

// The ProgramMetadata field may be empty. Treat this the same as null.
fidl::VectorPtr<fuchsia::sys::ProgramMetadata> program_metadata(size_t{0});

Component::ParseProgramMetadata(program_metadata, &data_path, &assets_path);
ComponentV1::ParseProgramMetadata(program_metadata, &data_path, &assets_path);

EXPECT_EQ(data_path, "");
EXPECT_EQ(assets_path, "");

// The assets_path defaults to the "data" value if unspecified
program_metadata = {{"data", "foobar"}};

Component::ParseProgramMetadata(program_metadata, &data_path, &assets_path);
ComponentV1::ParseProgramMetadata(program_metadata, &data_path, &assets_path);

EXPECT_EQ(data_path, "pkg/foobar");
EXPECT_EQ(assets_path, "pkg/foobar");
Expand All @@ -41,7 +41,7 @@ TEST(Component, ParseProgramMetadata) {

program_metadata = {{"not_data", "foo"}, {"data", "bar"}, {"assets", "baz"}};

Component::ParseProgramMetadata(program_metadata, &data_path, &assets_path);
ComponentV1::ParseProgramMetadata(program_metadata, &data_path, &assets_path);

EXPECT_EQ(data_path, "pkg/bar");
EXPECT_EQ(assets_path, "pkg/baz");
Expand Down
Loading