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
2 changes: 1 addition & 1 deletion ci/bin/lint.dart
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ void main(List<String> arguments) async {
final ProcessPool pool = ProcessPool();

await for (final WorkerJob job in pool.startWorkers(jobs)) {
if (job.result.stdout.isEmpty) {
if (job.result?.stdout.isEmpty ?? true) {
continue;
}
print('❌ Failures for ${job.name}:');
Expand Down
1 change: 1 addition & 0 deletions ci/lint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ fi

cd "$CI_DIR"
pub get && dart \
--disable-dart-dev \
bin/lint.dart \
--compile-commands="$COMPILE_COMMANDS" \
--repo="$SRC_DIR/flutter" \
Expand Down
15 changes: 10 additions & 5 deletions runtime/dart_isolate.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// 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.
// FLUTTER_NOLINT

#include "flutter/runtime/dart_isolate.h"

Expand Down Expand Up @@ -44,7 +43,7 @@ class DartErrorString {
}
char** error() { return &str_; }
const char* str() const { return str_; }
operator bool() const { return str_ != nullptr; }
explicit operator bool() const { return str_ != nullptr; }

private:
FML_DISALLOW_COPY_AND_ASSIGN(DartErrorString);
Expand Down Expand Up @@ -384,7 +383,7 @@ bool DartIsolate::LoadKernel(std::shared_ptr<const fml::Mapping> mapping,
if (GetIsolateGroupData().GetChildIsolatePreparer() == nullptr) {
GetIsolateGroupData().SetChildIsolatePreparer(
[buffers = kernel_buffers_](DartIsolate* isolate) {
for (unsigned long i = 0; i < buffers.size(); i++) {
for (uint64_t i = 0; i < buffers.size(); i++) {
bool last_piece = i + 1 == buffers.size();
const std::shared_ptr<const fml::Mapping>& buffer = buffers.at(i);
if (!isolate->PrepareForRunningFromKernel(buffer, last_piece)) {
Expand Down Expand Up @@ -675,6 +674,10 @@ Dart_Isolate DartIsolate::DartIsolateGroupCreateCallback(
);
}

if (!parent_isolate_data) {
return nullptr;
}

DartIsolateGroupData& parent_group_data =
(*parent_isolate_data)->GetIsolateGroupData();

Expand All @@ -689,7 +692,8 @@ Dart_Isolate DartIsolate::DartIsolateGroupCreateCallback(
parent_group_data.GetIsolateShutdownCallback())));

TaskRunners null_task_runners(advisory_script_uri,
/* platform= */ nullptr, /* raster= */ nullptr,
/* platform= */ nullptr,
/* raster= */ nullptr,
Comment on lines +695 to +696
Copy link
Member

Choose a reason for hiding this comment

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

The C++ style is to not have the spaces between the comments: https://google.github.io/styleguide/cppguide.html#Function_Argument_Comments

I'm not sure if the linter is smart enough to handle the spaces.

Copy link
Member Author

Choose a reason for hiding this comment

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

I hope it is, because the formatter is adding them =)

/* ui= */ nullptr,
/* io= */ nullptr);

Expand Down Expand Up @@ -731,7 +735,8 @@ bool DartIsolate::DartIsolateInitializeCallback(void** child_callback_data,
Dart_CurrentIsolateGroupData());

TaskRunners null_task_runners((*isolate_group_data)->GetAdvisoryScriptURI(),
/* platform= */ nullptr, /* raster= */ nullptr,
/* platform= */ nullptr,
/* raster= */ nullptr,
/* ui= */ nullptr,
/* io= */ nullptr);

Expand Down
89 changes: 59 additions & 30 deletions runtime/dart_isolate_unittests.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// 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.
// FLUTTER_NOLINT

#include "flutter/fml/mapping.h"
#include "flutter/fml/synchronization/count_down_latch.h"
Expand All @@ -19,7 +18,19 @@
namespace flutter {
namespace testing {

using DartIsolateTest = FixtureTest;
class DartIsolateTest : public FixtureTest {
public:
DartIsolateTest() {}

void Wait() { latch_.Wait(); }

void Signal() { latch_.Signal(); }

private:
fml::AutoResetWaitableEvent latch_;

FML_DISALLOW_COPY_AND_ASSIGN(DartIsolateTest);
};

TEST_F(DartIsolateTest, RootIsolateCreationAndShutdown) {
ASSERT_FALSE(DartVMRef::IsInstanceRunning());
Expand Down Expand Up @@ -153,11 +164,10 @@ TEST_F(DartIsolateTest, CanRunDartCodeCodeSynchronously) {

TEST_F(DartIsolateTest, CanRegisterNativeCallback) {
ASSERT_FALSE(DartVMRef::IsInstanceRunning());
fml::AutoResetWaitableEvent latch;
AddNativeCallback("NotifyNative",
CREATE_NATIVE_ENTRY(([&latch](Dart_NativeArguments args) {
CREATE_NATIVE_ENTRY(([this](Dart_NativeArguments args) {
FML_LOG(ERROR) << "Hello from Dart!";
latch.Signal();
Signal();
})));
const auto settings = CreateSettingsForFixture();
auto vm_ref = DartVMRef::Create(settings);
Expand All @@ -173,7 +183,7 @@ TEST_F(DartIsolateTest, CanRegisterNativeCallback) {
"canRegisterNativeCallback", {}, GetFixturesPath());
ASSERT_TRUE(isolate);
ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running);
latch.Wait();
Wait();
}

TEST_F(DartIsolateTest, CanSaveCompilationTrace) {
Expand All @@ -182,12 +192,11 @@ TEST_F(DartIsolateTest, CanSaveCompilationTrace) {
GTEST_SKIP();
return;
}
fml::AutoResetWaitableEvent latch;
AddNativeCallback("NotifyNative",
CREATE_NATIVE_ENTRY(([&latch](Dart_NativeArguments args) {
CREATE_NATIVE_ENTRY(([this](Dart_NativeArguments args) {
ASSERT_TRUE(tonic::DartConverter<bool>::FromDart(
Dart_GetNativeArgument(args, 0)));
latch.Signal();
Signal();
})));

const auto settings = CreateSettingsForFixture();
Expand All @@ -205,31 +214,52 @@ TEST_F(DartIsolateTest, CanSaveCompilationTrace) {
ASSERT_TRUE(isolate);
ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running);

latch.Wait();
Wait();
}

TEST_F(DartIsolateTest, CanLaunchSecondaryIsolates) {
fml::CountDownLatch latch(3);
fml::AutoResetWaitableEvent child_shutdown_latch;
fml::AutoResetWaitableEvent root_isolate_shutdown_latch;
class DartSecondaryIsolateTest : public FixtureTest {
public:
DartSecondaryIsolateTest() : latch_(3) {}

void LatchCountDown() { latch_.CountDown(); }

void LatchWait() { latch_.Wait(); }

void ChildShutdownSignal() { child_shutdown_latch_.Signal(); }

void ChildShutdownWait() { child_shutdown_latch_.Wait(); }

void RootIsolateShutdownSignal() { root_isolate_shutdown_latch_.Signal(); }

bool RootIsolateIsSignaled() {
return root_isolate_shutdown_latch_.IsSignaledForTest();
}

private:
fml::CountDownLatch latch_;
fml::AutoResetWaitableEvent child_shutdown_latch_;
fml::AutoResetWaitableEvent root_isolate_shutdown_latch_;

FML_DISALLOW_COPY_AND_ASSIGN(DartSecondaryIsolateTest);
};

TEST_F(DartSecondaryIsolateTest, CanLaunchSecondaryIsolates) {
AddNativeCallback("NotifyNative",
CREATE_NATIVE_ENTRY(([&latch](Dart_NativeArguments args) {
latch.CountDown();
CREATE_NATIVE_ENTRY(([this](Dart_NativeArguments args) {
LatchCountDown();
})));
AddNativeCallback(
"PassMessage", CREATE_NATIVE_ENTRY(([&latch](Dart_NativeArguments args) {
"PassMessage", CREATE_NATIVE_ENTRY(([this](Dart_NativeArguments args) {
auto message = tonic::DartConverter<std::string>::FromDart(
Dart_GetNativeArgument(args, 0));
ASSERT_EQ("Hello from code is secondary isolate.", message);
latch.CountDown();
LatchCountDown();
})));
auto settings = CreateSettingsForFixture();
settings.root_isolate_shutdown_callback = [&root_isolate_shutdown_latch]() {
root_isolate_shutdown_latch.Signal();
};
settings.isolate_shutdown_callback = [&child_shutdown_latch]() {
child_shutdown_latch.Signal();
settings.root_isolate_shutdown_callback = [this]() {
RootIsolateShutdownSignal();
};
settings.isolate_shutdown_callback = [this]() { ChildShutdownSignal(); };
auto vm_ref = DartVMRef::Create(settings);
auto thread = CreateNewThread();
TaskRunners task_runners(GetCurrentTestName(), //
Expand All @@ -243,19 +273,18 @@ TEST_F(DartIsolateTest, CanLaunchSecondaryIsolates) {
GetFixturesPath());
ASSERT_TRUE(isolate);
ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running);
child_shutdown_latch.Wait(); // wait for child isolate to shutdown first
ASSERT_FALSE(root_isolate_shutdown_latch.IsSignaledForTest());
latch.Wait(); // wait for last NotifyNative called by main isolate
ChildShutdownWait(); // wait for child isolate to shutdown first
ASSERT_FALSE(RootIsolateIsSignaled());
LatchWait(); // wait for last NotifyNative called by main isolate
// root isolate will be auto-shutdown
}

TEST_F(DartIsolateTest, CanRecieveArguments) {
fml::AutoResetWaitableEvent latch;
AddNativeCallback("NotifyNative",
CREATE_NATIVE_ENTRY(([&latch](Dart_NativeArguments args) {
CREATE_NATIVE_ENTRY(([this](Dart_NativeArguments args) {
ASSERT_TRUE(tonic::DartConverter<bool>::FromDart(
Dart_GetNativeArgument(args, 0)));
latch.Signal();
Signal();
})));

const auto settings = CreateSettingsForFixture();
Expand All @@ -273,7 +302,7 @@ TEST_F(DartIsolateTest, CanRecieveArguments) {
ASSERT_TRUE(isolate);
ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running);

latch.Wait();
Wait();
}

} // namespace testing
Expand Down
2 changes: 1 addition & 1 deletion runtime/dart_service_isolate.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// 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.
// FLUTTER_NOLINT

#include "flutter/runtime/dart_service_isolate.h"

Expand Down Expand Up @@ -202,6 +201,7 @@ bool DartServiceIsolate::Startup(std::string server_ip,
result = Dart_SetField(
library, Dart_NewStringFromCString("_enableServicePortFallback"),
Dart_NewBoolean(enable_service_port_fallback));
SHUTDOWN_ON_ERROR(result);
return true;
}

Expand Down
10 changes: 6 additions & 4 deletions runtime/dart_vm.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// 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.
// FLUTTER_NOLINT

#include "flutter/runtime/dart_vm.h"

Expand Down Expand Up @@ -129,13 +128,15 @@ bool DartFileModifiedCallback(const char* source_url, int64_t since_ms) {

const char* path = source_url + kFileUriPrefixLength;
struct stat info;
if (stat(path, &info) < 0)
if (stat(path, &info) < 0) {
return true;
}

// If st_mtime is zero, it's more likely that the file system doesn't support
// mtime than that the file was actually modified in the 1970s.
if (!info.st_mtime)
if (!info.st_mtime) {
return true;
}

// It's very unclear what time bases we're with here. The Dart API doesn't
// document the time base for since_ms. Reading the code, the value varies by
Expand Down Expand Up @@ -383,8 +384,9 @@ DartVM::DartVM(std::shared_ptr<const DartVMData> vm_data,
PushBackAll(&args, kDartTraceStreamsArgs, fml::size(kDartTraceStreamsArgs));
#endif

for (size_t i = 0; i < settings_.dart_flags.size(); i++)
for (size_t i = 0; i < settings_.dart_flags.size(); i++) {
args.push_back(settings_.dart_flags[i].c_str());
}

char* flags_error = Dart_SetVMFlags(args.size(), args.data());
if (flags_error) {
Expand Down
Loading