Skip to content

Commit

Permalink
Fix weak pointer use violations in shell and platform view. (#8046)
Browse files Browse the repository at this point in the history
  • Loading branch information
chinmaygarde authored Mar 5, 2019
1 parent dd80fc9 commit e37bd27
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 16 deletions.
17 changes: 14 additions & 3 deletions shell/common/platform_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,18 +60,29 @@ void PlatformView::SetViewportMetrics(const blink::ViewportMetrics& metrics) {
}

void PlatformView::NotifyCreated() {
delegate_.OnPlatformViewCreated(CreateRenderingSurface());
std::unique_ptr<Surface> surface;

// Threading: We want to use the platform view on the non-platform thread.
// Using the weak pointer is illegal. But, we are going to introduce a latch
// so that the platform view is not collected till the surface is obtained.
auto* platform_view = this;
fml::ManualResetWaitableEvent latch;
fml::TaskRunner::RunNowOrPostTask(
task_runners_.GetGPUTaskRunner(), [platform_view, &surface, &latch]() {
surface = platform_view->CreateRenderingSurface();
latch.Signal();
});
latch.Wait();
delegate_.OnPlatformViewCreated(std::move(surface));
}

void PlatformView::NotifyDestroyed() {
delegate_.OnPlatformViewDestroyed();
}

sk_sp<GrContext> PlatformView::CreateResourceContext() const {
#ifndef OS_FUCHSIA
FML_DLOG(WARNING) << "This platform does not setup the resource "
"context on the IO thread for async texture uploads.";
#endif // OS_FUCHSIA
return nullptr;
}

Expand Down
2 changes: 2 additions & 0 deletions shell/common/platform_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ class PlatformView {
SkISize size_;
fml::WeakPtrFactory<PlatformView> weak_factory_;

// Unlike all other methods on the platform view, this is called on the GPU
// task runner.
virtual std::unique_ptr<Surface> CreateRenderingSurface();

private:
Expand Down
25 changes: 13 additions & 12 deletions shell/common/shell.cc
Original file line number Diff line number Diff line change
Expand Up @@ -446,26 +446,27 @@ void Shell::OnPlatformViewCreated(std::unique_ptr<Surface> surface) {
fml::TaskRunner::RunNowOrPostTask(gpu_task_runner, gpu_task);
};

auto io_task = [io_manager = io_manager_->GetWeakPtr(),
platform_view = platform_view_->GetWeakPtr(),
// Threading: Capture platform view by raw pointer and not the weak pointer.
// We are going to use the pointer on the IO thread which is not safe with a
// weak pointer. However, we are preventing the platform view from being
// collected by using a latch.
auto* platform_view = platform_view_.get();

FML_DCHECK(platform_view);

auto io_task = [io_manager = io_manager_->GetWeakPtr(), platform_view,
ui_task_runner = task_runners_.GetUITaskRunner(), ui_task] {
if (io_manager) {
if (io_manager && !io_manager->GetResourceContext()) {
io_manager->NotifyResourceContextAvailable(
platform_view ? platform_view->CreateResourceContext() : nullptr);
platform_view->CreateResourceContext());
}
// Step 1: Next, post a task on the UI thread to tell the engine that it has
// an output surface.
fml::TaskRunner::RunNowOrPostTask(ui_task_runner, ui_task);
};

// Step 0: If the IOManager doesn't already have a ResourceContext, tell the
// IO thread that the PlatformView can make one for it now.
// Otherwise, jump right to step 1 on the UI thread.
if (!io_manager_->GetResourceContext()) {
fml::TaskRunner::RunNowOrPostTask(task_runners_.GetIOTaskRunner(), io_task);
} else {
fml::TaskRunner::RunNowOrPostTask(task_runners_.GetUITaskRunner(), ui_task);
}
fml::TaskRunner::RunNowOrPostTask(task_runners_.GetIOTaskRunner(), io_task);

latch.Wait();
}

Expand Down
2 changes: 1 addition & 1 deletion shell/common/surface.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
namespace shell {

/// Represents a Frame that has been fully configured for the underlying client
/// rendering API. A frame may only be sumitted once.
/// rendering API. A frame may only be submitted once.
class SurfaceFrame {
public:
using SubmitCallback =
Expand Down

0 comments on commit e37bd27

Please sign in to comment.