Skip to content

Commit

Permalink
Refactor to passing functions by const ref (flutter#13975)
Browse files Browse the repository at this point in the history
Moved our code to passing functions by const ref
  • Loading branch information
gaaclarke authored Nov 22, 2019
1 parent 11580eb commit 89e3958
Show file tree
Hide file tree
Showing 81 changed files with 213 additions and 187 deletions.
3 changes: 2 additions & 1 deletion flow/raster_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,14 @@ static bool IsPictureWorthRasterizing(SkPicture* picture,
return picture->approximateOpCount() > 5;
}

/// @note Procedure doesn't copy all closures.
static RasterCacheResult Rasterize(
GrContext* context,
const SkMatrix& ctm,
SkColorSpace* dst_color_space,
bool checkerboard,
const SkRect& logical_rect,
std::function<void(SkCanvas*)> draw_function) {
const std::function<void(SkCanvas*)>& draw_function) {
TRACE_EVENT0("flutter", "RasterCachePopulate");
SkIRect cache_rect = RasterCache::GetDeviceBounds(logical_rect, ctm);

Expand Down
2 changes: 1 addition & 1 deletion flow/scene_update_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class SceneUpdateContext {
virtual SkISize GetSize() const = 0;

virtual void SignalWritesFinished(
std::function<void(void)> on_writes_committed) = 0;
const std::function<void(void)>& on_writes_committed) = 0;

virtual scenic::Image* GetImage() = 0;

Expand Down
8 changes: 4 additions & 4 deletions flow/view_holder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ namespace flutter {
void ViewHolder::Create(zx_koid_t id,
fml::RefPtr<fml::TaskRunner> ui_task_runner,
fuchsia::ui::views::ViewHolderToken view_holder_token,
BindCallback on_bind_callback) {
const BindCallback& on_bind_callback) {
// This GPU thread contains at least 1 ViewHolder. Initialize the per-thread
// bindings.
if (tls_view_holder_bindings.get() == nullptr) {
Expand All @@ -64,7 +64,7 @@ void ViewHolder::Create(zx_koid_t id,

auto view_holder = std::make_unique<ViewHolder>(std::move(ui_task_runner),
std::move(view_holder_token),
std::move(on_bind_callback));
on_bind_callback);
bindings->emplace(id, std::move(view_holder));
}

Expand All @@ -91,10 +91,10 @@ ViewHolder* ViewHolder::FromId(zx_koid_t id) {

ViewHolder::ViewHolder(fml::RefPtr<fml::TaskRunner> ui_task_runner,
fuchsia::ui::views::ViewHolderToken view_holder_token,
BindCallback on_bind_callback)
const BindCallback& on_bind_callback)
: ui_task_runner_(std::move(ui_task_runner)),
pending_view_holder_token_(std::move(view_holder_token)),
pending_bind_callback_(std::move(on_bind_callback)) {
pending_bind_callback_(on_bind_callback) {
FML_DCHECK(ui_task_runner_);
FML_DCHECK(pending_view_holder_token_.value);
}
Expand Down
4 changes: 2 additions & 2 deletions flow/view_holder.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ class ViewHolder {
static void Create(zx_koid_t id,
fml::RefPtr<fml::TaskRunner> ui_task_runner,
fuchsia::ui::views::ViewHolderToken view_holder_token,
BindCallback on_bind_callback);
const BindCallback& on_bind_callback);
static void Destroy(zx_koid_t id);
static ViewHolder* FromId(zx_koid_t id);

ViewHolder(fml::RefPtr<fml::TaskRunner> ui_task_runner,
fuchsia::ui::views::ViewHolderToken view_holder_token,
BindCallback on_bind_callback);
const BindCallback& on_bind_callback);
~ViewHolder() = default;

// Sets the properties/opacity of the child view by issuing a Scenic command.
Expand Down
2 changes: 1 addition & 1 deletion fml/closure.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ using closure = std::function<void()>;
///
class ScopedCleanupClosure {
public:
ScopedCleanupClosure(fml::closure closure) : closure_(closure) {}
ScopedCleanupClosure(const fml::closure& closure) : closure_(closure) {}

~ScopedCleanupClosure() {
if (closure_) {
Expand Down
4 changes: 2 additions & 2 deletions fml/concurrent_message_loop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ std::shared_ptr<ConcurrentTaskRunner> ConcurrentMessageLoop::GetTaskRunner() {
return std::make_shared<ConcurrentTaskRunner>(weak_from_this());
}

void ConcurrentMessageLoop::PostTask(fml::closure task) {
void ConcurrentMessageLoop::PostTask(const fml::closure& task) {
if (!task) {
return;
}
Expand Down Expand Up @@ -107,7 +107,7 @@ ConcurrentTaskRunner::ConcurrentTaskRunner(

ConcurrentTaskRunner::~ConcurrentTaskRunner() = default;

void ConcurrentTaskRunner::PostTask(fml::closure task) {
void ConcurrentTaskRunner::PostTask(const fml::closure& task) {
if (!task) {
return;
}
Expand Down
4 changes: 2 additions & 2 deletions fml/concurrent_message_loop.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class ConcurrentMessageLoop

void WorkerMain();

void PostTask(fml::closure task);
void PostTask(const fml::closure& task);

FML_DISALLOW_COPY_AND_ASSIGN(ConcurrentMessageLoop);
};
Expand All @@ -55,7 +55,7 @@ class ConcurrentTaskRunner {

~ConcurrentTaskRunner();

void PostTask(fml::closure task);
void PostTask(const fml::closure& task);

private:
friend ConcurrentMessageLoop;
Expand Down
4 changes: 2 additions & 2 deletions fml/delayed_task.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
namespace fml {

DelayedTask::DelayedTask(size_t order,
fml::closure task,
const fml::closure& task,
fml::TimePoint target_time)
: order_(order), task_(std::move(task)), target_time_(target_time) {}
: order_(order), task_(task), target_time_(target_time) {}

DelayedTask::DelayedTask(const DelayedTask& other) = default;

Expand Down
4 changes: 3 additions & 1 deletion fml/delayed_task.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ namespace fml {

class DelayedTask {
public:
DelayedTask(size_t order, fml::closure task, fml::TimePoint target_time);
DelayedTask(size_t order,
const fml::closure& task,
fml::TimePoint target_time);

DelayedTask(const DelayedTask& other);

Expand Down
2 changes: 1 addition & 1 deletion fml/file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ ScopedTemporaryDirectory::~ScopedTemporaryDirectory() {
}

bool VisitFilesRecursively(const fml::UniqueFD& directory,
FileVisitor visitor) {
const FileVisitor& visitor) {
FileVisitor recursive_visitor = [&recursive_visitor, &visitor](
const UniqueFD& directory,
const std::string& filename) {
Expand Down
7 changes: 5 additions & 2 deletions fml/file.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ using FileVisitor = std::function<bool(const fml::UniqueFD& directory,
/// use our helper method `VisitFilesRecursively`.
///
/// @see `VisitFilesRecursively`.
bool VisitFiles(const fml::UniqueFD& directory, FileVisitor visitor);
/// @note Procedure doesn't copy all closures.
bool VisitFiles(const fml::UniqueFD& directory, const FileVisitor& visitor);

/// Recursively call `visitor` on all files inside the `directory`. Return false
/// if and only if the visitor returns false during the traversal.
Expand All @@ -119,7 +120,9 @@ bool VisitFiles(const fml::UniqueFD& directory, FileVisitor visitor);
/// compute the relative path between the root directory and the visited file.
///
/// @see `VisitFiles`.
bool VisitFilesRecursively(const fml::UniqueFD& directory, FileVisitor visitor);
/// @note Procedure doesn't copy all closures.
bool VisitFilesRecursively(const fml::UniqueFD& directory,
const FileVisitor& visitor);

class ScopedTemporaryDirectory {
public:
Expand Down
2 changes: 1 addition & 1 deletion fml/mapping.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ const uint8_t* DataMapping::GetMapping() const {

NonOwnedMapping::NonOwnedMapping(const uint8_t* data,
size_t size,
ReleaseProc release_proc)
const ReleaseProc& release_proc)
: data_(data), size_(size), release_proc_(release_proc) {}

NonOwnedMapping::~NonOwnedMapping() {
Expand Down
2 changes: 1 addition & 1 deletion fml/mapping.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ class NonOwnedMapping final : public Mapping {
using ReleaseProc = std::function<void(const uint8_t* data, size_t size)>;
NonOwnedMapping(const uint8_t* data,
size_t size,
ReleaseProc release_proc = nullptr);
const ReleaseProc& release_proc = nullptr);

~NonOwnedMapping() override;

Expand Down
2 changes: 1 addition & 1 deletion fml/message_loop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ fml::RefPtr<MessageLoopImpl> MessageLoop::GetLoopImpl() const {
return loop_;
}

void MessageLoop::AddTaskObserver(intptr_t key, fml::closure callback) {
void MessageLoop::AddTaskObserver(intptr_t key, const fml::closure& callback) {
loop_->AddTaskObserver(key, callback);
}

Expand Down
2 changes: 1 addition & 1 deletion fml/message_loop.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class MessageLoop {

void Terminate();

void AddTaskObserver(intptr_t key, fml::closure callback);
void AddTaskObserver(intptr_t key, const fml::closure& callback);

void RemoveTaskObserver(intptr_t key);

Expand Down
6 changes: 4 additions & 2 deletions fml/message_loop_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ MessageLoopImpl::~MessageLoopImpl() {
task_queue_->Dispose(queue_id_);
}

void MessageLoopImpl::PostTask(fml::closure task, fml::TimePoint target_time) {
void MessageLoopImpl::PostTask(const fml::closure& task,
fml::TimePoint target_time) {
FML_DCHECK(task != nullptr);
FML_DCHECK(task != nullptr);
if (terminated_) {
Expand All @@ -61,7 +62,8 @@ void MessageLoopImpl::PostTask(fml::closure task, fml::TimePoint target_time) {
task_queue_->RegisterTask(queue_id_, task, target_time);
}

void MessageLoopImpl::AddTaskObserver(intptr_t key, fml::closure callback) {
void MessageLoopImpl::AddTaskObserver(intptr_t key,
const fml::closure& callback) {
FML_DCHECK(callback != nullptr);
FML_DCHECK(MessageLoop::GetCurrent().GetLoopImpl().get() == this)
<< "Message loop task observer must be added on the same thread as the "
Expand Down
4 changes: 2 additions & 2 deletions fml/message_loop_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ class MessageLoopImpl : public Wakeable,

virtual void Terminate() = 0;

void PostTask(fml::closure task, fml::TimePoint target_time);
void PostTask(const fml::closure& task, fml::TimePoint target_time);

void AddTaskObserver(intptr_t key, fml::closure callback);
void AddTaskObserver(intptr_t key, const fml::closure& callback);

void RemoveTaskObserver(intptr_t key);

Expand Down
6 changes: 3 additions & 3 deletions fml/message_loop_task_queues.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,13 @@ void MessageLoopTaskQueues::DisposeTasks(TaskQueueId queue_id) {
}

void MessageLoopTaskQueues::RegisterTask(TaskQueueId queue_id,
fml::closure task,
const fml::closure& task,
fml::TimePoint target_time) {
std::scoped_lock queue_lock(GetMutex(queue_id));

size_t order = order_++;
const auto& queue_entry = queue_entries_[queue_id];
queue_entry->delayed_tasks.push({order, std::move(task), target_time});
queue_entry->delayed_tasks.push({order, task, target_time});
TaskQueueId loop_to_wake = queue_id;
if (queue_entry->subsumed_by != _kUnmerged) {
loop_to_wake = queue_entry->subsumed_by;
Expand Down Expand Up @@ -157,7 +157,7 @@ size_t MessageLoopTaskQueues::GetNumPendingTasks(TaskQueueId queue_id) const {

void MessageLoopTaskQueues::AddTaskObserver(TaskQueueId queue_id,
intptr_t key,
fml::closure callback) {
const fml::closure& callback) {
std::scoped_lock queue_lock(GetMutex(queue_id));

FML_DCHECK(callback != nullptr) << "Observer callback must be non-null.";
Expand Down
4 changes: 2 additions & 2 deletions fml/message_loop_task_queues.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class MessageLoopTaskQueues
// Tasks methods.

void RegisterTask(TaskQueueId queue_id,
fml::closure task,
const fml::closure& task,
fml::TimePoint target_time);

bool HasPendingTasks(TaskQueueId queue_id) const;
Expand All @@ -93,7 +93,7 @@ class MessageLoopTaskQueues

void AddTaskObserver(TaskQueueId queue_id,
intptr_t key,
fml::closure callback);
const fml::closure& callback);

void RemoveTaskObserver(TaskQueueId queue_id, intptr_t key);

Expand Down
2 changes: 1 addition & 1 deletion fml/platform/posix/file_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ bool WriteAtomically(const fml::UniqueFD& base_directory,
base_directory.get(), file_name) == 0;
}

bool VisitFiles(const fml::UniqueFD& directory, FileVisitor visitor) {
bool VisitFiles(const fml::UniqueFD& directory, const FileVisitor& visitor) {
fml::UniqueFD dup_fd(dup(directory.get()));
if (!dup_fd.is_valid()) {
FML_DLOG(ERROR) << "Can't dup the directory fd. Error: " << strerror(errno);
Expand Down
2 changes: 1 addition & 1 deletion fml/platform/win/file_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ bool WriteAtomically(const fml::UniqueFD& base_directory,
return true;
}

bool VisitFiles(const fml::UniqueFD& directory, FileVisitor visitor) {
bool VisitFiles(const fml::UniqueFD& directory, const FileVisitor& visitor) {
std::string search_pattern = GetFullHandlePath(directory) + "\\*";
WIN32_FIND_DATA find_file_data;
HANDLE find_handle = ::FindFirstFile(
Expand Down
15 changes: 8 additions & 7 deletions fml/task_runner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,18 @@ TaskRunner::TaskRunner(fml::RefPtr<MessageLoopImpl> loop)

TaskRunner::~TaskRunner() = default;

void TaskRunner::PostTask(fml::closure task) {
loop_->PostTask(std::move(task), fml::TimePoint::Now());
void TaskRunner::PostTask(const fml::closure& task) {
loop_->PostTask(task, fml::TimePoint::Now());
}

void TaskRunner::PostTaskForTime(fml::closure task,
void TaskRunner::PostTaskForTime(const fml::closure& task,
fml::TimePoint target_time) {
loop_->PostTask(std::move(task), target_time);
loop_->PostTask(task, target_time);
}

void TaskRunner::PostDelayedTask(fml::closure task, fml::TimeDelta delay) {
loop_->PostTask(std::move(task), fml::TimePoint::Now() + delay);
void TaskRunner::PostDelayedTask(const fml::closure& task,
fml::TimeDelta delay) {
loop_->PostTask(task, fml::TimePoint::Now() + delay);
}

TaskQueueId TaskRunner::GetTaskQueueId() {
Expand Down Expand Up @@ -62,7 +63,7 @@ bool TaskRunner::RunsTasksOnCurrentThread() {
}

void TaskRunner::RunNowOrPostTask(fml::RefPtr<fml::TaskRunner> runner,
fml::closure task) {
const fml::closure& task) {
FML_DCHECK(runner);
if (runner->RunsTasksOnCurrentThread()) {
task();
Expand Down
9 changes: 5 additions & 4 deletions fml/task_runner.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,19 @@ class TaskRunner : public fml::RefCountedThreadSafe<TaskRunner> {
public:
virtual ~TaskRunner();

virtual void PostTask(fml::closure task);
virtual void PostTask(const fml::closure& task);

virtual void PostTaskForTime(fml::closure task, fml::TimePoint target_time);
virtual void PostTaskForTime(const fml::closure& task,
fml::TimePoint target_time);

virtual void PostDelayedTask(fml::closure task, fml::TimeDelta delay);
virtual void PostDelayedTask(const fml::closure& task, fml::TimeDelta delay);

virtual bool RunsTasksOnCurrentThread();

virtual TaskQueueId GetTaskQueueId();

static void RunNowOrPostTask(fml::RefPtr<fml::TaskRunner> runner,
fml::closure task);
const fml::closure& task);

protected:
TaskRunner(fml::RefPtr<MessageLoopImpl> loop);
Expand Down
3 changes: 2 additions & 1 deletion lib/ui/painting/image_decoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,8 @@ static SkiaGPUObject<SkImage> UploadRasterImage(
return {texture_image, queue};
}

void ImageDecoder::Decode(ImageDescriptor descriptor, ImageResult callback) {
void ImageDecoder::Decode(ImageDescriptor descriptor,
const ImageResult& callback) {
TRACE_EVENT0("flutter", __FUNCTION__);
fml::tracing::TraceFlow flow(__FUNCTION__);

Expand Down
2 changes: 1 addition & 1 deletion lib/ui/painting/image_decoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class ImageDecoder {
// concurrently. Texture upload is done on the IO thread and the result
// returned back on the UI thread. On error, the texture is null but the
// callback is guaranteed to return on the UI thread.
void Decode(ImageDescriptor descriptor, ImageResult result);
void Decode(ImageDescriptor descriptor, const ImageResult& result);

fml::WeakPtr<ImageDecoder> GetWeakPtr() const;

Expand Down
Loading

0 comments on commit 89e3958

Please sign in to comment.