Skip to content

Commit

Permalink
Avoid dereferencing IO manager weak pointers on the UI thread (#13232)
Browse files Browse the repository at this point in the history
UI thread APIs that need to access the IO thread's resource context should
obtain a weak pointer to the IO manager and pass that to the IO thread.
  • Loading branch information
jason-simmons committed Oct 18, 2019
1 parent 8aefcd8 commit 8882bf3
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 33 deletions.
25 changes: 10 additions & 15 deletions lib/ui/painting/image_encoding.cc
Expand Up @@ -211,21 +211,16 @@ Dart_Handle EncodeImage(CanvasImage* canvas_image,
tonic::DartState::Current(), callback_handle);

const auto& task_runners = UIDartState::Current()->GetTaskRunners();
auto context = UIDartState::Current()->GetResourceContext();

task_runners.GetIOTaskRunner()->PostTask(
fml::MakeCopyable([callback = std::move(callback), //
image = canvas_image->image(), //
context = std::move(context), //
ui_task_runner = task_runners.GetUITaskRunner(), //
image_format //
]() mutable {
EncodeImageAndInvokeDataCallback(std::move(callback), //
std::move(image), //
context.get(), //
std::move(ui_task_runner), //
image_format //
);

task_runners.GetIOTaskRunner()->PostTask(fml::MakeCopyable(
[callback = std::move(callback), image = canvas_image->image(),
io_manager = UIDartState::Current()->GetIOManager(),
ui_task_runner = task_runners.GetUITaskRunner(),
image_format]() mutable {
EncodeImageAndInvokeDataCallback(std::move(callback), std::move(image),
io_manager->GetResourceContext().get(),
std::move(ui_task_runner),
image_format);
}));

return Dart_Null();
Expand Down
10 changes: 5 additions & 5 deletions lib/ui/painting/multi_frame_codec.cc
Expand Up @@ -168,11 +168,11 @@ Dart_Handle MultiFrameCodec::getNextFrame(Dart_Handle callback_handle) {
[callback = std::make_unique<DartPersistentValue>(
tonic::DartState::Current(), callback_handle),
this, trace_id, ui_task_runner = task_runners.GetUITaskRunner(),
queue = UIDartState::Current()->GetSkiaUnrefQueue(),
context = dart_state->GetResourceContext()]() mutable {
GetNextFrameAndInvokeCallback(std::move(callback),
std::move(ui_task_runner), context,
std::move(queue), trace_id);
io_manager = dart_state->GetIOManager()]() mutable {
GetNextFrameAndInvokeCallback(
std::move(callback), std::move(ui_task_runner),
io_manager->GetResourceContext(), io_manager->GetSkiaUnrefQueue(),
trace_id);
}));

return Dart_Null();
Expand Down
6 changes: 3 additions & 3 deletions lib/ui/painting/picture.cc
Expand Up @@ -136,7 +136,7 @@ Dart_Handle Picture::RasterizeToImage(sk_sp<SkPicture> picture,
auto unref_queue = dart_state->GetSkiaUnrefQueue();
auto ui_task_runner = dart_state->GetTaskRunners().GetUITaskRunner();
auto io_task_runner = dart_state->GetTaskRunners().GetIOTaskRunner();
fml::WeakPtr<GrContext> resource_context = dart_state->GetResourceContext();
fml::WeakPtr<IOManager> io_manager = dart_state->GetIOManager();

// We can't create an image on this task runner because we don't have a
// graphics context. Even if we did, it would be slow anyway. Also, this
Expand Down Expand Up @@ -173,9 +173,9 @@ Dart_Handle Picture::RasterizeToImage(sk_sp<SkPicture> picture,

fml::TaskRunner::RunNowOrPostTask(io_task_runner, [ui_task_runner, picture,
picture_bounds, ui_task,
resource_context] {
io_manager] {
sk_sp<SkSurface> surface =
MakeSnapshotSurface(picture_bounds, resource_context);
MakeSnapshotSurface(picture_bounds, io_manager->GetResourceContext());
sk_sp<SkImage> raster_image = MakeRasterSnapshot(picture, surface);

fml::TaskRunner::RunNowOrPostTask(
Expand Down
12 changes: 4 additions & 8 deletions lib/ui/ui_dart_state.cc
Expand Up @@ -78,6 +78,10 @@ const TaskRunners& UIDartState::GetTaskRunners() const {
return task_runners_;
}

fml::WeakPtr<IOManager> UIDartState::GetIOManager() const {
return io_manager_;
}

fml::RefPtr<flutter::SkiaUnrefQueue> UIDartState::GetSkiaUnrefQueue() const {
// TODO(gw280): The WeakPtr here asserts that we are derefing it on the
// same thread as it was created on. As we can't guarantee that currently
Expand Down Expand Up @@ -119,14 +123,6 @@ void UIDartState::AddOrRemoveTaskObserver(bool add) {
}
}

fml::WeakPtr<GrContext> UIDartState::GetResourceContext() const {
FML_DCHECK(task_runners_.GetIOTaskRunner()->RunsTasksOnCurrentThread());
if (!io_manager_) {
return {};
}
return io_manager_->GetResourceContext();
}

fml::WeakPtr<ImageDecoder> UIDartState::GetImageDecoder() const {
return image_decoder_;
}
Expand Down
4 changes: 2 additions & 2 deletions lib/ui/ui_dart_state.h
Expand Up @@ -48,9 +48,9 @@ class UIDartState : public tonic::DartState {

void FlushMicrotasksNow();

fml::RefPtr<flutter::SkiaUnrefQueue> GetSkiaUnrefQueue() const;
fml::WeakPtr<IOManager> GetIOManager() const;

fml::WeakPtr<GrContext> GetResourceContext() const;
fml::RefPtr<flutter::SkiaUnrefQueue> GetSkiaUnrefQueue() const;

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

Expand Down

0 comments on commit 8882bf3

Please sign in to comment.