Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -1318,6 +1318,7 @@ FILE: ../../../flutter/shell/platform/fuchsia/dart-pkg/fuchsia/sdk_ext/fuchsia.h
FILE: ../../../flutter/shell/platform/fuchsia/dart-pkg/zircon/lib/src/handle.dart
FILE: ../../../flutter/shell/platform/fuchsia/dart-pkg/zircon/lib/src/handle_disposition.dart
FILE: ../../../flutter/shell/platform/fuchsia/dart-pkg/zircon/lib/src/handle_waiter.dart
FILE: ../../../flutter/shell/platform/fuchsia/dart-pkg/zircon/lib/src/init.dart
FILE: ../../../flutter/shell/platform/fuchsia/dart-pkg/zircon/lib/src/system.dart
FILE: ../../../flutter/shell/platform/fuchsia/dart-pkg/zircon/lib/zircon.dart
FILE: ../../../flutter/shell/platform/fuchsia/dart-pkg/zircon/sdk_ext/handle.cc
Expand All @@ -1330,6 +1331,9 @@ FILE: ../../../flutter/shell/platform/fuchsia/dart-pkg/zircon/sdk_ext/natives.cc
FILE: ../../../flutter/shell/platform/fuchsia/dart-pkg/zircon/sdk_ext/natives.h
FILE: ../../../flutter/shell/platform/fuchsia/dart-pkg/zircon/sdk_ext/system.cc
FILE: ../../../flutter/shell/platform/fuchsia/dart-pkg/zircon/sdk_ext/system.h
FILE: ../../../flutter/shell/platform/fuchsia/dart-pkg/zircon_ffi/clock.cc
FILE: ../../../flutter/shell/platform/fuchsia/dart-pkg/zircon_ffi/clock.h
FILE: ../../../flutter/shell/platform/fuchsia/dart-pkg/zircon_ffi/lib/zircon_ffi.dart
FILE: ../../../flutter/shell/platform/fuchsia/dart/compiler.dart
FILE: ../../../flutter/shell/platform/fuchsia/dart_runner/builtin_libraries.cc
FILE: ../../../flutter/shell/platform/fuchsia/dart_runner/builtin_libraries.h
Expand Down
1 change: 1 addition & 0 deletions shell/platform/fuchsia/dart-pkg/zircon/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ source_set("zircon") {
"$fuchsia_sdk_root/pkg:async-loop-cpp",
"$fuchsia_sdk_root/pkg:fdio",
"$fuchsia_sdk_root/pkg:zx",
"../zircon_ffi",
"//flutter/fml",
"//flutter/third_party/tonic",
]
Expand Down
27 changes: 27 additions & 0 deletions shell/platform/fuchsia/dart-pkg/zircon/lib/src/init.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// 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.

part of zircon;

final _kLibZirconDartPath = '/pkg/lib/libzircon_ffi.so';

class _Bindings {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not put this logic directly into ZirconFFIBindings and expose an instance getter, then have systen.dart call ZirconFFIBindings.instance? This is how the framework does things and its nice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ZirconFFIBindings is autogenerated using ffigen and I wanted to keep it that way.

static ZirconFFIBindings? _bindings;

@pragma('vm:entry-point')
static ZirconFFIBindings? get() {
// For soft-transition until libzircon_ffi.so rolls into GI.
if (!File(_kLibZirconDartPath).existsSync()) {
return null;
}

if (_bindings == null) {
final _dylib = DynamicLibrary.open(_kLibZirconDartPath);
_bindings = ZirconFFIBindings(_dylib);
}
return _bindings;
}
}

final ZirconFFIBindings? zirconFFIBindings = _Bindings.get();
10 changes: 9 additions & 1 deletion shell/platform/fuchsia/dart-pkg/zircon/lib/src/system.dart
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,15 @@ class System extends NativeFieldWrapperClass1 {
static MapResult vmoMap(Handle vmo) native 'System_VmoMap';

// Time operations.
static int clockGetMonotonic() native 'System_ClockGetMonotonic';
static int clockGetMonotonic() {
Copy link
Contributor

Choose a reason for hiding this comment

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

btw... I don't think we currently have dart unittest/integration test coverage for this method: https://cs.opensource.google/search?q=%22clockGetMonotonic%22&sq=&ss=fuchsia%2Ffuchsia:sdk%2Fdart%2Fzircon%2Ftest%2F

Just wanted to make sure if you've manually checked if this worked since our existing tests don't cover this. 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if (zirconFFIBindings == null) {
return _nativeClockGetMonotonic();
} else {
return zirconFFIBindings!.zircon_dart_clock_get_monotonic();
}
}

static int _nativeClockGetMonotonic() native 'System_ClockGetMonotonic';

// TODO(edcoyne): Remove this, it is required to safely do an API transition across repos.
static int reboot() { return -2; /*ZX_ERR_NOT_SUPPORTED*/ }
Expand Down
4 changes: 4 additions & 0 deletions shell/platform/fuchsia/dart-pkg/zircon/lib/zircon.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,14 @@
library zircon;

import 'dart:convert' show utf8;
import 'dart:ffi';
import 'dart:io';
import 'dart:nativewrappers';
import 'dart:typed_data';
import 'dart:zircon_ffi';

part 'src/handle.dart';
part 'src/handle_disposition.dart';
part 'src/handle_waiter.dart';
part 'src/init.dart';
part 'src/system.dart';
23 changes: 23 additions & 0 deletions shell/platform/fuchsia/dart-pkg/zircon_ffi/BUILD.gn
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# 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.

import("//build/fuchsia/sdk.gni")

config("zircon_ffi_config") {
include_dirs = [ "." ]
}

shared_library("zircon_ffi") {
public_configs = [ ":zircon_ffi_config" ]

sources = [
"clock.cc",
"clock.h",
]

deps = [
"$fuchsia_sdk_root/pkg:zx",
"//third_party/dart/runtime:dart_api",
]
}
11 changes: 11 additions & 0 deletions shell/platform/fuchsia/dart-pkg/zircon_ffi/clock.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// 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 "clock.h"

#include <zircon/syscalls.h>

uint64_t zircon_dart_clock_get_monotonic() {
return zx_clock_get_monotonic();
}
22 changes: 22 additions & 0 deletions shell/platform/fuchsia/dart-pkg/zircon_ffi/clock.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// 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_FUCHSIA_DART_PKG_ZIRCON_SDK_FFI_CLOCK_H_
#define FLUTTER_SHELL_PLATFORM_FUCHSIA_DART_PKG_ZIRCON_SDK_FFI_CLOCK_H_

#include <stdint.h>

#define ZIRCON_FFI_EXPORT __attribute__((visibility("default")))

#ifdef __cplusplus
extern "C" {
#endif

ZIRCON_FFI_EXPORT uint64_t zircon_dart_clock_get_monotonic();

#ifdef __cplusplus
}
#endif

#endif // FLUTTER_SHELL_PLATFORM_FUCHSIA_DART_PKG_ZIRCON_SDK_FFI_CLOCK_H_
36 changes: 36 additions & 0 deletions shell/platform/fuchsia/dart-pkg/zircon_ffi/lib/zircon_ffi.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// AUTO GENERATED FILE, DO NOT EDIT.
//
// Generated by `package:ffigen`.
import 'dart:ffi' as ffi;

/// Bindings for `dart:zircon_ffi`.
class ZirconFFIBindings {
/// Holds the symbol lookup function.
final ffi.Pointer<T> Function<T extends ffi.NativeType>(String symbolName)
_lookup;

/// The symbols are looked up in [dynamicLibrary].
ZirconFFIBindings(ffi.DynamicLibrary dynamicLibrary)
: _lookup = dynamicLibrary.lookup;

/// The symbols are looked up with [lookup].
ZirconFFIBindings.fromLookup(
ffi.Pointer<T> Function<T extends ffi.NativeType>(String symbolName)
lookup)
: _lookup = lookup;

int zircon_dart_clock_get_monotonic() {
return _zircon_dart_clock_get_monotonic();
}

late final _zircon_dart_clock_get_monotonic_ptr =
_lookup<ffi.NativeFunction<_c_zircon_dart_clock_get_monotonic>>(
'zircon_dart_clock_get_monotonic');
late final _dart_zircon_dart_clock_get_monotonic
Copy link
Member

Choose a reason for hiding this comment

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

Why not call zx_clock_get_monotonic directly from DynamicLibrary.process? Creating a separate library with a trampoline doesn't seem to be adding any value. Perhaps you wanted to build on top of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chinmaygarde, yup, I'm planning on building on top of this. Choosing a simple method like clock monotonic was the easiest way for me to test the combination of:

  1. ffigen code generation within the engine repo.
  2. Integration of dart:zircon_ffi with dart:zircon.
  3. Testing this whole bundle in the fuchsia tree. (We do not test this code in the engine repo).

After this change, I wish to move handle_waiter.dart and handle.dart to the FFI model (away from tonic) as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like chinmay's question still has merit here. We will need a more complex trampoline as time goes on for sure, but why put the ZirconFFIBindings code into a separate .so, instead of linking in into the runner binary and using DynamicLibrary.process? I'm probably missing something, but I can only see an upside to that decision because its less artifacts to distribute

Copy link
Member

Choose a reason for hiding this comment

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

You run into symbol visibility issues and the linker or LTO thinking it can get rid of symbols with default visibility because they are not used. Kaushik and I were experimenting with it this morning and having a tough time making resolving symbols that way.

Copy link
Contributor

Choose a reason for hiding this comment

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

I know this is already merged but I have experimented with this before and didn't need to do any extra trampolining by adding this method to the package:zircon instead of dart:zircon in fuchsia.git. The zx_* functions in libzircon get linked in at runtime by the os so you can get at this function by just using DynamicLibrary.process.

_zircon_dart_clock_get_monotonic = _zircon_dart_clock_get_monotonic_ptr
.asFunction<_dart_zircon_dart_clock_get_monotonic>();
}

typedef _c_zircon_dart_clock_get_monotonic = ffi.Uint64 Function();

typedef _dart_zircon_dart_clock_get_monotonic = int Function();
40 changes: 40 additions & 0 deletions shell/platform/fuchsia/dart-pkg/zircon_ffi/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
name: zircon_ffi

environment:
sdk: '>=2.12.0 <3.0.0'

dependencies:
ffi: ^1.0.0

dev_dependencies:
ffigen: ^3.0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

nice! I didn't know this existed. 😀

lints: ^1.0.1

ffigen:
name: ZirconFFIBindings
description: Bindings for `dart:zircon_ffi`.
output: 'lib/zircon_ffi.dart'
headers:
entry-points:
- 'clock.h'
functions:
include:
- 'zircon_dart_.*'
macros:
include:
- nothing
enums:
include:
- nothing
unnamed-enums:
include:
- nothing
globals:
include:
- nothing
structs:
include:
- nothing
dependency-only: opaque
compiler-opts:
- '-I../../../../../../../third_party/dart/runtime'
15 changes: 13 additions & 2 deletions shell/platform/fuchsia/dart_runner/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@ import("//flutter/tools/fuchsia/dart.gni")
import("//flutter/tools/fuchsia/fuchsia_archive.gni")
import("//flutter/tools/fuchsia/fuchsia_libs.gni")

# Loaded via FFI
_common_runner_libs = common_libs + [
{
name = "libzircon_ffi.so"
path = rebase_path("$root_out_dir")
},
]

template("runner") {
assert(defined(invoker.product), "The parameter 'product' must be defined")
assert(defined(invoker.output_name),
Expand Down Expand Up @@ -40,6 +48,9 @@ template("runner") {

defines = extra_defines

# For `libzircon_ffi` see _common_runner_libs.
public_deps = [ "../dart-pkg/zircon_ffi:zircon_ffi" ]

dart_deps = []
if (!invoker.product) {
dart_deps += [
Expand Down Expand Up @@ -151,7 +162,7 @@ template("aot_runner_package") {

cmx_file = rebase_path("meta/dart_aot${product_suffix}_runner.cmx")

libraries = common_libs
libraries = _common_runner_libs

resources = []
if (!invoker.product) {
Expand Down Expand Up @@ -212,7 +223,7 @@ template("jit_runner_package") {
cmx_file = rebase_path("meta/dart_jit${product_suffix}_runner.cmx")
cml_file = rebase_path("meta/dart_jit${product_suffix}_runner.cml")

libraries = common_libs
libraries = _common_runner_libs

resources = [
{
Expand Down
5 changes: 4 additions & 1 deletion shell/platform/fuchsia/dart_runner/kernel/libraries.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"comment:0": "NOTE: THIS FILE IS GENERATED. DO NOT EDIT.",
"comment:1": "Instead modify 'shell/platform/fuchsia/dart_runner/kernel/libraries.yaml' and follow the instructions therein.",
"comment:1": "Instead modify './flutter/shell/platform/fuchsia/dart_runner/kernel/libraries.yaml' and follow the instructions therein.",
"dart_runner": {
"libraries": {
"_builtin": {
Expand Down Expand Up @@ -152,6 +152,9 @@
"zircon": {
"uri": "../../dart-pkg/zircon/lib/zircon.dart"
},
"zircon_ffi": {
"uri": "../../dart-pkg/zircon_ffi/lib/zircon_ffi.dart"
},
"fuchsia": {
"uri": "../../dart-pkg/fuchsia/lib/fuchsia.dart"
},
Expand Down
3 changes: 3 additions & 0 deletions shell/platform/fuchsia/dart_runner/kernel/libraries.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,9 @@ dart_runner:
zircon:
uri: "../../dart-pkg/zircon/lib/zircon.dart"

zircon_ffi:
uri: "../../dart-pkg/zircon_ffi/lib/zircon_ffi.dart"

fuchsia:
uri: "../../dart-pkg/fuchsia/lib/fuchsia.dart"

Expand Down
15 changes: 13 additions & 2 deletions shell/platform/fuchsia/flutter/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,14 @@ runner_sources("flutter_runner_sources_product") {
product = true
}

# Loaded via FFI
_common_runner_libs = common_libs + [
{
name = "libzircon_ffi.so"
path = rebase_path("$root_out_dir")
},
]

# Things that explicitly being excluded:
# 1. Kernel snapshot framework mode.
# 2. Profiler symbols.
Expand Down Expand Up @@ -195,6 +203,9 @@ template("flutter_runner") {
"$fuchsia_sdk_root/pkg:trace-provider-so",
] + extra_deps

# For `libzircon_ffi` see _common_runner_libs.
public_deps = [ "../dart-pkg/zircon_ffi:zircon_ffi" ]

# The flags below are needed so that Dart's CPU profiler can walk the
# C++ stack.
cflags = [ "-fno-omit-frame-pointer" ]
Expand Down Expand Up @@ -331,7 +342,7 @@ template("jit_runner") {
]

_vulkan_icds = []
_libs = common_libs
_libs = _common_runner_libs
if (enable_vulkan_validation_layers) {
_libs += vulkan_validation_libs
_vulkan_icds += vulkan_icds
Expand Down Expand Up @@ -389,7 +400,7 @@ template("aot_runner") {
}

_vulkan_icds = []
_libs = common_libs
_libs = _common_runner_libs
if (enable_vulkan_validation_layers) {
_libs += vulkan_validation_libs
_vulkan_icds += vulkan_icds
Expand Down
5 changes: 4 additions & 1 deletion shell/platform/fuchsia/flutter/kernel/libraries.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"comment:0": "NOTE: THIS FILE IS GENERATED. DO NOT EDIT.",
"comment:1": "Instead modify 'shell/platform/fuchsia/flutter/kernel/libraries.yaml' and follow the instructions therein.",
"comment:1": "Instead modify './flutter/shell/platform/fuchsia/flutter/kernel/libraries.yaml' and follow the instructions therein.",
"flutter_runner": {
"libraries": {
"_builtin": {
Expand Down Expand Up @@ -152,6 +152,9 @@
"zircon": {
"uri": "../../dart-pkg/zircon/lib/zircon.dart"
},
"zircon_ffi": {
"uri": "../../dart-pkg/zircon_ffi/lib/zircon_ffi.dart"
},
"fuchsia": {
"uri": "../../dart-pkg/fuchsia/lib/fuchsia.dart"
},
Expand Down
3 changes: 3 additions & 0 deletions shell/platform/fuchsia/flutter/kernel/libraries.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,9 @@ flutter_runner:
zircon:
uri: "../../dart-pkg/zircon/lib/zircon.dart"

zircon_ffi:
uri: "../../dart-pkg/zircon_ffi/lib/zircon_ffi.dart"

fuchsia:
uri: "../../dart-pkg/fuchsia/lib/fuchsia.dart"

Expand Down
5 changes: 5 additions & 0 deletions tools/fuchsia/build_fuchsia_artifacts.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ def CopyFlutterTesterBinIfExists(source, destination):
destination_base = os.path.join(destination, 'flutter_binaries')
FindFileAndCopyTo('flutter_tester', source_root, destination_base)

def CopyZirconFFILibIfExists(source, destination):
source_root = os.path.join(_out_dir, source)
destination_base = os.path.join(destination, 'flutter_binaries')
FindFileAndCopyTo('libzircon_ffi.so', source_root, destination_base)

def CopyToBucketWithMode(source, destination, aot, product, runner_type):
mode = 'aot' if aot else 'jit'
Expand All @@ -146,6 +150,7 @@ def CopyToBucketWithMode(source, destination, aot, product, runner_type):
CopyPath(patched_sdk_dir, dest_sdk_path)
CopyGenSnapshotIfExists(source_root, destination)
CopyFlutterTesterBinIfExists(source_root, destination)
CopyZirconFFILibIfExists(source_root, destination)


def CopyToBucket(src, dst, product=False):
Expand Down