Commit 49a968d7 authored by ckitagawa's avatar ckitagawa Committed by Commit Bot

[Paint Preview] Clarify thread behaviors

Clairfy the correct thread behaviors for PaintPreviewClient and
PaintPreviewBaseService.

Largely just a documentation change.

Change-Id: Ia1f9cadc8420a80610b3fd8ddb17d77bae3b12d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2090198Reviewed-by: default avatarMehran Mahmoudi <mahmoudi@chromium.org>
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747780}
parent 44f9ae29
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
#include "components/paint_preview/browser/paint_preview_client.h" #include "components/paint_preview/browser/paint_preview_client.h"
#include "components/paint_preview/browser/paint_preview_compositor_service_impl.h" #include "components/paint_preview/browser/paint_preview_compositor_service_impl.h"
#include "components/paint_preview/common/mojom/paint_preview_recorder.mojom.h" #include "components/paint_preview/common/mojom/paint_preview_recorder.mojom.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "ui/gfx/geometry/rect.h" #include "ui/gfx/geometry/rect.h"
...@@ -73,6 +74,7 @@ void PaintPreviewBaseService::CapturePaintPreview( ...@@ -73,6 +74,7 @@ void PaintPreviewBaseService::CapturePaintPreview(
const base::FilePath& root_dir, const base::FilePath& root_dir,
gfx::Rect clip_rect, gfx::Rect clip_rect,
OnCapturedCallback callback) { OnCapturedCallback callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (policy_ && !policy_->SupportedForContents(web_contents)) { if (policy_ && !policy_->SupportedForContents(web_contents)) {
std::move(callback).Run(kContentUnsupported, nullptr); std::move(callback).Run(kContentUnsupported, nullptr);
return; return;
......
...@@ -81,6 +81,11 @@ class PaintPreviewBaseService : public KeyedService { ...@@ -81,6 +81,11 @@ class PaintPreviewBaseService : public KeyedService {
const DirectoryKey& key, const DirectoryKey& key,
OnReadProtoCallback on_read_proto_callback); OnReadProtoCallback on_read_proto_callback);
// Captures need to run on the Browser UI thread! Captures may involve child
// frames so the PaintPreviewClient (WebContentsObserver) must be stored as
// WebContentsUserData which is not thread safe and must only be accessible
// from a specific sequence i.e. the UI thread.
//
// The following methods both capture a Paint Preview; however, their behavior // The following methods both capture a Paint Preview; however, their behavior
// and intended use is different. The first method is intended for capturing // and intended use is different. The first method is intended for capturing
// full page contents. Generally, this is what you should be using for most // full page contents. Generally, this is what you should be using for most
......
...@@ -193,8 +193,6 @@ void PaintPreviewClient::RenderFrameDeleted( ...@@ -193,8 +193,6 @@ void PaintPreviewClient::RenderFrameDeleted(
PaintPreviewClient::CreateResult PaintPreviewClient::CreateFileHandle( PaintPreviewClient::CreateResult PaintPreviewClient::CreateFileHandle(
const base::FilePath& path) { const base::FilePath& path) {
base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
base::BlockingType::MAY_BLOCK);
base::File file(path, base::File file(path,
base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE); base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
return CreateResult(std::move(file), file.error_details()); return CreateResult(std::move(file), file.error_details());
...@@ -215,6 +213,7 @@ mojom::PaintPreviewCaptureParamsPtr PaintPreviewClient::CreateMojoParams( ...@@ -215,6 +213,7 @@ mojom::PaintPreviewCaptureParamsPtr PaintPreviewClient::CreateMojoParams(
void PaintPreviewClient::CapturePaintPreviewInternal( void PaintPreviewClient::CapturePaintPreviewInternal(
const PaintPreviewParams& params, const PaintPreviewParams& params,
content::RenderFrameHost* render_frame_host) { content::RenderFrameHost* render_frame_host) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
// Use a frame's embedding token as its GUID. Note that we create a GUID for // Use a frame's embedding token as its GUID. Note that we create a GUID for
// the main frame so that we can treat it the same as other frames. // the main frame so that we can treat it the same as other frames.
auto token = render_frame_host->GetEmbeddingToken(); auto token = render_frame_host->GetEmbeddingToken();
......
...@@ -27,6 +27,9 @@ namespace paint_preview { ...@@ -27,6 +27,9 @@ namespace paint_preview {
// Client responsible for making requests to the mojom::PaintPreviewService. A // Client responsible for making requests to the mojom::PaintPreviewService. A
// client coordinates between multiple frames and handles capture and // client coordinates between multiple frames and handles capture and
// aggreagation of data from both the main frame and subframes. // aggreagation of data from both the main frame and subframes.
//
// Should be created and accessed from the UI thread as WebContentsUserData
// requires this behavior.
class PaintPreviewClient class PaintPreviewClient
: public content::WebContentsUserData<PaintPreviewClient>, : public content::WebContentsUserData<PaintPreviewClient>,
public content::WebContentsObserver { public content::WebContentsObserver {
...@@ -57,6 +60,8 @@ class PaintPreviewClient ...@@ -57,6 +60,8 @@ class PaintPreviewClient
~PaintPreviewClient() override; ~PaintPreviewClient() override;
// IMPORTANT: The Capture* methods must be called on the UI thread!
// Captures a paint preview corresponding to the content of // Captures a paint preview corresponding to the content of
// |render_frame_host|. This will work for capturing entire documents if // |render_frame_host|. This will work for capturing entire documents if
// passed the main frame or for just a specific subframe depending on // passed the main frame or for just a specific subframe depending on
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment