From 9668ba422f0df9a0e950a2cc71cf378dea4892cd Mon Sep 17 00:00:00 2001 From: Adam Barth Date: Sat, 6 Aug 2016 00:53:43 -0700 Subject: [PATCH] Remove //base from DartController --- flutter/lib/ui/painting/image_decoding.cc | 33 ++++++------- flutter/tonic/dart_library_loader.cc | 14 +++--- flutter/tonic/dart_library_loader.h | 3 +- flutter/tonic/dart_snapshot_loader.cc | 49 +++++++++---------- flutter/tonic/dart_snapshot_loader.h | 17 ++----- glue/BUILD.gn | 4 ++ glue/drain_data_pipe_job.cc | 43 +++++++++++++++++ glue/drain_data_pipe_job.h | 35 ++++++++++++++ glue/trace_event.h | 2 + sky/engine/core/script/dart_controller.cc | 14 ++---- sky/engine/core/script/dart_controller.h | 16 +----- sky/engine/platform/BUILD.gn | 2 - sky/engine/platform/mojo/data_pipe.cc | 59 ----------------------- sky/engine/platform/mojo/data_pipe.h | 20 -------- sky/engine/wtf/BUILD.gn | 9 ++++ 15 files changed, 152 insertions(+), 168 deletions(-) create mode 100644 glue/drain_data_pipe_job.cc create mode 100644 glue/drain_data_pipe_job.h delete mode 100644 sky/engine/platform/mojo/data_pipe.cc delete mode 100644 sky/engine/platform/mojo/data_pipe.h diff --git a/flutter/lib/ui/painting/image_decoding.cc b/flutter/lib/ui/painting/image_decoding.cc index 889bbf6766a..091c8b33996 100644 --- a/flutter/lib/ui/painting/image_decoding.cc +++ b/flutter/lib/ui/painting/image_decoding.cc @@ -6,14 +6,13 @@ #include "flow/texture_image.h" #include "flutter/lib/ui/painting/image.h" +#include "glue/drain_data_pipe_job.h" #include "glue/movable_wrapper.h" #include "glue/trace_event.h" #include "lib/tonic/dart_persistent_value.h" #include "lib/tonic/logging/dart_invoke.h" #include "lib/tonic/mojo_converter.h" #include "lib/tonic/typed_data/uint8_list.h" -#include "sky/engine/platform/mojo/data_pipe.h" -#include "sky/engine/platform/SharedBuffer.h" #include "sky/engine/public/platform/Platform.h" #include "sky/engine/wtf/PassOwnPtr.h" #include "sky/shell/platform_view.h" @@ -26,16 +25,14 @@ using tonic::ToDart; namespace blink { namespace { -sk_sp DecodeImage(PassRefPtr buffer) { +sk_sp DecodeImage(std::vector buffer) { TRACE_EVENT0("blink", "DecodeImage"); - const size_t data_size = buffer->size(); - - if (buffer == nullptr || data_size == 0) { + if (buffer.empty()) { return nullptr; } - sk_sp sk_data = SkData::MakeWithoutCopy(buffer->data(), data_size); + sk_sp sk_data = SkData::MakeWithoutCopy(buffer.data(), buffer.size()); if (sk_data == nullptr) { return nullptr; @@ -77,8 +74,8 @@ void InvokeImageCallback(sk_sp image, } void DecodeImageAndInvokeImageCallback(PassOwnPtr callback, - PassRefPtr buffer) { - sk_sp image = DecodeImage(buffer); + std::vector buffer) { + sk_sp image = DecodeImage(std::move(buffer)); Platform::current()->GetUITaskRunner()->PostTask( [callback, image]() { InvokeImageCallback(image, callback); }); } @@ -105,10 +102,12 @@ void DecodeImageFromDataPipe(Dart_NativeArguments args) { Platform::current()->GetIOTaskRunner()->PostTask( [callback, consumer]() mutable { - DrainDataPipe(consumer.Unwrap(), - [callback](PassRefPtr buffer) { - DecodeImageAndInvokeImageCallback(callback, buffer); - }); + glue::DrainDataPipeJob* job = nullptr; + job = new glue::DrainDataPipeJob( + consumer.Unwrap(), [callback, job](std::vector buffer) { + delete job; + DecodeImageAndInvokeImageCallback(callback, std::move(buffer)); + }); }); } @@ -130,12 +129,12 @@ void DecodeImageFromList(Dart_NativeArguments args) { PassOwnPtr callback = adoptPtr(new DartPersistentValue(DartState::Current(), callback_handle)); - RefPtr buffer = SharedBuffer::create(); - buffer->append(reinterpret_cast(list.data()), - list.num_elements()); + const char* bytes = reinterpret_cast(list.data()); + PassOwnPtr> buffer = + adoptPtr(new std::vector(bytes, bytes + list.num_elements())); Platform::current()->GetIOTaskRunner()->PostTask([callback, buffer]() { - DecodeImageAndInvokeImageCallback(callback, buffer); + DecodeImageAndInvokeImageCallback(callback, std::move(*buffer)); }); } diff --git a/flutter/tonic/dart_library_loader.cc b/flutter/tonic/dart_library_loader.cc index 15ede5dc180..edca795fec6 100644 --- a/flutter/tonic/dart_library_loader.cc +++ b/flutter/tonic/dart_library_loader.cc @@ -134,7 +134,7 @@ class DartLibraryLoader::SourceJob : public Job { class DartLibraryLoader::DependencyWatcher { public: DependencyWatcher(const std::unordered_set& dependencies, - const base::Closure& callback) + const ftl::Closure& callback) : dependencies_(dependencies), callback_(callback) { DCHECK(!dependencies_.empty()); } @@ -151,11 +151,11 @@ class DartLibraryLoader::DependencyWatcher { return dependencies_.empty(); } - const base::Closure& callback() const { return callback_; } + const ftl::Closure& callback() const { return callback_; } private: std::unordered_set dependencies_; - base::Closure callback_; + ftl::Closure callback_; }; // A WatcherSignaler is responsible for signaling DependencyWatchers when their @@ -185,7 +185,7 @@ class DartLibraryLoader::WatcherSignaler { // callbacks before running any of them. We don't want to be re-entered // below the callbacks and end up in an inconsistent state. catcher_ = nullptr; - std::vector callbacks; + std::vector callbacks; for (const auto& watcher : completed_watchers) { callbacks.push_back(watcher->callback()); EraseUniquePtr(loader_.dependency_watchers_, watcher); @@ -193,7 +193,7 @@ class DartLibraryLoader::WatcherSignaler { // Finally, run all the callbacks while touching only data on the stack. for (const auto& callback : callbacks) - callback.Run(); + callback(); } private: @@ -232,9 +232,9 @@ Dart_Handle DartLibraryLoader::HandleLibraryTag(Dart_LibraryTag tag, void DartLibraryLoader::WaitForDependencies( const std::unordered_set& dependencies, - const base::Closure& callback) { + const ftl::Closure& callback) { if (dependencies.empty()) - return callback.Run(); + return callback(); dependency_watchers_.insert(std::unique_ptr( new DependencyWatcher(dependencies, callback))); } diff --git a/flutter/tonic/dart_library_loader.h b/flutter/tonic/dart_library_loader.h index 236d60b0ea3..047598c6743 100644 --- a/flutter/tonic/dart_library_loader.h +++ b/flutter/tonic/dart_library_loader.h @@ -15,6 +15,7 @@ #include "base/macros.h" #include "base/memory/weak_ptr.h" #include "dart/runtime/include/dart_api.h" +#include "lib/ftl/functional/closure.h" namespace blink { class DartDependency; @@ -41,7 +42,7 @@ class DartLibraryLoader { void WaitForDependencies( const std::unordered_set& dependencies, - const base::Closure& callback); + const ftl::Closure& callback); void set_dependency_catcher(DartDependencyCatcher* dependency_catcher) { DCHECK(!dependency_catcher_ || !dependency_catcher); diff --git a/flutter/tonic/dart_snapshot_loader.cc b/flutter/tonic/dart_snapshot_loader.cc index 3628a80165d..b060967965b 100644 --- a/flutter/tonic/dart_snapshot_loader.cc +++ b/flutter/tonic/dart_snapshot_loader.cc @@ -4,15 +4,15 @@ #include "flutter/tonic/dart_snapshot_loader.h" -#include "base/callback.h" -#include "base/trace_event/trace_event.h" -#include "lib/tonic/scopes/dart_api_scope.h" -#include "lib/tonic/logging/dart_error.h" -#include "lib/tonic/scopes/dart_isolate_scope.h" -#include "flutter/tonic/dart_state.h" -#include "lib/tonic/converter/dart_converter.h" +#include + +#include "flutter/tonic/dart_state.h" +#include "glue/trace_event.h" +#include "lib/tonic/converter/dart_converter.h" +#include "lib/tonic/logging/dart_error.h" +#include "lib/tonic/scopes/dart_api_scope.h" +#include "lib/tonic/scopes/dart_isolate_scope.h" -using mojo::common::DataPipeDrainer; using tonic::LogIfError; namespace blink { @@ -23,29 +23,24 @@ DartSnapshotLoader::DartSnapshotLoader(tonic::DartState* dart_state) DartSnapshotLoader::~DartSnapshotLoader() {} void DartSnapshotLoader::LoadSnapshot(mojo::ScopedDataPipeConsumerHandle pipe, - const base::Closure& callback) { + const ftl::Closure& callback) { TRACE_EVENT_ASYNC_BEGIN0("flutter", "DartSnapshotLoader::LoadSnapshot", this); - callback_ = callback; - drainer_.reset(new DataPipeDrainer(this, pipe.Pass())); -} + drain_job_.reset(new glue::DrainDataPipeJob( + std::move(pipe), [this, callback](std::vector buffer) { + TRACE_EVENT_ASYNC_END0("flutter", "DartSnapshotLoader::LoadSnapshot", + this); + // TODO(abarth): Should we check dart_state_ for null? + { + tonic::DartIsolateScope scope(dart_state_->isolate()); + tonic::DartApiScope api_scope; -void DartSnapshotLoader::OnDataAvailable(const void* data, size_t num_bytes) { - const uint8_t* bytes = static_cast(data); - buffer_.insert(buffer_.end(), bytes, bytes + num_bytes); -} + LogIfError(Dart_LoadScriptFromSnapshot( + reinterpret_cast(buffer.data()), buffer.size())); + } -void DartSnapshotLoader::OnDataComplete() { - TRACE_EVENT_ASYNC_END0("flutter", "DartSnapshotLoader::LoadSnapshot", this); - // TODO(abarth): Should we check dart_state_ for null? - { - tonic::DartIsolateScope scope(dart_state_->isolate()); - tonic::DartApiScope api_scope; - - LogIfError(Dart_LoadScriptFromSnapshot(buffer_.data(), buffer_.size())); - } - - callback_.Run(); + callback(); + })); } } // namespace blink diff --git a/flutter/tonic/dart_snapshot_loader.h b/flutter/tonic/dart_snapshot_loader.h index 45bd41087a7..4abad241ddf 100644 --- a/flutter/tonic/dart_snapshot_loader.h +++ b/flutter/tonic/dart_snapshot_loader.h @@ -7,32 +7,25 @@ #include -#include "base/callback_forward.h" #include "dart/runtime/include/dart_api.h" +#include "glue/drain_data_pipe_job.h" +#include "lib/ftl/functional/closure.h" #include "lib/ftl/macros.h" #include "lib/tonic/dart_state.h" -#include "mojo/data_pipe_utils/data_pipe_drainer.h" namespace blink { -class DartSnapshotLoader : public mojo::common::DataPipeDrainer::Client { +class DartSnapshotLoader { public: explicit DartSnapshotLoader(tonic::DartState* dart_state); ~DartSnapshotLoader(); void LoadSnapshot(mojo::ScopedDataPipeConsumerHandle pipe, - const base::Closure& callback); + const ftl::Closure& callback); private: - // mojo::common::DataPipeDrainer::Client - void OnDataAvailable(const void* data, size_t num_bytes) override; - void OnDataComplete() override; - ftl::WeakPtr dart_state_; - std::unique_ptr drainer_; - // TODO(abarth): Should we be using SharedBuffer to buffer the data? - std::vector buffer_; - base::Closure callback_; + std::unique_ptr drain_job_; FTL_DISALLOW_COPY_AND_ASSIGN(DartSnapshotLoader); }; diff --git a/glue/BUILD.gn b/glue/BUILD.gn index 773ae0da538..09dd2fcee75 100644 --- a/glue/BUILD.gn +++ b/glue/BUILD.gn @@ -4,6 +4,8 @@ source_set("glue") { sources = [ + "drain_data_pipe_job.cc", + "drain_data_pipe_job.h", "movable_wrapper.h", "stack_trace.cc", "stack_trace.h", @@ -14,5 +16,7 @@ source_set("glue") { deps = [ "//base", + "//lib/ftl", + "//mojo/data_pipe_utils", ] } diff --git a/glue/drain_data_pipe_job.cc b/glue/drain_data_pipe_job.cc new file mode 100644 index 00000000000..06fe05620d3 --- /dev/null +++ b/glue/drain_data_pipe_job.cc @@ -0,0 +1,43 @@ +// Copyright 2016 The Chromium 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 "glue/drain_data_pipe_job.h" + +#include + +#include "mojo/data_pipe_utils/data_pipe_drainer.h" + +using mojo::common::DataPipeDrainer; + +namespace glue { + +class DrainDataPipeJob::JobImpl : public DataPipeDrainer::Client { + public: + explicit JobImpl(mojo::ScopedDataPipeConsumerHandle handle, + const ResultCallback& callback) + : callback_(callback), drainer_(this, std::move(handle)) {} + + private: + // mojo::common::DataPipeDrainer::Client + void OnDataAvailable(const void* data, size_t num_bytes) override { + const char* bytes = static_cast(data); + buffer_.insert(buffer_.end(), bytes, bytes + num_bytes); + } + + void OnDataComplete() override { callback_(std::move(buffer_)); } + + std::vector buffer_; + ResultCallback callback_; + mojo::common::DataPipeDrainer drainer_; + + FTL_DISALLOW_COPY_AND_ASSIGN(JobImpl); +}; + +DrainDataPipeJob::DrainDataPipeJob(mojo::ScopedDataPipeConsumerHandle handle, + const ResultCallback& callback) + : impl_(new JobImpl(std::move(handle), callback)) {} + +DrainDataPipeJob::~DrainDataPipeJob() {} + +} // namespace glue diff --git a/glue/drain_data_pipe_job.h b/glue/drain_data_pipe_job.h new file mode 100644 index 00000000000..7baa1628ae3 --- /dev/null +++ b/glue/drain_data_pipe_job.h @@ -0,0 +1,35 @@ +// Copyright 2016 The Chromium 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 GLUE_DRAIN_DATA_PIPE_JOB_H_ +#define GLUE_DRAIN_DATA_PIPE_JOB_H_ + +#include +#include +#include + +#include "lib/ftl/macros.h" +#include "mojo/public/cpp/system/data_pipe.h" + +namespace glue { + +class DrainDataPipeJob { + public: + using ResultCallback = std::function)>; + + DrainDataPipeJob(mojo::ScopedDataPipeConsumerHandle handle, + const ResultCallback& callback); + ~DrainDataPipeJob(); + + private: + class JobImpl; + + std::unique_ptr impl_; + + FTL_DISALLOW_COPY_AND_ASSIGN(DrainDataPipeJob); +}; + +} // namespace glue + +#endif // GLUE_DRAIN_DATA_PIPE_JOB_H_ diff --git a/glue/trace_event.h b/glue/trace_event.h index 468d1853495..f06268ff99c 100644 --- a/glue/trace_event.h +++ b/glue/trace_event.h @@ -6,6 +6,8 @@ #define TRACE_EVENT0(a, b) #define TRACE_EVENT1(a, b, c, d) #define TRACE_EVENT2(a, b, c, d, e, f) +#define TRACE_EVENT_ASYNC_BEGIN0(a, b, c) +#define TRACE_EVENT_ASYNC_END0(a, b, c) #else #include "base/trace_event/trace_event.h" #endif // defined(__Fuchsia__) diff --git a/sky/engine/core/script/dart_controller.cc b/sky/engine/core/script/dart_controller.cc index 6f561e808c0..7683f471259 100644 --- a/sky/engine/core/script/dart_controller.cc +++ b/sky/engine/core/script/dart_controller.cc @@ -4,7 +4,6 @@ #include "sky/engine/core/script/dart_controller.h" -#include "base/bind.h" #include "dart/runtime/include/dart_tools_api.h" #include "flutter/tonic/dart_debugger.h" #include "flutter/tonic/dart_dependency_catcher.h" @@ -41,8 +40,7 @@ using tonic::ToDart; namespace blink { -DartController::DartController() - : ui_dart_state_(nullptr), weak_factory_(this) {} +DartController::DartController() : ui_dart_state_(nullptr) {} DartController::~DartController() { if (ui_dart_state_) { @@ -132,9 +130,8 @@ void DartController::RunFromPrecompiledSnapshot() { void DartController::RunFromSnapshot( mojo::ScopedDataPipeConsumerHandle snapshot) { snapshot_loader_ = WTF::MakeUnique(dart_state()); - snapshot_loader_->LoadSnapshot( - snapshot.Pass(), - base::Bind(&DartController::DidLoadSnapshot, weak_factory_.GetWeakPtr())); + snapshot_loader_->LoadSnapshot(snapshot.Pass(), + [this]() { DidLoadSnapshot(); }); } void DartController::RunFromSnapshotBuffer(const uint8_t* buffer, size_t size) { @@ -147,7 +144,7 @@ void DartController::RunFromSnapshotBuffer(const uint8_t* buffer, size_t size) { exit(1); } -void DartController::RunFromLibrary(const std::string& name, +void DartController::RunFromLibrary(std::string name, DartLibraryProvider* library_provider) { DartState::Scope scope(dart_state()); @@ -157,8 +154,7 @@ void DartController::RunFromLibrary(const std::string& name, DartDependencyCatcher dependency_catcher(loader); loader.LoadScript(name); loader.WaitForDependencies(dependency_catcher.dependencies(), - base::Bind(&DartController::DidLoadMainLibrary, - weak_factory_.GetWeakPtr(), name)); + [this, name]() { DidLoadMainLibrary(name); }); } void DartController::CreateIsolateFor(std::unique_ptr state) { diff --git a/sky/engine/core/script/dart_controller.h b/sky/engine/core/script/dart_controller.h index 7c978b3949e..2913955959e 100644 --- a/sky/engine/core/script/dart_controller.h +++ b/sky/engine/core/script/dart_controller.h @@ -7,22 +7,14 @@ #include -#include "base/callback_forward.h" -#include "base/memory/weak_ptr.h" #include "dart/runtime/include/dart_api.h" #include "lib/ftl/macros.h" #include "mojo/public/cpp/system/data_pipe.h" -#include "sky/engine/wtf/OwnPtr.h" -#include "sky/engine/wtf/text/AtomicString.h" -#include "sky/engine/wtf/text/TextPosition.h" namespace blink { -class AbstractModule; -class DartUI; class UIDartState; class DartLibraryProvider; class DartSnapshotLoader; -class View; class DartController { public: @@ -31,8 +23,7 @@ class DartController { static void InitVM(); - void RunFromLibrary(const std::string& name, - DartLibraryProvider* library_provider); + void RunFromLibrary(std::string name, DartLibraryProvider* library_provider); void RunFromPrecompiledSnapshot(); void RunFromSnapshot(mojo::ScopedDataPipeConsumerHandle snapshot); void RunFromSnapshotBuffer(const uint8_t* buffer, size_t size); @@ -53,11 +44,8 @@ class DartController { std::unique_ptr snapshot_loader_; - base::WeakPtrFactory weak_factory_; - FTL_DISALLOW_COPY_AND_ASSIGN(DartController); }; - } -#endif // SKY_ENGINE_CORE_SCRIPT_DART_CONTROLLER_H_ +#endif // SKY_ENGINE_CORE_SCRIPT_DART_CONTROLLER_H_ diff --git a/sky/engine/platform/BUILD.gn b/sky/engine/platform/BUILD.gn index 7236ca77388..34515513c8f 100644 --- a/sky/engine/platform/BUILD.gn +++ b/sky/engine/platform/BUILD.gn @@ -189,8 +189,6 @@ source_set("platform") { "graphics/skia/SkiaUtils.cpp", "graphics/skia/SkiaUtils.h", "heap/Handle.h", - "mojo/data_pipe.cc", - "mojo/data_pipe.h", "text/BidiCharacterRun.cpp", "text/BidiCharacterRun.h", "text/BidiContext.cpp", diff --git a/sky/engine/platform/mojo/data_pipe.cc b/sky/engine/platform/mojo/data_pipe.cc deleted file mode 100644 index cb935b793f0..00000000000 --- a/sky/engine/platform/mojo/data_pipe.cc +++ /dev/null @@ -1,59 +0,0 @@ -// Copyright 2013 The Chromium 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 "sky/engine/platform/mojo/data_pipe.h" - -#include - -#include "lib/ftl/macros.h" -#include "mojo/data_pipe_utils/data_pipe_drainer.h" -#include "sky/engine/platform/mojo/data_pipe.h" -#include "sky/engine/public/platform/Platform.h" -#include "sky/engine/wtf/RefPtr.h" - -using mojo::common::DataPipeDrainer; - -namespace blink { -namespace { - -class DrainJob : public DataPipeDrainer::Client { - public: - explicit DrainJob(std::function)> callback) - : callback_(callback) {} - - void Start(mojo::ScopedDataPipeConsumerHandle handle) { - buffer_ = SharedBuffer::create(); - drainer_.reset(new DataPipeDrainer(this, handle.Pass())); - } - - private: - // mojo::common::DataPipeDrainer::Client - void OnDataAvailable(const void* data, size_t num_bytes) override { - buffer_->append(static_cast(data), num_bytes); - } - - void OnDataComplete() override { - RefPtr buffer = buffer_.release(); - std::function)> callback = callback_; - - delete this; - - callback(buffer); - } - - std::function)> callback_; - RefPtr buffer_; - std::unique_ptr drainer_; - - FTL_DISALLOW_COPY_AND_ASSIGN(DrainJob); -}; - -} // namespace - -void DrainDataPipe(mojo::ScopedDataPipeConsumerHandle handle, - std::function)> callback) { - (new DrainJob(callback))->Start(handle.Pass()); -} - -} // namespace blink diff --git a/sky/engine/platform/mojo/data_pipe.h b/sky/engine/platform/mojo/data_pipe.h deleted file mode 100644 index 4cf57fead17..00000000000 --- a/sky/engine/platform/mojo/data_pipe.h +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright 2013 The Chromium 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 SKY_ENGINE_PLATFORM_MOJO_DATA_PIPE_H_ -#define SKY_ENGINE_PLATFORM_MOJO_DATA_PIPE_H_ - -#include - -#include "mojo/public/cpp/system/data_pipe.h" -#include "sky/engine/platform/SharedBuffer.h" - -namespace blink { - -void DrainDataPipe(mojo::ScopedDataPipeConsumerHandle handle, - std::function)> callback); - -} // namespace blink - -#endif // SKY_ENGINE_PLATFORM_MOJO_DATA_PIPE_H_ diff --git a/sky/engine/wtf/BUILD.gn b/sky/engine/wtf/BUILD.gn index 00b08550744..83040c82fd4 100644 --- a/sky/engine/wtf/BUILD.gn +++ b/sky/engine/wtf/BUILD.gn @@ -241,6 +241,15 @@ test("unittests") { ":wtf", "//third_party/gtest", ] + + # TODO(abarth): This is a lie - this test is not embedded in an environment + # that injects the system thunks, so system calls don't actually work. This + # just tricks the linker into thinking that an implementation of these calls + # will be injected at runtime so the link succeeds. + deps += [ + "//mojo/public/cpp/environment:standalone", + "//mojo/public/platform/native:system", + ] } component("test_support") {