From 25e4beff0dfe06089a81e3aa96312a2fe6e8d55e Mon Sep 17 00:00:00 2001 From: Zach Anderson Date: Fri, 31 Jul 2020 11:15:52 -0700 Subject: [PATCH] Enable more linting --- ci/bin/lint.dart | 2 +- ci/lint.sh | 1 + runtime/dart_isolate.cc | 15 +++-- runtime/dart_isolate_unittests.cc | 89 +++++++++++++++++++---------- runtime/dart_service_isolate.cc | 2 +- runtime/dart_vm.cc | 10 ++-- runtime/service_protocol.cc | 32 +++++------ runtime/service_protocol.h | 8 +-- shell/common/shell.cc | 94 +++++++++++++++---------------- shell/common/shell.h | 18 +++--- shell/common/shell_test.cc | 4 +- shell/common/shell_test.h | 2 +- shell/common/shell_unittests.cc | 2 +- 13 files changed, 158 insertions(+), 121 deletions(-) diff --git a/ci/bin/lint.dart b/ci/bin/lint.dart index 658388f4e9773..c573ab578c149 100644 --- a/ci/bin/lint.dart +++ b/ci/bin/lint.dart @@ -241,7 +241,7 @@ void main(List 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}:'); diff --git a/ci/lint.sh b/ci/lint.sh index c3292d8e369fe..c926c5998e111 100755 --- a/ci/lint.sh +++ b/ci/lint.sh @@ -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" \ diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index 53459a9415b75..e2a4c1ec35d7f 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -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" @@ -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); @@ -384,7 +383,7 @@ bool DartIsolate::LoadKernel(std::shared_ptr 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& buffer = buffers.at(i); if (!isolate->PrepareForRunningFromKernel(buffer, last_piece)) { @@ -675,6 +674,10 @@ Dart_Isolate DartIsolate::DartIsolateGroupCreateCallback( ); } + if (!parent_isolate_data) { + return nullptr; + } + DartIsolateGroupData& parent_group_data = (*parent_isolate_data)->GetIsolateGroupData(); @@ -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, /* ui= */ nullptr, /* io= */ nullptr); @@ -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); diff --git a/runtime/dart_isolate_unittests.cc b/runtime/dart_isolate_unittests.cc index e751100f071b2..0e9e4cf59521c 100644 --- a/runtime/dart_isolate_unittests.cc +++ b/runtime/dart_isolate_unittests.cc @@ -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" @@ -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()); @@ -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); @@ -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) { @@ -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::FromDart( Dart_GetNativeArgument(args, 0))); - latch.Signal(); + Signal(); }))); const auto settings = CreateSettingsForFixture(); @@ -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::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(), // @@ -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::FromDart( Dart_GetNativeArgument(args, 0))); - latch.Signal(); + Signal(); }))); const auto settings = CreateSettingsForFixture(); @@ -273,7 +302,7 @@ TEST_F(DartIsolateTest, CanRecieveArguments) { ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); - latch.Wait(); + Wait(); } } // namespace testing diff --git a/runtime/dart_service_isolate.cc b/runtime/dart_service_isolate.cc index 9ad95767234da..9c1bbce2b016f 100644 --- a/runtime/dart_service_isolate.cc +++ b/runtime/dart_service_isolate.cc @@ -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" @@ -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; } diff --git a/runtime/dart_vm.cc b/runtime/dart_vm.cc index 47d2149dc4790..1824ea709ce97 100644 --- a/runtime/dart_vm.cc +++ b/runtime/dart_vm.cc @@ -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" @@ -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 @@ -383,8 +384,9 @@ DartVM::DartVM(std::shared_ptr 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) { diff --git a/runtime/service_protocol.cc b/runtime/service_protocol.cc index 331e25e03fc73..b5320f42acb68 100644 --- a/runtime/service_protocol.cc +++ b/runtime/service_protocol.cc @@ -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 #define RAPIDJSON_HAS_STDSTRING 1 @@ -76,8 +75,9 @@ void ServiceProtocol::SetHandlerDescription(Handler* handler, Handler::Description description) { fml::SharedLock lock(*handlers_mutex_); auto it = handlers_.find(handler); - if (it != handlers_.end()) + if (it != handlers_.end()) { it->second.Store(description); + } } void ServiceProtocol::ToggleHooks(bool set) { @@ -90,13 +90,13 @@ void ServiceProtocol::ToggleHooks(bool set) { } } -static void WriteServerErrorResponse(rapidjson::Document& document, +static void WriteServerErrorResponse(rapidjson::Document* document, const char* message) { - document.SetObject(); - document.AddMember("code", -32000, document.GetAllocator()); + document->SetObject(); + document->AddMember("code", -32000, document->GetAllocator()); rapidjson::Value message_value; - message_value.SetString(message, document.GetAllocator()); - document.AddMember("message", message_value, document.GetAllocator()); + message_value.SetString(message, document->GetAllocator()); + document->AddMember("message", message_value, document->GetAllocator()); } bool ServiceProtocol::HandleMessage(const char* method, @@ -123,7 +123,7 @@ bool ServiceProtocol::HandleMessage(const char* method, bool result = HandleMessage(std::string_view{method}, // params, // static_cast(user_data), // - document // + &document // ); rapidjson::StringBuffer buffer; rapidjson::Writer writer(buffer); @@ -141,7 +141,7 @@ bool ServiceProtocol::HandleMessage(const char* method, bool ServiceProtocol::HandleMessage(std::string_view method, const Handler::ServiceProtocolMap& params, ServiceProtocol* service_protocol, - rapidjson::Document& response) { + rapidjson::Document* response) { if (service_protocol == nullptr) { WriteServerErrorResponse(response, "Service protocol unavailable."); return false; @@ -154,7 +154,7 @@ bool ServiceProtocol::HandleMessage(std::string_view method, ServiceProtocol::Handler* handler, std::string_view method, const ServiceProtocol::Handler::ServiceProtocolMap& params, - rapidjson::Document& document) { + rapidjson::Document* document) { FML_DCHECK(handler); fml::AutoResetWaitableEvent latch; bool result = false; @@ -177,7 +177,7 @@ bool ServiceProtocol::HandleMessage(std::string_view method, bool ServiceProtocol::HandleMessage(std::string_view method, const Handler::ServiceProtocolMap& params, - rapidjson::Document& response) const { + rapidjson::Document* response) const { if (method == kListViewsExtensionName) { // So far, this is the only built-in method that does not forward to the // dynamic set of handlers. @@ -254,7 +254,7 @@ void ServiceProtocol::Handler::Description::Write( } bool ServiceProtocol::HandleListViewsMethod( - rapidjson::Document& response) const { + rapidjson::Document* response) const { fml::SharedLock lock(*handlers_mutex_); std::vector> descriptions; for (const auto& handler : handlers_) { @@ -262,11 +262,11 @@ bool ServiceProtocol::HandleListViewsMethod( handler.second.Load()); } - auto& allocator = response.GetAllocator(); + auto& allocator = response->GetAllocator(); // Construct the response objects. - response.SetObject(); - response.AddMember("type", "FlutterViewList", allocator); + response->SetObject(); + response->AddMember("type", "FlutterViewList", allocator); rapidjson::Value viewsList(rapidjson::Type::kArrayType); for (const auto& description : descriptions) { @@ -276,7 +276,7 @@ bool ServiceProtocol::HandleListViewsMethod( viewsList.PushBack(view, allocator); } - response.AddMember("views", viewsList, allocator); + response->AddMember("views", viewsList, allocator); return true; } diff --git a/runtime/service_protocol.h b/runtime/service_protocol.h index c66a836a2c52e..dfc0cd458fbd9 100644 --- a/runtime/service_protocol.h +++ b/runtime/service_protocol.h @@ -56,7 +56,7 @@ class ServiceProtocol { virtual bool HandleServiceProtocolMessage( std::string_view method, // one if the extension names specified above. const ServiceProtocolMap& params, - rapidjson::Document& response) = 0; + rapidjson::Document* response) = 0; }; ServiceProtocol(); @@ -87,12 +87,12 @@ class ServiceProtocol { std::string_view method, const Handler::ServiceProtocolMap& params, ServiceProtocol* service_protocol, - rapidjson::Document& response); + rapidjson::Document* response); [[nodiscard]] bool HandleMessage(std::string_view method, const Handler::ServiceProtocolMap& params, - rapidjson::Document& response) const; + rapidjson::Document* response) const; - [[nodiscard]] bool HandleListViewsMethod(rapidjson::Document& response) const; + [[nodiscard]] bool HandleListViewsMethod(rapidjson::Document* response) const; FML_DISALLOW_COPY_AND_ASSIGN(ServiceProtocol); }; diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 9137ebcd945a8..bbcd00632bfef 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -1230,7 +1230,7 @@ fml::RefPtr Shell::GetServiceProtocolHandlerTaskRunner( bool Shell::HandleServiceProtocolMessage( std::string_view method, // one if the extension names specified above. const ServiceProtocolMap& params, - rapidjson::Document& response) { + rapidjson::Document* response) { auto found = service_protocol_handlers_.find(method); if (found != service_protocol_handlers_.end()) { return found->second.second(params, response); @@ -1247,44 +1247,44 @@ ServiceProtocol::Handler::Description Shell::GetServiceProtocolDescription() }; } -static void ServiceProtocolParameterError(rapidjson::Document& response, +static void ServiceProtocolParameterError(rapidjson::Document* response, std::string error_details) { - auto& allocator = response.GetAllocator(); - response.SetObject(); + auto& allocator = response->GetAllocator(); + response->SetObject(); const int64_t kInvalidParams = -32602; - response.AddMember("code", kInvalidParams, allocator); - response.AddMember("message", "Invalid params", allocator); + response->AddMember("code", kInvalidParams, allocator); + response->AddMember("message", "Invalid params", allocator); { rapidjson::Value details(rapidjson::kObjectType); details.AddMember("details", error_details, allocator); - response.AddMember("data", details, allocator); + response->AddMember("data", details, allocator); } } -static void ServiceProtocolFailureError(rapidjson::Document& response, +static void ServiceProtocolFailureError(rapidjson::Document* response, std::string message) { - auto& allocator = response.GetAllocator(); - response.SetObject(); + auto& allocator = response->GetAllocator(); + response->SetObject(); const int64_t kJsonServerError = -32000; - response.AddMember("code", kJsonServerError, allocator); - response.AddMember("message", message, allocator); + response->AddMember("code", kJsonServerError, allocator); + response->AddMember("message", message, allocator); } // Service protocol handler bool Shell::OnServiceProtocolScreenshot( const ServiceProtocol::Handler::ServiceProtocolMap& params, - rapidjson::Document& response) { + rapidjson::Document* response) { FML_DCHECK(task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread()); auto screenshot = rasterizer_->ScreenshotLastLayerTree( Rasterizer::ScreenshotType::CompressedImage, true); if (screenshot.data) { - response.SetObject(); - auto& allocator = response.GetAllocator(); - response.AddMember("type", "Screenshot", allocator); + response->SetObject(); + auto& allocator = response->GetAllocator(); + response->AddMember("type", "Screenshot", allocator); rapidjson::Value image; image.SetString(static_cast(screenshot.data->data()), screenshot.data->size(), allocator); - response.AddMember("screenshot", image, allocator); + response->AddMember("screenshot", image, allocator); return true; } ServiceProtocolFailureError(response, "Could not capture image screenshot."); @@ -1294,18 +1294,18 @@ bool Shell::OnServiceProtocolScreenshot( // Service protocol handler bool Shell::OnServiceProtocolScreenshotSKP( const ServiceProtocol::Handler::ServiceProtocolMap& params, - rapidjson::Document& response) { + rapidjson::Document* response) { FML_DCHECK(task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread()); auto screenshot = rasterizer_->ScreenshotLastLayerTree( Rasterizer::ScreenshotType::SkiaPicture, true); if (screenshot.data) { - response.SetObject(); - auto& allocator = response.GetAllocator(); - response.AddMember("type", "ScreenshotSkp", allocator); + response->SetObject(); + auto& allocator = response->GetAllocator(); + response->AddMember("type", "ScreenshotSkp", allocator); rapidjson::Value skp; skp.SetString(static_cast(screenshot.data->data()), screenshot.data->size(), allocator); - response.AddMember("skp", skp, allocator); + response->AddMember("skp", skp, allocator); return true; } ServiceProtocolFailureError(response, "Could not capture SKP screenshot."); @@ -1315,7 +1315,7 @@ bool Shell::OnServiceProtocolScreenshotSKP( // Service protocol handler bool Shell::OnServiceProtocolRunInView( const ServiceProtocol::Handler::ServiceProtocolMap& params, - rapidjson::Document& response) { + rapidjson::Document* response) { FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread()); if (params.count("mainScript") == 0) { @@ -1351,14 +1351,14 @@ bool Shell::OnServiceProtocolRunInView( std::make_unique(fml::OpenDirectory( asset_directory_path.c_str(), false, fml::FilePermission::kRead))); - auto& allocator = response.GetAllocator(); - response.SetObject(); + auto& allocator = response->GetAllocator(); + response->SetObject(); if (engine_->Restart(std::move(configuration))) { - response.AddMember("type", "Success", allocator); + response->AddMember("type", "Success", allocator); auto new_description = GetServiceProtocolDescription(); rapidjson::Value view(rapidjson::kObjectType); new_description.Write(this, view, allocator); - response.AddMember("view", view, allocator); + response->AddMember("view", view, allocator); return true; } else { FML_DLOG(ERROR) << "Could not run configuration in engine."; @@ -1374,7 +1374,7 @@ bool Shell::OnServiceProtocolRunInView( // Service protocol handler bool Shell::OnServiceProtocolFlushUIThreadTasks( const ServiceProtocol::Handler::ServiceProtocolMap& params, - rapidjson::Document& response) { + rapidjson::Document* response) { FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread()); // This API should not be invoked by production code. // It can potentially starve the service isolate if the main isolate pauses @@ -1382,28 +1382,28 @@ bool Shell::OnServiceProtocolFlushUIThreadTasks( // // It should be invoked from the VM Service and and blocks it until UI thread // tasks are processed. - response.SetObject(); - response.AddMember("type", "Success", response.GetAllocator()); + response->SetObject(); + response->AddMember("type", "Success", response->GetAllocator()); return true; } bool Shell::OnServiceProtocolGetDisplayRefreshRate( const ServiceProtocol::Handler::ServiceProtocolMap& params, - rapidjson::Document& response) { + rapidjson::Document* response) { FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread()); - response.SetObject(); - response.AddMember("type", "DisplayRefreshRate", response.GetAllocator()); - response.AddMember("fps", engine_->GetDisplayRefreshRate(), - response.GetAllocator()); + response->SetObject(); + response->AddMember("type", "DisplayRefreshRate", response->GetAllocator()); + response->AddMember("fps", engine_->GetDisplayRefreshRate(), + response->GetAllocator()); return true; } bool Shell::OnServiceProtocolGetSkSLs( const ServiceProtocol::Handler::ServiceProtocolMap& params, - rapidjson::Document& response) { + rapidjson::Document* response) { FML_DCHECK(task_runners_.GetIOTaskRunner()->RunsTasksOnCurrentThread()); - response.SetObject(); - response.AddMember("type", "GetSkSLs", response.GetAllocator()); + response->SetObject(); + response->AddMember("type", "GetSkSLs", response->GetAllocator()); rapidjson::Value shaders_json(rapidjson::kObjectType); PersistentCache* persistent_cache = PersistentCache::GetCacheForProcess(); @@ -1415,19 +1415,19 @@ bool Shell::OnServiceProtocolGetSkSLs( char* b64_char = static_cast(b64_data->writable_data()); SkBase64::Encode(sksl.second->data(), sksl.second->size(), b64_char); b64_char[b64_size] = 0; // make it null terminated for printing - rapidjson::Value shader_value(b64_char, response.GetAllocator()); + rapidjson::Value shader_value(b64_char, response->GetAllocator()); rapidjson::Value shader_key(PersistentCache::SkKeyToFilePath(*sksl.first), - response.GetAllocator()); - shaders_json.AddMember(shader_key, shader_value, response.GetAllocator()); + response->GetAllocator()); + shaders_json.AddMember(shader_key, shader_value, response->GetAllocator()); } - response.AddMember("SkSLs", shaders_json, response.GetAllocator()); + response->AddMember("SkSLs", shaders_json, response->GetAllocator()); return true; } // Service protocol handler bool Shell::OnServiceProtocolSetAssetBundlePath( const ServiceProtocol::Handler::ServiceProtocolMap& params, - rapidjson::Document& response) { + rapidjson::Document* response) { FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread()); if (params.count("assetDirectory") == 0) { @@ -1436,8 +1436,8 @@ bool Shell::OnServiceProtocolSetAssetBundlePath( return false; } - auto& allocator = response.GetAllocator(); - response.SetObject(); + auto& allocator = response->GetAllocator(); + response->SetObject(); auto asset_manager = std::make_shared(); @@ -1446,11 +1446,11 @@ bool Shell::OnServiceProtocolSetAssetBundlePath( fml::FilePermission::kRead))); if (engine_->UpdateAssetManager(std::move(asset_manager))) { - response.AddMember("type", "Success", allocator); + response->AddMember("type", "Success", allocator); auto new_description = GetServiceProtocolDescription(); rapidjson::Value view(rapidjson::kObjectType); new_description.Write(this, view, allocator); - response.AddMember("view", view, allocator); + response->AddMember("view", view, allocator); return true; } else { FML_DLOG(ERROR) << "Could not update asset directory."; diff --git a/shell/common/shell.h b/shell/common/shell.h index fd1a30d3f4e35..29a4320d50e4b 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -365,7 +365,7 @@ class Shell final : public PlatformView::Delegate, private: using ServiceProtocolHandler = std::function; + rapidjson::Document*)>; const TaskRunners task_runners_; const Settings settings_; @@ -541,7 +541,7 @@ class Shell final : public PlatformView::Delegate, bool HandleServiceProtocolMessage( std::string_view method, // one if the extension names specified above. const ServiceProtocolMap& params, - rapidjson::Document& response) override; + rapidjson::Document* response) override; // |ServiceProtocol::Handler| ServiceProtocol::Handler::Description GetServiceProtocolDescription() @@ -550,39 +550,39 @@ class Shell final : public PlatformView::Delegate, // Service protocol handler bool OnServiceProtocolScreenshot( const ServiceProtocol::Handler::ServiceProtocolMap& params, - rapidjson::Document& response); + rapidjson::Document* response); // Service protocol handler bool OnServiceProtocolScreenshotSKP( const ServiceProtocol::Handler::ServiceProtocolMap& params, - rapidjson::Document& response); + rapidjson::Document* response); // Service protocol handler bool OnServiceProtocolRunInView( const ServiceProtocol::Handler::ServiceProtocolMap& params, - rapidjson::Document& response); + rapidjson::Document* response); // Service protocol handler bool OnServiceProtocolFlushUIThreadTasks( const ServiceProtocol::Handler::ServiceProtocolMap& params, - rapidjson::Document& response); + rapidjson::Document* response); // Service protocol handler bool OnServiceProtocolSetAssetBundlePath( const ServiceProtocol::Handler::ServiceProtocolMap& params, - rapidjson::Document& response); + rapidjson::Document* response); // Service protocol handler bool OnServiceProtocolGetDisplayRefreshRate( const ServiceProtocol::Handler::ServiceProtocolMap& params, - rapidjson::Document& response); + rapidjson::Document* response); // Service protocol handler // // The returned SkSLs are base64 encoded. Decode before storing them to files. bool OnServiceProtocolGetSkSLs( const ServiceProtocol::Handler::ServiceProtocolMap& params, - rapidjson::Document& response); + rapidjson::Document* response); fml::WeakPtrFactory weak_factory_; diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc index c8ebf642e056f..f2d2671d343c6 100644 --- a/shell/common/shell_test.cc +++ b/shell/common/shell_test.cc @@ -181,10 +181,10 @@ void ShellTest::OnServiceProtocol( ServiceProtocolEnum some_protocol, fml::RefPtr task_runner, const ServiceProtocol::Handler::ServiceProtocolMap& params, - rapidjson::Document& response) { + rapidjson::Document* response) { std::promise finished; fml::TaskRunner::RunNowOrPostTask( - task_runner, [shell, some_protocol, params, &response, &finished]() { + task_runner, [shell, some_protocol, params, response, &finished]() { switch (some_protocol) { case ServiceProtocolEnum::kGetSkSLs: shell->OnServiceProtocolGetSkSLs(params, response); diff --git a/shell/common/shell_test.h b/shell/common/shell_test.h index 2b46881c3c68f..e296ba118c73e 100644 --- a/shell/common/shell_test.h +++ b/shell/common/shell_test.h @@ -92,7 +92,7 @@ class ShellTest : public FixtureTest { ServiceProtocolEnum some_protocol, fml::RefPtr task_runner, const ServiceProtocol::Handler::ServiceProtocolMap& params, - rapidjson::Document& response); + rapidjson::Document* response); std::shared_ptr GetFontCollection(Shell* shell); diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 5f82ee64b5b50..318977fbed5aa 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -1260,7 +1260,7 @@ TEST_F(ShellTest, OnServiceProtocolGetSkSLsWorks) { rapidjson::Document document; OnServiceProtocol(shell.get(), ServiceProtocolEnum::kGetSkSLs, shell->GetTaskRunners().GetIOTaskRunner(), empty_params, - document); + &document); rapidjson::StringBuffer buffer; rapidjson::Writer writer(buffer); document.Accept(writer);