diff options
Diffstat (limited to 'chromium/components/paint_preview')
54 files changed, 1581 insertions, 619 deletions
diff --git a/chromium/components/paint_preview/DIR_METADATA b/chromium/components/paint_preview/DIR_METADATA new file mode 100644 index 00000000000..2782d66a2e5 --- /dev/null +++ b/chromium/components/paint_preview/DIR_METADATA @@ -0,0 +1,3 @@ +monorail { + component: "Internals>FreezeDriedTabs" +} diff --git a/chromium/components/paint_preview/OWNERS b/chromium/components/paint_preview/OWNERS index 6c3da5b1b43..f14326f08eb 100644 --- a/chromium/components/paint_preview/OWNERS +++ b/chromium/components/paint_preview/OWNERS @@ -2,4 +2,3 @@ ckitagawa@chromium.org mahmoudi@chromium.org vollick@chromium.org yfriedman@chromium.org -# COMPONENT: Internals>FreezeDriedTabs diff --git a/chromium/components/paint_preview/browser/BUILD.gn b/chromium/components/paint_preview/browser/BUILD.gn index fe905fd1dd9..41d74474d50 100644 --- a/chromium/components/paint_preview/browser/BUILD.gn +++ b/chromium/components/paint_preview/browser/BUILD.gn @@ -2,6 +2,7 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +import("//build/util/version.gni") import("//testing/test.gni") assert(!is_ios, "Paint Previews are not supported on iOS.") @@ -24,6 +25,8 @@ source_set("browser") { "paint_preview_compositor_client_impl.h", "paint_preview_compositor_service_impl.cc", "paint_preview_compositor_service_impl.h", + "paint_preview_file_mixin.cc", + "paint_preview_file_mixin.h", "paint_preview_policy.h", "service_sandbox_type.h", "warm_compositor.cc", @@ -45,6 +48,8 @@ source_set("browser") { "//third_party/blink/public:blink_headers", "//third_party/blink/public/common", "//third_party/zlib/google:zip", + "//ui/accessibility", + "//ui/accessibility/mojom", "//ui/gfx/geometry", "//url", ] @@ -56,6 +61,13 @@ source_set("browser") { "//components/paint_preview/public", "//components/services/paint_preview_compositor/public/mojom", ] + + defines = [ + "CHROME_VERSION_MAJOR=" + chrome_version_major, + "CHROME_VERSION_MINOR=" + chrome_version_minor, + "CHROME_VERSION_BUILD=" + chrome_version_build, + "CHROME_VERSION_PATCH=" + chrome_version_patch, + ] } source_set("test_support") { @@ -99,6 +111,13 @@ source_set("unit_tests") { "//ui/gfx/geometry", "//url", ] + + defines = [ + "CHROME_VERSION_MAJOR=" + chrome_version_major, + "CHROME_VERSION_MINOR=" + chrome_version_minor, + "CHROME_VERSION_BUILD=" + chrome_version_build, + "CHROME_VERSION_PATCH=" + chrome_version_patch, + ] } test("paint_preview_browser_unit_tests") { diff --git a/chromium/components/paint_preview/browser/DEPS b/chromium/components/paint_preview/browser/DEPS index 727a3fcbea5..fea23bd7267 100644 --- a/chromium/components/paint_preview/browser/DEPS +++ b/chromium/components/paint_preview/browser/DEPS @@ -12,4 +12,5 @@ include_rules = [ "+sandbox/policy/sandbox_type.h", "+third_party/blink/public/common", "+third_party/zlib/google", + "+ui/accessibility", ] diff --git a/chromium/components/paint_preview/browser/file_manager.cc b/chromium/components/paint_preview/browser/file_manager.cc index c1baabd1401..4eae8b11dd8 100644 --- a/chromium/components/paint_preview/browser/file_manager.cc +++ b/chromium/components/paint_preview/browser/file_manager.cc @@ -14,6 +14,7 @@ #include "base/metrics/histogram_functions.h" #include "base/strings/string_number_conversions.h" #include "components/paint_preview/common/file_utils.h" +#include "components/paint_preview/common/proto_validator.h" #include "third_party/zlib/google/zip.h" namespace paint_preview { @@ -230,8 +231,9 @@ FileManager::DeserializePaintPreviewProto(const DirectoryKey& key) const { return std::make_pair(ProtoReadStatus::kNoProto, nullptr); auto proto = ReadProtoFromFile(path->AppendASCII(kProtoName)); - if (proto == nullptr) + if (proto == nullptr || !PaintPreviewProtoValid(*proto)) { return std::make_pair(ProtoReadStatus::kDeserializationError, nullptr); + } return std::make_pair(ProtoReadStatus::kOk, std::move(proto)); } diff --git a/chromium/components/paint_preview/browser/paint_preview_base_service.cc b/chromium/components/paint_preview/browser/paint_preview_base_service.cc index 148874e86c9..67771f6d527 100644 --- a/chromium/components/paint_preview/browser/paint_preview_base_service.cc +++ b/chromium/components/paint_preview/browser/paint_preview_base_service.cc @@ -12,10 +12,7 @@ #include "base/files/file_path.h" #include "base/logging.h" #include "base/metrics/histogram_functions.h" -#include "base/task/task_traits.h" -#include "base/task/thread_pool.h" #include "build/build_config.h" -#include "components/keyed_service/core/keyed_service.h" #include "components/paint_preview/browser/compositor_utils.h" #include "components/paint_preview/browser/paint_preview_client.h" #include "components/paint_preview/common/mojom/paint_preview_recorder.mojom.h" @@ -25,99 +22,23 @@ namespace paint_preview { -namespace { - -const char kPaintPreviewDir[] = "paint_preview"; - -} // namespace - PaintPreviewBaseService::PaintPreviewBaseService( - const base::FilePath& path, - base::StringPiece ascii_feature_name, + std::unique_ptr<PaintPreviewFileMixin> file_mixin, std::unique_ptr<PaintPreviewPolicy> policy, bool is_off_the_record) - : policy_(std::move(policy)), - task_runner_(base::ThreadPool::CreateSequencedTaskRunner( - {base::MayBlock(), base::TaskPriority::USER_VISIBLE, - base::TaskShutdownBehavior::BLOCK_SHUTDOWN, - base::ThreadPolicy::MUST_USE_FOREGROUND})), - file_manager_(base::MakeRefCounted<FileManager>( - path.AppendASCII(kPaintPreviewDir).AppendASCII(ascii_feature_name), - task_runner_)), + : file_mixin_(std::move(file_mixin)), + policy_(std::move(policy)), is_off_the_record_(is_off_the_record) {} PaintPreviewBaseService::~PaintPreviewBaseService() = default; -void PaintPreviewBaseService::GetCapturedPaintPreviewProto( - const DirectoryKey& key, - base::Optional<base::TimeDelta> expiry_horizon, - OnReadProtoCallback on_read_proto_callback) { - task_runner_->PostTaskAndReplyWithResult( - FROM_HERE, - base::BindOnce( - [](scoped_refptr<FileManager> file_manager, const DirectoryKey& key, - base::Optional<base::TimeDelta> expiry_horizon) - -> std::pair<PaintPreviewBaseService::ProtoReadStatus, - std::unique_ptr<PaintPreviewProto>> { - if (expiry_horizon.has_value()) { - auto file_info = file_manager->GetInfo(key); - if (!file_info.has_value()) - return std::make_pair(ProtoReadStatus::kNoProto, nullptr); - - if (file_info->last_modified + expiry_horizon.value() < - base::Time::NowFromSystemTime()) { - return std::make_pair(ProtoReadStatus::kExpired, nullptr); - } - } - auto result = file_manager->DeserializePaintPreviewProto(key); - PaintPreviewBaseService::ProtoReadStatus status = - ProtoReadStatus::kNoProto; - switch (result.first) { - case FileManager::ProtoReadStatus::kOk: - status = ProtoReadStatus::kOk; - break; - case FileManager::ProtoReadStatus::kNoProto: - status = ProtoReadStatus::kNoProto; - break; - case FileManager::ProtoReadStatus::kDeserializationError: - status = ProtoReadStatus::kDeserializationError; - break; - default: - NOTREACHED(); - } - return std::make_pair(status, std::move(result.second)); - }, - file_manager_, key, expiry_horizon), - base::BindOnce( - [](OnReadProtoCallback callback, - std::pair<PaintPreviewBaseService::ProtoReadStatus, - std::unique_ptr<PaintPreviewProto>> result) { - std::move(callback).Run(result.first, std::move(result.second)); - }, - std::move(on_read_proto_callback))); -} - -void PaintPreviewBaseService::CapturePaintPreview( - content::WebContents* web_contents, - const base::FilePath& root_dir, - gfx::Rect clip_rect, - bool capture_links, - size_t max_per_capture_size, - OnCapturedCallback callback) { - CapturePaintPreview(web_contents, web_contents->GetMainFrame(), root_dir, - clip_rect, capture_links, max_per_capture_size, - std::move(callback)); -} - -void PaintPreviewBaseService::CapturePaintPreview( - content::WebContents* web_contents, - content::RenderFrameHost* render_frame_host, - const base::FilePath& root_dir, - gfx::Rect clip_rect, - bool capture_links, - size_t max_per_capture_size, - OnCapturedCallback callback) { +void PaintPreviewBaseService::CapturePaintPreview(CaptureParams capture_params, + OnCapturedCallback callback) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + content::WebContents* web_contents = capture_params.web_contents; + content::RenderFrameHost* render_frame_host = + capture_params.render_frame_host ? capture_params.render_frame_host + : web_contents->GetMainFrame(); if (policy_ && !policy_->SupportedForContents(web_contents)) { std::move(callback).Run(CaptureStatus::kContentUnsupported, {}); return; @@ -130,14 +51,15 @@ void PaintPreviewBaseService::CapturePaintPreview( return; } - PaintPreviewClient::PaintPreviewParams params( - RecordingPersistence::kFileSystem); - params.root_dir = root_dir; - params.inner.clip_rect = clip_rect; + PaintPreviewClient::PaintPreviewParams params(capture_params.persistence); + if (capture_params.root_dir) { + params.root_dir = *capture_params.root_dir; + } + params.inner.clip_rect = capture_params.clip_rect; params.inner.is_main_frame = (render_frame_host == web_contents->GetMainFrame()); - params.inner.capture_links = capture_links; - params.inner.max_capture_size = max_per_capture_size; + params.inner.capture_links = capture_params.capture_links; + params.inner.max_capture_size = capture_params.max_per_capture_size; // TODO(crbug/1064253): Consider moving to client so that this always happens. // Although, it is harder to get this right in the client due to its diff --git a/chromium/components/paint_preview/browser/paint_preview_base_service.h b/chromium/components/paint_preview/browser/paint_preview_base_service.h index a9ca093e675..4d3fee26ad8 100644 --- a/chromium/components/paint_preview/browser/paint_preview_base_service.h +++ b/chromium/components/paint_preview/browser/paint_preview_base_service.h @@ -9,7 +9,6 @@ #include "base/bind.h" #include "base/callback.h" -#include "base/files/file_path.h" #include "base/macros.h" #include "base/memory/weak_ptr.h" #include "base/optional.h" @@ -17,28 +16,32 @@ #include "base/time/time.h" #include "base/unguessable_token.h" #include "build/build_config.h" -#include "components/keyed_service/core/keyed_service.h" -#include "components/paint_preview/browser/file_manager.h" +#include "components/paint_preview/browser/paint_preview_file_mixin.h" #include "components/paint_preview/browser/paint_preview_policy.h" #include "components/paint_preview/common/capture_result.h" #include "components/paint_preview/common/file_utils.h" #include "components/paint_preview/common/mojom/paint_preview_recorder.mojom.h" #include "components/paint_preview/common/proto/paint_preview.pb.h" +#include "components/paint_preview/common/serialized_recording.h" #include "content/public/browser/web_contents.h" namespace paint_preview { -// A base KeyedService that serves as the Public API for Paint Previews. +// A base class that serves as the Public API for Paint Previews. // Features that want to use Paint Previews should extend this class. +// This service supports both in-memery and in-file captures. +// // The KeyedService provides a 1:1 mapping between the service and a key or // profile allowing each feature built on Paint Previews to reliably store // necessary data to the right directory on disk. // -// Implementations of this service should be created by implementing a factory -// that extends one of: -// - BrowserContextKeyedServiceFactory -// OR preferably the -// - SimpleKeyedServiceFactory +// [NOTE] for file system captures: +// - PaintPreviewFileMixin object needs to be supplied. +// - Implementations of the service should be created by implementing a factory +// that extends one of: +// - BrowserContextKeyedServiceFactory +// OR preferably the +// - SimpleKeyedServiceFactory class PaintPreviewBaseService : public KeyedService { public: enum class CaptureStatus : int { @@ -48,96 +51,77 @@ class PaintPreviewBaseService : public KeyedService { kCaptureFailed, }; - enum class ProtoReadStatus : int { - kOk = 0, - kNoProto, - kDeserializationError, - kExpired, + struct CaptureParams { + content::WebContents* web_contents = nullptr; + + // In case of specifying, an individual |render_frame_host| and its + // descendents will be captured. In case of nullptr, full page contents will + // be captured. + // + // Generally, leaving this as nullptr is what you should be doing for most + // features. Specifying a |render_frame_host| is intended for capturing + // individual subframes and should be used for only a few use cases. + content::RenderFrameHost* render_frame_host = nullptr; + + // Store artifacts in the file system or in memory buffers. + RecordingPersistence persistence; + + // |root_dir| should be created using + // GetFileManager()->CreateOrGetDirectoryFor(). However, to provide + // flexibility in managing the lifetime of created objects and ease cleanup + // if a capture fails the service implementation is responsible for + // implementing this management and tracking the directories in existence. + // Data in a directory will contain: + // - a number of SKPs listed as <guid>.skp (one per frame) + // + // Will be ignored if persistence = kMemoryBuffer + const base::FilePath* root_dir = nullptr; + + // The captured area is clipped to |clip_rect| if it is non-zero. + gfx::Rect clip_rect; + + // Whether to record links. + bool capture_links; + + // Cap the perframe SkPicture size to |max_per_capture_size| if non-zero. + size_t max_per_capture_size; }; using OnCapturedCallback = base::OnceCallback<void(CaptureStatus, std::unique_ptr<CaptureResult>)>; - using OnReadProtoCallback = - base::OnceCallback<void(ProtoReadStatus, - std::unique_ptr<PaintPreviewProto>)>; - // Creates a service instance for a feature. Artifacts produced will live in // |profile_dir|/paint_preview/|ascii_feature_name|. Implementers of the // factory can also elect their factory to not construct services in the event // a profile |is_off_the_record|. The |policy| object is responsible for // determining whether or not a given WebContents is amenable to paint // preview. If nullptr is passed as |policy| all content is deemed amenable. - PaintPreviewBaseService(const base::FilePath& profile_dir, - base::StringPiece ascii_feature_name, + // + // NOTE: Pass nullptr as |file_mixin| if you're planning to use the service + // for only in-memory captures. + PaintPreviewBaseService(std::unique_ptr<PaintPreviewFileMixin> file_mixin, std::unique_ptr<PaintPreviewPolicy> policy, bool is_off_the_record); ~PaintPreviewBaseService() override; - // Returns the file manager for the directory associated with the service. - scoped_refptr<FileManager> GetFileManager() { return file_manager_; } - - // Returns the task runner that IO tasks should be scheduled on. - scoped_refptr<base::SequencedTaskRunner> GetTaskRunner() { - return task_runner_; - } + PaintPreviewFileMixin* GetFileMixin() { return file_mixin_.get(); } // Returns whether the created service is off the record. bool IsOffTheRecord() const { return is_off_the_record_; } - // Acquires the PaintPreviewProto that is associated with |key| and sends it - // to |on_read_proto_callback|. The default implementation attempts to invoke - // GetFileManager()->DeserializePaintPreviewProto(). If |expiry_horizon| is - // provided a proto that was last modified earlier than |now - expiry_horizon| - // will return the kExpired status. - // - // Derived classes may override this function; for example, the proto is - // cached in memory. - virtual void GetCapturedPaintPreviewProto( - const DirectoryKey& key, - base::Optional<base::TimeDelta> expiry_horizon, - 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 - // 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 - // features. The second method is intended for capturing just an individual - // RenderFrameHost and its descendents. This is intended for capturing - // individual subframes and should be used for only a few use cases. + // Captures the main frame of |capture_params.web_contents| (an observer for + // capturing Paint Previews is created for web contents if it does not exist). + // The capture is attributed to the URL of the main frame. On completion the + // status of the capture is provided via |callback|. // - // NOTE: |root_dir| in the following methods should be created using - // GetFileManager()->CreateOrGetDirectoryFor(). However, to provide - // flexibility in managing the lifetime of created objects and ease cleanup - // if a capture fails the service implementation is responsible for - // implementing this management and tracking the directories in existence. - // Data in a directory will contain: - // - a number of SKPs listed as <guid>.skp (one per frame) - // - // Captures the main frame of |web_contents| (an observer for capturing Paint - // Previews is created for web contents if it does not exist). The capture is - // attributed to the URL of the main frame and is stored in |root_dir|. The - // captured area is clipped to |clip_rect| if it is non-zero. Caps the per - // frame SkPicture size to |max_per_capture_size| if non-zero. On completion - // the status of the capture is provided via |callback|. - void CapturePaintPreview(content::WebContents* web_contents, - const base::FilePath& root_dir, - gfx::Rect clip_rect, - bool capture_links, - size_t max_per_capture_size, - OnCapturedCallback callback); - // Same as above except |render_frame_host| is directly captured rather than - // the main frame. - void CapturePaintPreview(content::WebContents* web_contents, - content::RenderFrameHost* render_frame_host, - const base::FilePath& root_dir, - gfx::Rect clip_rect, - bool capture_links, - size_t max_per_capture_size, + // See |PaintPreviewBaseService::CaptureParams| for more info about the + // capture parameters. + void CapturePaintPreview(CaptureParams capture_params, OnCapturedCallback callback); private: @@ -148,9 +132,8 @@ class PaintPreviewBaseService : public KeyedService { mojom::PaintPreviewStatus status, std::unique_ptr<CaptureResult> result); + std::unique_ptr<PaintPreviewFileMixin> file_mixin_ = nullptr; std::unique_ptr<PaintPreviewPolicy> policy_ = nullptr; - scoped_refptr<base::SequencedTaskRunner> task_runner_; - scoped_refptr<FileManager> file_manager_; bool is_off_the_record_; base::WeakPtrFactory<PaintPreviewBaseService> weak_ptr_factory_{this}; diff --git a/chromium/components/paint_preview/browser/paint_preview_base_service_test_factory.cc b/chromium/components/paint_preview/browser/paint_preview_base_service_test_factory.cc index 8ae8532e3b1..f3f4f1f1a9d 100644 --- a/chromium/components/paint_preview/browser/paint_preview_base_service_test_factory.cc +++ b/chromium/components/paint_preview/browser/paint_preview_base_service_test_factory.cc @@ -40,7 +40,8 @@ void PaintPreviewBaseServiceTestFactory::Destroy(SimpleFactoryKey* key) { std::unique_ptr<KeyedService> PaintPreviewBaseServiceTestFactory::Build( SimpleFactoryKey* key) { return std::make_unique<PaintPreviewBaseService>( - key->GetPath(), kTestFeatureDir, nullptr, key->IsOffTheRecord()); + std::make_unique<PaintPreviewFileMixin>(key->GetPath(), kTestFeatureDir), + nullptr, key->IsOffTheRecord()); } PaintPreviewBaseServiceTestFactory::~PaintPreviewBaseServiceTestFactory() = diff --git a/chromium/components/paint_preview/browser/paint_preview_base_service_unittest.cc b/chromium/components/paint_preview/browser/paint_preview_base_service_unittest.cc index d8d75868eea..002447566c7 100644 --- a/chromium/components/paint_preview/browser/paint_preview_base_service_unittest.cc +++ b/chromium/components/paint_preview/browser/paint_preview_base_service_unittest.cc @@ -4,6 +4,9 @@ #include "components/paint_preview/browser/paint_preview_base_service.h" +#include <memory> +#include <utility> + #include "base/files/scoped_temp_dir.h" #include "base/macros.h" #include "base/no_destructor.h" @@ -11,7 +14,9 @@ #include "base/strings/utf_string_conversions.h" #include "build/build_config.h" #include "components/paint_preview/browser/paint_preview_base_service_test_factory.h" +#include "components/paint_preview/browser/paint_preview_file_mixin.h" #include "components/paint_preview/common/mojom/paint_preview_recorder.mojom.h" +#include "components/paint_preview/common/serialized_recording.h" #include "components/paint_preview/common/test_utils.h" #include "content/public/browser/render_process_host.h" #include "content/public/test/navigation_simulator.h" @@ -46,7 +51,7 @@ class RejectionPaintPreviewPolicy : public PaintPreviewPolicy { std::unique_ptr<KeyedService> BuildServiceWithRejectionPolicy( SimpleFactoryKey* key) { return std::make_unique<PaintPreviewBaseService>( - key->GetPath(), kTestFeatureDir, + std::make_unique<PaintPreviewFileMixin>(key->GetPath(), kTestFeatureDir), std::make_unique<RejectionPaintPreviewPolicy>(), key->IsOffTheRecord()); } @@ -120,7 +125,9 @@ class MockPaintPreviewRecorder : public mojom::PaintPreviewRecorder { mojo::AssociatedReceiver<mojom::PaintPreviewRecorder> binding_{this}; }; -class PaintPreviewBaseServiceTest : public content::RenderViewHostTestHarness { +class PaintPreviewBaseServiceTest + : public content::RenderViewHostTestHarness, + public testing::WithParamInterface<RecordingPersistence> { public: PaintPreviewBaseServiceTest() = default; ~PaintPreviewBaseServiceTest() override = default; @@ -165,12 +172,29 @@ class PaintPreviewBaseServiceTest : public content::RenderViewHostTestHarness { rejection_policy_key_.get()); } + PaintPreviewBaseService::CaptureParams CreateCaptureParams( + content::WebContents* web_contents, + base::FilePath* root_dir, + RecordingPersistence persistence, + gfx::Rect clip_rect, + bool capture_links, + size_t max_per_capture_size) { + PaintPreviewBaseService::CaptureParams capture_params; + capture_params.web_contents = web_contents; + capture_params.root_dir = root_dir; + capture_params.persistence = persistence; + capture_params.clip_rect = clip_rect; + capture_params.capture_links = capture_links; + capture_params.max_per_capture_size = max_per_capture_size; + return capture_params; + } + private: std::unique_ptr<SimpleFactoryKey> key_ = nullptr; std::unique_ptr<SimpleFactoryKey> rejection_policy_key_ = nullptr; }; -TEST_F(PaintPreviewBaseServiceTest, CaptureMainFrame) { +TEST_P(PaintPreviewBaseServiceTest, CaptureMainFrame) { MockPaintPreviewRecorder recorder; auto params = mojom::PaintPreviewCaptureParams::New(); params->clip_rect = gfx::Rect(0, 0, 0, 0); @@ -179,18 +203,22 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureMainFrame) { recorder.SetExpectedParams(std::move(params)); auto response = mojom::PaintPreviewCaptureResponse::New(); response->embedding_token = base::nullopt; + if (GetParam() == RecordingPersistence::kMemoryBuffer) { + response->skp.emplace(mojo_base::BigBuffer()); + } recorder.SetResponse(mojom::PaintPreviewStatus::kOk, std::move(response)); OverrideInterface(&recorder); auto* service = GetService(); EXPECT_FALSE(service->IsOffTheRecord()); - auto manager = service->GetFileManager(); + auto manager = service->GetFileMixin()->GetFileManager(); base::FilePath path = CreateDir( manager, manager->CreateKey(web_contents()->GetLastCommittedURL())); base::RunLoop loop; service->CapturePaintPreview( - web_contents(), path, gfx::Rect(0, 0, 0, 0), true, 50, + CreateCaptureParams(web_contents(), &path, GetParam(), + gfx::Rect(0, 0, 0, 0), true, 50), base::BindOnce( [](base::OnceClosure quit_closure, PaintPreviewBaseService::CaptureStatus expected_status, @@ -204,19 +232,32 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureMainFrame) { auto token = base::UnguessableToken::Deserialize( result->proto.root_frame().embedding_token_high(), result->proto.root_frame().embedding_token_low()); + switch (GetParam()) { + case RecordingPersistence::kFileSystem: { #if defined(OS_WIN) - base::FilePath path = base::FilePath( - base::UTF8ToUTF16(result->proto.root_frame().file_path())); - base::FilePath name( - base::UTF8ToUTF16(base::StrCat({token.ToString(), ".skp"}))); + base::FilePath path = base::FilePath( + base::UTF8ToWide(result->proto.root_frame().file_path())); + base::FilePath name( + base::UTF8ToWide(base::StrCat({token.ToString(), ".skp"}))); #else - base::FilePath path = - base::FilePath(result->proto.root_frame().file_path()); - base::FilePath name(base::StrCat({token.ToString(), ".skp"})); + base::FilePath path = + base::FilePath(result->proto.root_frame().file_path()); + base::FilePath name(base::StrCat({token.ToString(), ".skp"})); #endif - EXPECT_EQ(path.DirName(), expected_path); - LOG(ERROR) << expected_path; - EXPECT_EQ(path.BaseName(), name); + EXPECT_EQ(path.DirName(), expected_path); + LOG(ERROR) << expected_path; + EXPECT_EQ(path.BaseName(), name); + } break; + + case RecordingPersistence::kMemoryBuffer: { + EXPECT_EQ(result->serialized_skps.size(), 1u); + EXPECT_TRUE(result->serialized_skps.contains(token)); + } break; + + default: + NOTREACHED(); + break; + } std::move(quit_closure).Run(); }, loop.QuitClosure(), PaintPreviewBaseService::CaptureStatus::kOk, @@ -224,7 +265,7 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureMainFrame) { loop.Run(); } -TEST_F(PaintPreviewBaseServiceTest, CaptureFailed) { +TEST_P(PaintPreviewBaseServiceTest, CaptureFailed) { MockPaintPreviewRecorder recorder; auto params = mojom::PaintPreviewCaptureParams::New(); params->clip_rect = gfx::Rect(0, 0, 0, 0); @@ -238,13 +279,14 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureFailed) { auto* service = GetService(); EXPECT_FALSE(service->IsOffTheRecord()); - auto manager = service->GetFileManager(); + auto manager = service->GetFileMixin()->GetFileManager(); base::FilePath path = CreateDir( manager, manager->CreateKey(web_contents()->GetLastCommittedURL())); base::RunLoop loop; service->CapturePaintPreview( - web_contents(), path, gfx::Rect(0, 0, 0, 0), true, 0, + CreateCaptureParams(web_contents(), &path, GetParam(), + gfx::Rect(0, 0, 0, 0), true, 0), base::BindOnce( [](base::OnceClosure quit_closure, PaintPreviewBaseService::CaptureStatus expected_status, @@ -259,7 +301,7 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureFailed) { loop.Run(); } -TEST_F(PaintPreviewBaseServiceTest, CaptureDisallowed) { +TEST_P(PaintPreviewBaseServiceTest, CaptureDisallowed) { MockPaintPreviewRecorder recorder; auto params = mojom::PaintPreviewCaptureParams::New(); params->clip_rect = gfx::Rect(0, 0, 0, 0); @@ -273,13 +315,14 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureDisallowed) { auto* service = GetServiceWithRejectionPolicy(); EXPECT_FALSE(service->IsOffTheRecord()); - auto manager = service->GetFileManager(); + auto manager = service->GetFileMixin()->GetFileManager(); base::FilePath path = CreateDir( manager, manager->CreateKey(web_contents()->GetLastCommittedURL())); base::RunLoop loop; service->CapturePaintPreview( - web_contents(), path, gfx::Rect(0, 0, 0, 0), true, 0, + CreateCaptureParams(web_contents(), &path, GetParam(), + gfx::Rect(0, 0, 0, 0), true, 0), base::BindOnce( [](base::OnceClosure quit_closure, PaintPreviewBaseService::CaptureStatus expected_status, @@ -294,4 +337,10 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureDisallowed) { loop.Run(); } +INSTANTIATE_TEST_SUITE_P(All, + PaintPreviewBaseServiceTest, + testing::Values(RecordingPersistence::kFileSystem, + RecordingPersistence::kMemoryBuffer), + PersistenceParamToString); + } // namespace paint_preview diff --git a/chromium/components/paint_preview/browser/paint_preview_client.cc b/chromium/components/paint_preview/browser/paint_preview_client.cc index 1b055c56eb0..03f6f52df6e 100644 --- a/chromium/components/paint_preview/browser/paint_preview_client.cc +++ b/chromium/components/paint_preview/browser/paint_preview_client.cc @@ -18,6 +18,7 @@ #include "base/unguessable_token.h" #include "components/paint_preview/common/capture_result.h" #include "components/paint_preview/common/mojom/paint_preview_recorder.mojom-forward.h" +#include "components/paint_preview/common/proto_validator.h" #include "components/paint_preview/common/version.h" #include "components/ukm/content/source_url_recorder.h" #include "content/public/browser/browser_task_traits.h" @@ -300,6 +301,11 @@ void PaintPreviewClient::CapturePaintPreview( auto* metadata = document_data.proto.mutable_metadata(); metadata->set_url(url.spec()); metadata->set_version(kPaintPreviewVersion); + auto* chromeVersion = metadata->mutable_chrome_version(); + chromeVersion->set_major(CHROME_VERSION_MAJOR); + chromeVersion->set_minor(CHROME_VERSION_MINOR); + chromeVersion->set_build(CHROME_VERSION_BUILD); + chromeVersion->set_patch(CHROME_VERSION_PATCH); document_data.callback = std::move(callback); document_data.source_id = ukm::GetSourceIdForWebContentsDocument(web_contents()); @@ -537,6 +543,10 @@ void PaintPreviewClient::OnFinished( if (!document_data || !document_data->callback) return; + if (!PaintPreviewProtoValid(document_data->proto)) { + document_data->had_success = false; + } + TRACE_EVENT_NESTABLE_ASYNC_END2( "paint_preview", "PaintPreviewClient::CapturePaintPreview", TRACE_ID_LOCAL(document_data), "success", document_data->had_success, diff --git a/chromium/components/paint_preview/browser/paint_preview_client.h b/chromium/components/paint_preview/browser/paint_preview_client.h index 73124fce2be..1b3537afcc5 100644 --- a/chromium/components/paint_preview/browser/paint_preview_client.h +++ b/chromium/components/paint_preview/browser/paint_preview_client.h @@ -100,6 +100,7 @@ class PaintPreviewClient // artifacts to. Ignored if |RecordingPersistence::kMemoryBuffer|. base::FilePath root_dir; + // This corresponds to RenderFrameHost::EmbeddingToken. base::UnguessableToken root_frame_token; // Got from the first recording params. Whether to capture links and the diff --git a/chromium/components/paint_preview/browser/paint_preview_client_unittest.cc b/chromium/components/paint_preview/browser/paint_preview_client_unittest.cc index 838d159f921..8718a99d4e9 100644 --- a/chromium/components/paint_preview/browser/paint_preview_client_unittest.cc +++ b/chromium/components/paint_preview/browser/paint_preview_client_unittest.cc @@ -168,6 +168,11 @@ TEST_P(PaintPreviewClientRenderViewHostTest, CaptureMainFrameMock) { auto* metadata = expected_proto.mutable_metadata(); metadata->set_url(expected_url.spec()); metadata->set_version(kPaintPreviewVersion); + auto* chromeVersion = metadata->mutable_chrome_version(); + chromeVersion->set_major(CHROME_VERSION_MAJOR); + chromeVersion->set_minor(CHROME_VERSION_MINOR); + chromeVersion->set_build(CHROME_VERSION_BUILD); + chromeVersion->set_patch(CHROME_VERSION_PATCH); PaintPreviewFrameProto* main_frame = expected_proto.mutable_root_frame(); main_frame->set_is_main_frame(true); main_frame->set_scroll_offset_x(5); @@ -207,7 +212,7 @@ TEST_P(PaintPreviewClientRenderViewHostTest, CaptureMainFrameMock) { base::ScopedAllowBlockingForTesting scope; #if defined(OS_WIN) base::FilePath path = base::FilePath( - base::UTF8ToUTF16(result->proto.root_frame().file_path())); + base::UTF8ToWide(result->proto.root_frame().file_path())); #else base::FilePath path = base::FilePath(result->proto.root_frame().file_path()); diff --git a/chromium/components/paint_preview/browser/paint_preview_compositor_client_impl.cc b/chromium/components/paint_preview/browser/paint_preview_compositor_client_impl.cc index 51e1ecdb3ac..a4a008015ff 100644 --- a/chromium/components/paint_preview/browser/paint_preview_compositor_client_impl.cc +++ b/chromium/components/paint_preview/browser/paint_preview_compositor_client_impl.cc @@ -6,44 +6,11 @@ #include <utility> +#include "base/bind_post_task.h" #include "base/callback.h" namespace paint_preview { -namespace { - -// These methods bind a callback to a task runner. This simplifies situations -// where a caller provides a callback which should be passed to |compositor_| -// verbatim, but should be run on the caller's task runner rather than -// |compositor_task_runner_|. -// -// Based on the implementation in: chromecast/base/bind_to_task_runner.h - -template <typename Sig> -struct BindToTaskRunnerTrampoline; - -template <typename... Args> -struct BindToTaskRunnerTrampoline<void(Args...)> { - static void Run(base::TaskRunner* task_runner, - base::OnceCallback<void(Args...)> callback, - Args... args) { - task_runner->PostTask( - FROM_HERE, - base::BindOnce(std::move(callback), std::forward<Args>(args)...)); - } -}; - -template <typename T> -base::OnceCallback<T> BindToTaskRunner( - scoped_refptr<base::TaskRunner> task_runner, - base::OnceCallback<T> callback) { - return base::BindOnce(&BindToTaskRunnerTrampoline<T>::Run, - base::RetainedRef(std::move(task_runner)), - std::move(callback)); -} - -} // namespace - PaintPreviewCompositorClientImpl::PaintPreviewCompositorClientImpl( scoped_refptr<base::SequencedTaskRunner> compositor_task_runner, base::WeakPtr<PaintPreviewCompositorServiceImpl> service) @@ -91,7 +58,7 @@ void PaintPreviewCompositorClientImpl::BeginSeparatedFrameComposite( base::BindOnce( &mojom::PaintPreviewCompositor::BeginSeparatedFrameComposite, base::Unretained(compositor_.get()->get()), std::move(request), - BindToTaskRunner(default_task_runner_, std::move(callback)))); + base::BindPostTask(default_task_runner_, std::move(callback)))); } void PaintPreviewCompositorClientImpl::BitmapForSeparatedFrame( @@ -112,7 +79,7 @@ void PaintPreviewCompositorClientImpl::BitmapForSeparatedFrame( CHECK_EQ(bitmap.colorType(), kN32_SkColorType); std::move(callback).Run(status, bitmap); }, - BindToTaskRunner(default_task_runner_, std::move(callback))); + base::BindPostTask(default_task_runner_, std::move(callback))); compositor_task_runner_->PostTask( FROM_HERE, @@ -130,7 +97,7 @@ void PaintPreviewCompositorClientImpl::BeginMainFrameComposite( base::BindOnce( &mojom::PaintPreviewCompositor::BeginMainFrameComposite, base::Unretained(compositor_.get()->get()), std::move(request), - BindToTaskRunner(default_task_runner_, std::move(callback)))); + base::BindPostTask(default_task_runner_, std::move(callback)))); } void PaintPreviewCompositorClientImpl::BitmapForMainFrame( @@ -143,7 +110,7 @@ void PaintPreviewCompositorClientImpl::BitmapForMainFrame( base::BindOnce( &mojom::PaintPreviewCompositor::BitmapForMainFrame, base::Unretained(compositor_.get()->get()), clip_rect, scale_factor, - BindToTaskRunner(default_task_runner_, std::move(callback)))); + base::BindPostTask(default_task_runner_, std::move(callback)))); } void PaintPreviewCompositorClientImpl::SetRootFrameUrl(const GURL& url) { @@ -172,18 +139,12 @@ void PaintPreviewCompositorClientImpl::IsBoundAndConnected( std::move(callback))); } -mojo::PendingReceiver<mojom::PaintPreviewCompositor> -PaintPreviewCompositorClientImpl::BindNewPipeAndPassReceiver() { - DCHECK(compositor_task_runner_->RunsTasksInCurrentSequence()); - return compositor_->BindNewPipeAndPassReceiver(); -} - PaintPreviewCompositorClientImpl::OnCompositorCreatedCallback PaintPreviewCompositorClientImpl::BuildCompositorCreatedCallback( base::OnceClosure user_closure, OnCompositorCreatedCallback service_callback) { DCHECK(default_task_runner_->RunsTasksInCurrentSequence()); - return BindToTaskRunner( + return base::BindPostTask( default_task_runner_, base::BindOnce(&PaintPreviewCompositorClientImpl::OnCompositorCreated, weak_ptr_factory_.GetWeakPtr(), std::move(user_closure), @@ -203,7 +164,7 @@ void PaintPreviewCompositorClientImpl::OnCompositorCreated( base::BindOnce( &mojo::Remote<mojom::PaintPreviewCompositor>::set_disconnect_handler, base::Unretained(compositor_.get()), - BindToTaskRunner( + base::BindPostTask( default_task_runner_, base::BindOnce( &PaintPreviewCompositorClientImpl::DisconnectHandler, diff --git a/chromium/components/paint_preview/browser/paint_preview_compositor_client_impl.h b/chromium/components/paint_preview/browser/paint_preview_compositor_client_impl.h index 128ff60ea54..b8ec2b510db 100644 --- a/chromium/components/paint_preview/browser/paint_preview_compositor_client_impl.h +++ b/chromium/components/paint_preview/browser/paint_preview_compositor_client_impl.h @@ -59,9 +59,10 @@ class PaintPreviewCompositorClientImpl : public PaintPreviewCompositorClient { override; void SetRootFrameUrl(const GURL& url) override; - // Exposes underlying BindNewPipeAndPassReceiver method of |compositor_|. - mojo::PendingReceiver<mojom::PaintPreviewCompositor> - BindNewPipeAndPassReceiver(); + // The returned remote should only be used on `compositor_task_runner_`. + mojo::Remote<mojom::PaintPreviewCompositor>* GetCompositor() { + return compositor_.get(); + } void IsBoundAndConnected(base::OnceCallback<void(bool)> callback); diff --git a/chromium/components/paint_preview/browser/paint_preview_compositor_service_impl.cc b/chromium/components/paint_preview/browser/paint_preview_compositor_service_impl.cc index f1489ddc24f..394cf2b4406 100644 --- a/chromium/components/paint_preview/browser/paint_preview_compositor_service_impl.cc +++ b/chromium/components/paint_preview/browser/paint_preview_compositor_service_impl.cc @@ -4,6 +4,7 @@ #include "components/paint_preview/browser/paint_preview_compositor_service_impl.h" +#include "base/bind_post_task.h" #include "base/callback.h" #include "components/paint_preview/browser/compositor_utils.h" #include "components/paint_preview/browser/paint_preview_compositor_client_impl.h" @@ -11,21 +12,6 @@ namespace paint_preview { -namespace { - -base::OnceClosure BindToTaskRunner( - scoped_refptr<base::SequencedTaskRunner> task_runner, - base::OnceClosure closure) { - return base::BindOnce( - [](scoped_refptr<base::SequencedTaskRunner> task_runner, - base::OnceClosure closure) { - task_runner->PostTask(FROM_HERE, std::move(closure)); - }, - task_runner, std::move(closure)); -} - -} // namespace - PaintPreviewCompositorServiceImpl::PaintPreviewCompositorServiceImpl( mojo::PendingRemote<mojom::PaintPreviewCompositorCollection> pending_remote, scoped_refptr<base::SequencedTaskRunner> compositor_task_runner_, @@ -48,7 +34,7 @@ PaintPreviewCompositorServiceImpl::PaintPreviewCompositorServiceImpl( remote->set_disconnect_handler(std::move(disconnect_closure)); }, compositor_service_.get(), std::move(pending_remote), - BindToTaskRunner( + base::BindPostTask( default_task_runner_, base::BindOnce( &PaintPreviewCompositorServiceImpl::DisconnectHandler, @@ -75,7 +61,7 @@ PaintPreviewCompositorServiceImpl::CreateCompositor( FROM_HERE, base::BindOnce( [](mojo::Remote<mojom::PaintPreviewCompositorCollection>* remote, - PaintPreviewCompositorClientImpl* compositor, + mojo::Remote<mojom::PaintPreviewCompositor>* compositor, base::OnceCallback<void(const base::UnguessableToken&)> on_connected) { // This binds the remote in compositor to the @@ -84,7 +70,10 @@ PaintPreviewCompositorServiceImpl::CreateCompositor( compositor->BindNewPipeAndPassReceiver(), std::move(on_connected)); }, - compositor_service_.get(), compositor.get(), + // These are both deleted on `compositor_task_runner_` using + // TaskRunnerDeleter and at this point neither can be scheduled for + // deletion so passing raw pointers is safe. + compositor_service_.get(), compositor->GetCompositor(), // This builder ensures the callback it returns is called on the // correct sequence. compositor->BuildCompositorCreatedCallback( diff --git a/chromium/components/paint_preview/browser/paint_preview_file_mixin.cc b/chromium/components/paint_preview/browser/paint_preview_file_mixin.cc new file mode 100644 index 00000000000..3ad349dc92f --- /dev/null +++ b/chromium/components/paint_preview/browser/paint_preview_file_mixin.cc @@ -0,0 +1,136 @@ +// Copyright 2020 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 "components/paint_preview/browser/paint_preview_file_mixin.h" + +#include <utility> + +#include "base/files/file_util.h" +#include "base/task/task_traits.h" +#include "base/task/thread_pool.h" +#include "ui/accessibility/mojom/ax_tree_update.mojom.h" + +namespace paint_preview { + +namespace { + +const char kPaintPreviewDir[] = "paint_preview"; +const char kAxTreeFilename[] = "ax_tree.message"; + +} // namespace + +PaintPreviewFileMixin::PaintPreviewFileMixin( + const base::FilePath& path, + base::StringPiece ascii_feature_name) + : task_runner_(base::ThreadPool::CreateSequencedTaskRunner( + {base::MayBlock(), base::TaskPriority::USER_VISIBLE, + base::TaskShutdownBehavior::BLOCK_SHUTDOWN, + base::ThreadPolicy::MUST_USE_FOREGROUND})), + file_manager_(base::MakeRefCounted<FileManager>( + path.AppendASCII(kPaintPreviewDir).AppendASCII(ascii_feature_name), + task_runner_)) {} + +PaintPreviewFileMixin::~PaintPreviewFileMixin() = default; + +void PaintPreviewFileMixin::GetCapturedPaintPreviewProto( + const DirectoryKey& key, + base::Optional<base::TimeDelta> expiry_horizon, + OnReadProtoCallback on_read_proto_callback) { + task_runner_->PostTaskAndReplyWithResult( + FROM_HERE, + base::BindOnce( + [](scoped_refptr<FileManager> file_manager, const DirectoryKey& key, + base::Optional<base::TimeDelta> expiry_horizon) + -> std::pair<PaintPreviewFileMixin::ProtoReadStatus, + std::unique_ptr<PaintPreviewProto>> { + if (expiry_horizon.has_value()) { + auto file_info = file_manager->GetInfo(key); + if (!file_info.has_value()) + return std::make_pair(ProtoReadStatus::kNoProto, nullptr); + + if (file_info->last_modified + expiry_horizon.value() < + base::Time::NowFromSystemTime()) { + return std::make_pair(ProtoReadStatus::kExpired, nullptr); + } + } + auto result = file_manager->DeserializePaintPreviewProto(key); + PaintPreviewFileMixin::ProtoReadStatus status = + ProtoReadStatus::kNoProto; + switch (result.first) { + case FileManager::ProtoReadStatus::kOk: + status = ProtoReadStatus::kOk; + break; + case FileManager::ProtoReadStatus::kNoProto: + status = ProtoReadStatus::kNoProto; + break; + case FileManager::ProtoReadStatus::kDeserializationError: + status = ProtoReadStatus::kDeserializationError; + break; + default: + NOTREACHED(); + } + return std::make_pair(status, std::move(result.second)); + }, + file_manager_, key, expiry_horizon), + base::BindOnce( + [](OnReadProtoCallback callback, + std::pair<PaintPreviewFileMixin::ProtoReadStatus, + std::unique_ptr<PaintPreviewProto>> result) { + std::move(callback).Run(result.first, std::move(result.second)); + }, + std::move(on_read_proto_callback))); +} + +void PaintPreviewFileMixin::WriteAXTreeUpdate( + const DirectoryKey& key, + base::OnceCallback<void(bool)> finished_callback, + const ui::AXTreeUpdate& ax_tree_update) { + std::vector<uint8_t> ax_data = + ax::mojom::AXTreeUpdate::Serialize(&ax_tree_update); + task_runner_->PostTaskAndReplyWithResult( + FROM_HERE, + base::BindOnce( + [](scoped_refptr<FileManager> file_manager, const DirectoryKey& key, + const std::vector<uint8_t>& ax_data) { + auto directory = file_manager->CreateOrGetDirectory(key, false); + if (!directory.has_value()) { + return false; + } + return base::WriteFile(directory->AppendASCII(kAxTreeFilename), + ax_data); + }, + file_manager_, key, ax_data), + std::move(finished_callback)); +} + +void PaintPreviewFileMixin::GetAXTreeUpdate(const DirectoryKey& key, + OnReadAXTree callback) { + task_runner_->PostTaskAndReplyWithResult( + FROM_HERE, + base::BindOnce( + [](scoped_refptr<FileManager> file_manager, + const DirectoryKey& key) -> std::unique_ptr<ui::AXTreeUpdate> { + auto dir = file_manager->CreateOrGetDirectory(key, false); + if (!dir.has_value()) { + return nullptr; + } + + auto path = dir->AppendASCII(kAxTreeFilename); + std::string content; + if (!base::ReadFileToString(path, &content)) { + return nullptr; + } + + auto update = std::make_unique<ui::AXTreeUpdate>(); + if (!ax::mojom::AXTreeUpdate::Deserialize( + content.data(), content.size(), update.get())) { + return nullptr; + } + return update; + }, + file_manager_, key), + std::move(callback)); +} + +} // namespace paint_preview diff --git a/chromium/components/paint_preview/browser/paint_preview_file_mixin.h b/chromium/components/paint_preview/browser/paint_preview_file_mixin.h new file mode 100644 index 00000000000..0af1317ccd7 --- /dev/null +++ b/chromium/components/paint_preview/browser/paint_preview_file_mixin.h @@ -0,0 +1,79 @@ +// Copyright 2020 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 COMPONENTS_PAINT_PREVIEW_BROWSER_PAINT_PREVIEW_FILE_MIXIN_H_ +#define COMPONENTS_PAINT_PREVIEW_BROWSER_PAINT_PREVIEW_FILE_MIXIN_H_ + +#include "base/callback.h" +#include "base/files/file_path.h" +#include "base/memory/weak_ptr.h" +#include "components/keyed_service/core/keyed_service.h" +#include "components/paint_preview/browser/file_manager.h" +#include "ui/accessibility/ax_tree_update.h" + +namespace paint_preview { + +class PaintPreviewFileMixin { + public: + enum class ProtoReadStatus : int { + kOk = 0, + kNoProto, + kDeserializationError, + kExpired, + }; + + using OnReadProtoCallback = + base::OnceCallback<void(ProtoReadStatus, + std::unique_ptr<PaintPreviewProto>)>; + + using OnReadAXTree = + base::OnceCallback<void(std::unique_ptr<ui::AXTreeUpdate>)>; + + // Creates an instance for a profile. FileManager's root directory will be set + // to |profile_dir|/paint_preview/|ascii_feature_name|. + PaintPreviewFileMixin(const base::FilePath& profile_dir, + base::StringPiece ascii_feature_name); + PaintPreviewFileMixin(const PaintPreviewFileMixin&) = delete; + PaintPreviewFileMixin& operator=(const PaintPreviewFileMixin&) = delete; + virtual ~PaintPreviewFileMixin(); + + // Returns the file manager for the directory associated with the profile. + scoped_refptr<FileManager> GetFileManager() { return file_manager_; } + + // Returns the task runner that IO tasks should be scheduled on. + scoped_refptr<base::SequencedTaskRunner> GetTaskRunner() { + return task_runner_; + } + + base::WeakPtr<PaintPreviewFileMixin> GetWeakPtr() { + return weak_ptr_factory_.GetWeakPtr(); + } + + // Acquires the PaintPreviewProto that is associated with |key| and sends it + // to |on_read_proto_callback|. The default implementation attempts to invoke + // GetFileManager()->DeserializePaintPreviewProto(). If |expiry_horizon| is + // provided a proto that was last modified earlier than 'now - expiry_horizon' + // will return the kExpired status. + virtual void GetCapturedPaintPreviewProto( + const DirectoryKey& key, + base::Optional<base::TimeDelta> expiry_horizon, + OnReadProtoCallback on_read_proto_callback); + + // Writes an Accessibility Tree snapshot to the directory listed in key. + void WriteAXTreeUpdate(const DirectoryKey& key, + base::OnceCallback<void(bool)> finished_callback, + const ui::AXTreeUpdate& ax_tree_update); + + // Gets an Accessibility Tree snapshot for key. + void GetAXTreeUpdate(const DirectoryKey& key, OnReadAXTree callback); + + private: + scoped_refptr<base::SequencedTaskRunner> task_runner_; + scoped_refptr<FileManager> file_manager_; + base::WeakPtrFactory<PaintPreviewFileMixin> weak_ptr_factory_{this}; +}; + +} // namespace paint_preview + +#endif // COMPONENTS_PAINT_PREVIEW_BROWSER_PAINT_PREVIEW_FILE_HELPER_H_ diff --git a/chromium/components/paint_preview/common/BUILD.gn b/chromium/components/paint_preview/common/BUILD.gn index 283ad37ee9c..ed85de2f1a8 100644 --- a/chromium/components/paint_preview/common/BUILD.gn +++ b/chromium/components/paint_preview/common/BUILD.gn @@ -4,92 +4,95 @@ import("//testing/test.gni") -if (!is_ios) { - static_library("common") { - sources = [ - "capture_result.cc", - "capture_result.h", - "file_stream.cc", - "file_stream.h", - "file_utils.cc", - "file_utils.h", - "glyph_usage.cc", - "glyph_usage.h", - "paint_preview_tracker.cc", - "paint_preview_tracker.h", - "recording_map.cc", - "recording_map.h", - "serial_utils.cc", - "serial_utils.h", - "serialized_recording.cc", - "serialized_recording.h", - "subset_font.cc", - "subset_font.h", - "version.cc", - "version.h", - ] +assert(!is_ios, "Paint Previews are not supported on iOS.") - deps = [ - "//base", - "//skia", - "//third_party:freetype_harfbuzz", - "//third_party/harfbuzz-ng:hb_scoped_util", - "//ui/gfx/geometry", - "//url", - ] +source_set("common") { + sources = [ + "capture_result.cc", + "capture_result.h", + "file_stream.cc", + "file_stream.h", + "file_utils.cc", + "file_utils.h", + "glyph_usage.cc", + "glyph_usage.h", + "paint_preview_tracker.cc", + "paint_preview_tracker.h", + "proto_validator.cc", + "proto_validator.h", + "recording_map.cc", + "recording_map.h", + "serial_utils.cc", + "serial_utils.h", + "serialized_recording.cc", + "serialized_recording.h", + "subset_font.cc", + "subset_font.h", + "version.cc", + "version.h", + ] - public_deps = [ - "//components/paint_preview/common/mojom", - "//components/paint_preview/common/proto", - ] - } + deps = [ + "//base", + "//skia", + "//third_party:freetype_harfbuzz", + "//third_party/harfbuzz-ng:hb_scoped_util", + "//ui/gfx/geometry", + "//url", + ] - source_set("test_utils") { - testonly = true + public_deps = [ + "//components/paint_preview/common/mojom", + "//components/paint_preview/common/proto", + ] +} + +source_set("test_utils") { + testonly = true - sources = [ - "test_utils.cc", - "test_utils.h", - ] + sources = [ + "test_utils.cc", + "test_utils.h", + ] - deps = [ - "//components/paint_preview/common", - "//testing/gmock", - "//testing/gtest", - ] - } + deps = [ + "//components/paint_preview/common", + "//testing/gmock", + "//testing/gtest", + ] +} - source_set("unit_tests") { - testonly = true +source_set("unit_tests") { + testonly = true - sources = [ - "file_stream_unittest.cc", - "glyph_usage_unittest.cc", - "paint_preview_tracker_unittest.cc", - "serial_utils_unittest.cc", - "serialized_recording_unittest.cc", - "subset_font_unittest.cc", - ] + sources = [ + "file_stream_unittest.cc", + "glyph_usage_unittest.cc", + "paint_preview_tracker_unittest.cc", + "proto_validator_unittest.cc", + "serial_utils_unittest.cc", + "serialized_recording_unittest.cc", + "subset_font_unittest.cc", + ] - deps = [ - ":common", - ":test_utils", - "//base", - "//base/test:test_support", - "//skia", - "//testing/gmock", - "//testing/gtest", - "//third_party:freetype_harfbuzz", - "//ui/gfx/geometry", - ] - } + deps = [ + ":common", + ":test_utils", + "//base", + "//base/test:test_support", + "//skia", + "//testing/gmock", + "//testing/gtest", + "//third_party:freetype_harfbuzz", + "//ui/gfx/geometry", + ] +} - test("paint_preview_common_unit_tests") { - deps = [ - ":unit_tests", - "//base", - "//base/test:test_support", - "//components/test:run_all_unittests", - ] - } +test("paint_preview_common_unit_tests") { + deps = [ + ":unit_tests", + "//base", + "//base/test:test_support", + "//components/test:run_all_unittests", + ] } diff --git a/chromium/components/paint_preview/common/paint_preview_tracker.cc b/chromium/components/paint_preview/common/paint_preview_tracker.cc index 66bbe4c3bec..4d87b57912c 100644 --- a/chromium/components/paint_preview/common/paint_preview_tracker.cc +++ b/chromium/components/paint_preview/common/paint_preview_tracker.cc @@ -10,7 +10,7 @@ #include <utility> #include "base/check.h" -#include "base/stl_util.h" +#include "base/containers/contains.h" #include "components/paint_preview/common/glyph_usage.h" #include "components/paint_preview/common/mojom/paint_preview_recorder.mojom.h" #include "third_party/skia/include/core/SkCanvas.h" diff --git a/chromium/components/paint_preview/common/paint_preview_tracker.h b/chromium/components/paint_preview/common/paint_preview_tracker.h index 8160031f738..2d3f5fcce5f 100644 --- a/chromium/components/paint_preview/common/paint_preview_tracker.h +++ b/chromium/components/paint_preview/common/paint_preview_tracker.h @@ -112,6 +112,7 @@ class PaintPreviewTracker { const base::Optional<base::UnguessableToken> embedding_token_; const bool is_main_frame_; + // TODO(crbug.com/1155544): Change this to an SkM44. SkMatrix matrix_; std::vector<SkMatrix> states_; diff --git a/chromium/components/paint_preview/common/proto/BUILD.gn b/chromium/components/paint_preview/common/proto/BUILD.gn index 2694d4489e4..53141b92cb3 100644 --- a/chromium/components/paint_preview/common/proto/BUILD.gn +++ b/chromium/components/paint_preview/common/proto/BUILD.gn @@ -7,3 +7,12 @@ import("//third_party/protobuf/proto_library.gni") proto_library("proto") { sources = [ "paint_preview.proto" ] } + +if (is_android) { + import("//build/config/android/rules.gni") + + proto_java_library("proto_java") { + proto_path = "." + sources = [ "paint_preview.proto" ] + } +} diff --git a/chromium/components/paint_preview/common/proto/paint_preview.proto b/chromium/components/paint_preview/common/proto/paint_preview.proto index 84dda9b1f3d..4a5ac533955 100644 --- a/chromium/components/paint_preview/common/proto/paint_preview.proto +++ b/chromium/components/paint_preview/common/proto/paint_preview.proto @@ -7,29 +7,42 @@ syntax = "proto2"; package paint_preview; option optimize_for = LITE_RUNTIME; +option java_package = "org.chromium.components.paint_preview.common.proto"; + +// Any fields annotated with "// required" are enforced as if they are +// required by paint_preview::PaintPreviewProtoValid(). // A proto representation of a gfx::Rect. // NEXT_TAG = 5 message RectProto { - required int64 x = 1; - required int64 y = 2; - required int64 width = 3; - required int64 height = 4; + // required + optional int64 x = 1; + // required + optional int64 y = 2; + // required + optional int64 width = 3; + // required + optional int64 height = 4; } // A link represented by its absolute URL and a bounding box for the hit area. // NEXT_TAG = 3 message LinkDataProto { - required RectProto rect = 1; - required string url = 2; + // required + optional RectProto rect = 1; + // required + optional string url = 2; } // A mapping from a content ID to the serialized embedding token. // NEXT_TAG = 4 message ContentIdEmbeddingTokenPairProto { - required uint32 content_id = 1; - required uint64 embedding_token_low = 2; - required uint64 embedding_token_high = 3; + // required + optional uint32 content_id = 1; + // required + optional uint64 embedding_token_low = 2; + // required + optional uint64 embedding_token_high = 3; } // A paint preview of a single frame. @@ -37,11 +50,14 @@ message ContentIdEmbeddingTokenPairProto { message PaintPreviewFrameProto { // The embedding token for this frame to its parent. Every frame other than // the main frame should have a non-zero value here. - required uint64 embedding_token_low = 1; - required uint64 embedding_token_high = 2; + // required + optional uint64 embedding_token_low = 1; + // required + optional uint64 embedding_token_high = 2; // Boolean indicating if the frame is the main frame. - required bool is_main_frame = 3; + // required + optional bool is_main_frame = 3; // The file path to the serialized Skia Picture. // null if the persistence type of the |PaintPreviewCaptureParams| is @@ -59,16 +75,29 @@ message PaintPreviewFrameProto { optional uint32 scroll_offset_y = 8; } +// Stores Chrome version. +// NEXT_TAG = 5 +message ChromeVersionProto { + optional uint64 major = 1; + optional uint64 minor = 2; + optional uint64 build = 3; + optional uint64 patch = 4; +} + // Metadata for the capture. -// NEXT_TAG = 3 +// NEXT_TAG = 4 message MetadataProto { // URL of the root frame. - required string url = 1; + // required + optional string url = 1; // Records the version number of the recording. Should be incremented if there // is a breaking change to the custorm SkPicture deserialization or storage // system. optional uint64 version = 2; + + // Records the version of Chrome when the capture occurred. + optional ChromeVersionProto chrome_version = 3; } // A paint preview of the entire page. @@ -76,7 +105,9 @@ message MetadataProto { message PaintPreviewProto { // The root frame of the RenderFrame tree. This is often the main frame, but // may be a root node of a subtree (e.g. paint preview of an iframe). - required PaintPreviewFrameProto root_frame = 1; + // required + optional PaintPreviewFrameProto root_frame = 1; repeated PaintPreviewFrameProto subframes = 2; - required MetadataProto metadata = 3; + // required + optional MetadataProto metadata = 3; } diff --git a/chromium/components/paint_preview/common/proto_validator.cc b/chromium/components/paint_preview/common/proto_validator.cc new file mode 100644 index 00000000000..da356f0a9ea --- /dev/null +++ b/chromium/components/paint_preview/common/proto_validator.cc @@ -0,0 +1,71 @@ +// Copyright 2020 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 "components/paint_preview/common/proto_validator.h" + +namespace paint_preview { + +namespace { + +bool RectProtoValid(const RectProto& rect) { + return rect.has_x() && rect.has_y() && rect.has_width() && rect.has_height(); +} + +bool LinkDataProtoValid(const LinkDataProto& link_data) { + return link_data.has_rect() && link_data.has_url() && + RectProtoValid(link_data.rect()); +} + +bool ContentIdEmbeddingTokenPairProtoValid( + const ContentIdEmbeddingTokenPairProto& content_id_embedding_token_pair) { + return content_id_embedding_token_pair.has_content_id() && + content_id_embedding_token_pair.has_embedding_token_low() && + content_id_embedding_token_pair.has_embedding_token_high(); +} + +bool PaintPreviewFrameProtoValid( + const PaintPreviewFrameProto& paint_preview_frame) { + if (!paint_preview_frame.has_embedding_token_low() || + !paint_preview_frame.has_embedding_token_high() || + !paint_preview_frame.has_is_main_frame()) { + return false; + } + + for (const LinkDataProto& link_data : paint_preview_frame.links()) { + if (!LinkDataProtoValid(link_data)) { + return false; + } + } + + for (const ContentIdEmbeddingTokenPairProto& content_id_embedding_token_pair : + paint_preview_frame.content_id_to_embedding_tokens()) { + if (!ContentIdEmbeddingTokenPairProtoValid( + content_id_embedding_token_pair)) { + return false; + } + } + return true; +} + +bool MetadataProtoValid(const MetadataProto& metadata) { + return metadata.has_url(); +} + +} // namespace + +bool PaintPreviewProtoValid(const PaintPreviewProto& paint_preview) { + if (!paint_preview.has_metadata() || !paint_preview.has_root_frame() || + !MetadataProtoValid(paint_preview.metadata()) || + !PaintPreviewFrameProtoValid(paint_preview.root_frame())) { + return false; + } + for (const PaintPreviewFrameProto& subframe : paint_preview.subframes()) { + if (!PaintPreviewFrameProtoValid(subframe)) { + return false; + } + } + return true; +} + +} // namespace paint_preview diff --git a/chromium/components/paint_preview/common/proto_validator.h b/chromium/components/paint_preview/common/proto_validator.h new file mode 100644 index 00000000000..da3f1ea788b --- /dev/null +++ b/chromium/components/paint_preview/common/proto_validator.h @@ -0,0 +1,19 @@ +// Copyright 2020 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 COMPONENTS_PAINT_PREVIEW_COMMON_PROTO_VALIDATOR_H_ +#define COMPONENTS_PAINT_PREVIEW_COMMON_PROTO_VALIDATOR_H_ + +#include "components/paint_preview/common/proto/paint_preview.pb.h" + +namespace paint_preview { + +// Verifies that all the expected fields of `paint_preview` are present. If this +// returns true, downstream callers can treat all fields in the proto that are +// stated to be required in paint_preview.proto to be present. +bool PaintPreviewProtoValid(const PaintPreviewProto& paint_preview); + +} // namespace paint_preview + +#endif // COMPONENTS_PAINT_PREVIEW_COMMON_PROTO_VALIDATOR_H_ diff --git a/chromium/components/paint_preview/common/proto_validator_unittest.cc b/chromium/components/paint_preview/common/proto_validator_unittest.cc new file mode 100644 index 00000000000..91036cd42ac --- /dev/null +++ b/chromium/components/paint_preview/common/proto_validator_unittest.cc @@ -0,0 +1,173 @@ +// Copyright 2020 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 "components/paint_preview/common/proto_validator.h" + +#include "components/paint_preview/common/proto/paint_preview.pb.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace paint_preview { + +namespace { + +// Generate a fully populated valid proto. +PaintPreviewProto CreatePopulatedValidProto() { + PaintPreviewProto proto; + + auto* metadata = proto.mutable_metadata(); + metadata->set_url("https://www.example.com/"); + metadata->set_version(1); + + auto* root_frame = proto.mutable_root_frame(); + root_frame->set_embedding_token_low(12345); + root_frame->set_embedding_token_high(67890); + root_frame->set_is_main_frame(true); + root_frame->set_scroll_offset_x(50); + root_frame->set_scroll_offset_y(100); + root_frame->set_file_path("/foo/bar"); + + auto* link = root_frame->add_links(); + link->set_url("https://www.chromium.org/"); + + auto* rect = link->mutable_rect(); + rect->set_x(1); + rect->set_y(2); + rect->set_width(3); + rect->set_height(4); + + const int subframe_low = 654; + const int subframe_high = 7321981; + auto* pair = root_frame->add_content_id_to_embedding_tokens(); + pair->set_content_id(1); + pair->set_embedding_token_low(subframe_low); + pair->set_embedding_token_high(subframe_high); + + auto* subframe = proto.add_subframes(); + subframe->set_embedding_token_low(subframe_low); + subframe->set_embedding_token_high(subframe_high); + subframe->set_is_main_frame(false); + return proto; +} + +} // namespace + +// Rect validation fails if the rect is missing any dimension. +TEST(PaintPreviewProtoValid, RectValidation) { + auto proto = CreatePopulatedValidProto(); + auto* root_frame = proto.mutable_root_frame(); + auto* link_data = root_frame->mutable_links(0); + auto* rect = link_data->mutable_rect(); + rect->clear_x(); + EXPECT_FALSE(PaintPreviewProtoValid(proto)); + + proto = CreatePopulatedValidProto(); + root_frame = proto.mutable_root_frame(); + link_data = root_frame->mutable_links(0); + rect = link_data->mutable_rect(); + rect->clear_y(); + EXPECT_FALSE(PaintPreviewProtoValid(proto)); + + proto = CreatePopulatedValidProto(); + root_frame = proto.mutable_root_frame(); + link_data = root_frame->mutable_links(0); + rect = link_data->mutable_rect(); + rect->clear_width(); + EXPECT_FALSE(PaintPreviewProtoValid(proto)); + + proto = CreatePopulatedValidProto(); + root_frame = proto.mutable_root_frame(); + link_data = root_frame->mutable_links(0); + rect = link_data->mutable_rect(); + rect->clear_height(); + EXPECT_FALSE(PaintPreviewProtoValid(proto)); +} + +// Link validation fails if it is missing a url or rect. +TEST(PaintPreviewProtoValid, LinkDataValidation) { + auto proto = CreatePopulatedValidProto(); + auto* root_frame = proto.mutable_root_frame(); + auto* link_data = root_frame->mutable_links(0); + link_data->clear_url(); + EXPECT_FALSE(PaintPreviewProtoValid(proto)); + + proto = CreatePopulatedValidProto(); + root_frame = proto.mutable_root_frame(); + link_data = root_frame->mutable_links(0); + link_data->clear_rect(); + EXPECT_FALSE(PaintPreviewProtoValid(proto)); +} + +// Content ID and embedding token pair validation should fail if any field is +// missing. +TEST(PaintPreviewProtoValid, ContentIdEmbeddingTokenPairValidation) { + auto proto = CreatePopulatedValidProto(); + auto* root_frame = proto.mutable_root_frame(); + auto* pair = root_frame->mutable_content_id_to_embedding_tokens(0); + pair->clear_content_id(); + EXPECT_FALSE(PaintPreviewProtoValid(proto)); + + proto = CreatePopulatedValidProto(); + root_frame = proto.mutable_root_frame(); + pair = root_frame->mutable_content_id_to_embedding_tokens(0); + pair->clear_embedding_token_low(); + EXPECT_FALSE(PaintPreviewProtoValid(proto)); + + proto = CreatePopulatedValidProto(); + root_frame = proto.mutable_root_frame(); + pair = root_frame->mutable_content_id_to_embedding_tokens(0); + pair->clear_embedding_token_high(); + EXPECT_FALSE(PaintPreviewProtoValid(proto)); +} + +// Frame validation should fail if the embedding token or main frame fields are +// missing. +TEST(PaintPreviewProtoValid, PaintPreviewFrameProtoValidation) { + auto proto = CreatePopulatedValidProto(); + auto* root_frame = proto.mutable_root_frame(); + root_frame->clear_embedding_token_low(); + EXPECT_FALSE(PaintPreviewProtoValid(proto)); + + proto = CreatePopulatedValidProto(); + root_frame = proto.mutable_root_frame(); + root_frame->clear_embedding_token_high(); + EXPECT_FALSE(PaintPreviewProtoValid(proto)); + + proto = CreatePopulatedValidProto(); + root_frame = proto.mutable_root_frame(); + root_frame->clear_embedding_token_high(); + EXPECT_FALSE(PaintPreviewProtoValid(proto)); + + proto = CreatePopulatedValidProto(); + root_frame = proto.mutable_root_frame(); + root_frame->clear_is_main_frame(); + EXPECT_FALSE(PaintPreviewProtoValid(proto)); +} + +// Metadata validation fails if the url is missing. +TEST(PaintPreviewProtoValid, MetadataValidation) { + auto proto = CreatePopulatedValidProto(); + auto* metadata = proto.mutable_metadata(); + metadata->clear_url(); + EXPECT_FALSE(PaintPreviewProtoValid(proto)); +} + +// Proto validation should succeed unless the root frame, or metadata is missing +// or if a subframe is invalid. +TEST(PaintPreviewProtoValid, PaintPreviewValidation) { + auto proto = CreatePopulatedValidProto(); + EXPECT_TRUE(PaintPreviewProtoValid(proto)); + + proto.clear_metadata(); + EXPECT_FALSE(PaintPreviewProtoValid(proto)); + + proto = CreatePopulatedValidProto(); + proto.clear_root_frame(); + EXPECT_FALSE(PaintPreviewProtoValid(proto)); + + proto = CreatePopulatedValidProto(); + proto.add_subframes(); + EXPECT_FALSE(PaintPreviewProtoValid(proto)); +} + +} // namespace paint_preview diff --git a/chromium/components/paint_preview/common/recording_map.cc b/chromium/components/paint_preview/common/recording_map.cc index 44e60aca0aa..aa3073e8726 100644 --- a/chromium/components/paint_preview/common/recording_map.cc +++ b/chromium/components/paint_preview/common/recording_map.cc @@ -15,7 +15,7 @@ namespace { base::FilePath ToFilePath(base::StringPiece path_str) { #if defined(OS_WIN) - return base::FilePath(base::UTF8ToUTF16(path_str)); + return base::FilePath(base::UTF8ToWide(path_str)); #else return base::FilePath(path_str); #endif diff --git a/chromium/components/paint_preview/common/serial_utils.cc b/chromium/components/paint_preview/common/serial_utils.cc index 8fd6f25be7c..e614199ba67 100644 --- a/chromium/components/paint_preview/common/serial_utils.cc +++ b/chromium/components/paint_preview/common/serial_utils.cc @@ -76,6 +76,9 @@ sk_sp<SkData> SerializeTypeface(SkTypeface* typeface, void* ctx) { sk_sp<SkData> SerializeImage(SkImage* image, void* ctx) { ImageSerializationContext* context = reinterpret_cast<ImageSerializationContext*>(ctx); + if (image->isTextureBacked()) { + return SkData::MakeEmpty(); + } const SkImageInfo& image_info = image->imageInfo(); // If decoding/encoding the image would result in it exceeding the allowable diff --git a/chromium/components/paint_preview/common/serial_utils_unittest.cc b/chromium/components/paint_preview/common/serial_utils_unittest.cc index f249210e018..35410e9f253 100644 --- a/chromium/components/paint_preview/common/serial_utils_unittest.cc +++ b/chromium/components/paint_preview/common/serial_utils_unittest.cc @@ -146,9 +146,9 @@ TEST(PaintPreviewSerialUtils, TestImageContextLimitBudget) { canvas1.drawRect(SkRect::MakeWH(1, 4), paint); SkPictureRecorder recorder; SkCanvas* canvas = recorder.beginRecording(SkRect::MakeWH(40, 40)); - canvas->drawBitmap(bitmap1, 0, 0); - canvas->drawBitmap(bitmap1, 0, 0); - canvas->drawBitmap(bitmap1, 0, 0); + canvas->drawImage(bitmap1.asImage(), 0, 0); + canvas->drawImage(bitmap1.asImage(), 0, 0); + canvas->drawImage(bitmap1.asImage(), 0, 0); auto pic = recorder.finishRecordingAsPicture(); PictureSerializationContext picture_ctx; @@ -194,8 +194,8 @@ TEST(PaintPreviewSerialUtils, TestImageContextLimitSize) { canvas2.drawRect(SkRect::MakeWH(20, 5), paint); SkPictureRecorder recorder; SkCanvas* canvas = recorder.beginRecording(SkRect::MakeWH(40, 40)); - canvas->drawBitmap(bitmap1, 0, 0); - canvas->drawBitmap(bitmap2, 0, 0); + canvas->drawImage(bitmap1.asImage(), 0, 0); + canvas->drawImage(bitmap2.asImage(), 0, 0); auto pic = recorder.finishRecordingAsPicture(); PictureSerializationContext picture_ctx; diff --git a/chromium/components/paint_preview/player/BUILD.gn b/chromium/components/paint_preview/player/BUILD.gn index 64ed62f7efb..ef4e090c163 100644 --- a/chromium/components/paint_preview/player/BUILD.gn +++ b/chromium/components/paint_preview/player/BUILD.gn @@ -2,6 +2,8 @@ # Use of this source code is governed by a BSD - style license that can be # found in the LICENSE file. +import("//build/util/version.gni") + source_set("player") { sources = [ "bitmap_request.cc", @@ -18,12 +20,20 @@ source_set("player") { "//components/paint_preview/common/proto", "//components/paint_preview/public", "//mojo/public/cpp/bindings", + "//ui/accessibility", "//ui/gfx/geometry", "//url", ] public_deps = [ "//components/services/paint_preview_compositor/public/mojom" ] + + defines = [ + "CHROME_VERSION_MAJOR=" + chrome_version_major, + "CHROME_VERSION_MINOR=" + chrome_version_minor, + "CHROME_VERSION_BUILD=" + chrome_version_build, + "CHROME_VERSION_PATCH=" + chrome_version_patch, + ] } source_set("unit_tests") { diff --git a/chromium/components/paint_preview/player/README.md b/chromium/components/paint_preview/player/README.md new file mode 100644 index 00000000000..142ff182cf5 --- /dev/null +++ b/chromium/components/paint_preview/player/README.md @@ -0,0 +1,49 @@ +# Paint Preview Player + +The player displays a paint preview that has been previously recorded. +Currently, the player is only fully implemented for Android. However, there +are a few platform-independent base classes ([`PlayerCompositorDelegate`](https://source.chromium.org/chromium/chromium/src/+/master:/components/paint_preview/player/player_compositor_delegate.cc) +, [`CompositorStatus`](https://source.chromium.org/chromium/chromium/src/+/master:/components/paint_preview/player/compositor_status.h) +, [`BitmapRequest `](https://source.chromium.org/chromium/chromium/src/+/master:/components/paint_preview/player/bitmap_request.cc) +) than can be used to extend the playback support for other platforms. + +`PlayerCompositorDelegate` uses the [StartCompositorService](https://source.chromium.org/chromium/chromium/src/+/master:components/paint_preview/browser/compositor_utils.h;bpv=1;bpt=1;l=16?q=StartCompositorService&ss=chromium&gsn=StartCompositorService&gs=kythe%3A%2F%2Fchromium.googlesource.com%2Fchromium%2Fsrc%3Flang%3Dc%252B%252B%3Fpath%3Dsrc%2Fcomponents%2Fpaint_preview%2Fbrowser%2Fcompositor_utils.h%23E_S97S1y6_-ukZj8vR6XZUzOTYFPwOkwR0LybvqxVWg) +API. Alternatively, playback support for other platforms can be provided by +using `StartCompositorService` directly for more flexibility. + +The remainder of this doc describes the Android-specific implementation. + +## TL;DR + +Want to use the player? Construct a [`PlayerManager`](https://source.chromium.org/chromium/chromium/src/+/master:/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/PlayerManager.java) +and use [`PlayerManager#getView`](https://source.chromium.org/chromium/chromium/src/+/master:components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/PlayerManager.java;drc=c62b2799aeef847ccb9fbd5d1fc3e19e2938df9a;l=240) +to display it. + +## Design + +As mentioned in the main [`README`](https://source.chromium.org/chromium/chromium/src/+/master:/components/paint_preview/README.md) +, a paint preview can have multiple frames in a nested structure. +Consequently, the player is desinged in a nested structure as well, to +facilitate the display of mulitple nested frames. + + +* `android/java/src/.../player/`: This directory contains per-player classes. +In another word, these classes are aware that the player might have multiple +frames and are not involved in the logic for displaying a single frame. +* `android/java/src/.../player/frame`: This directory contains per-frame +classes. These are responsible for displaying a single frame. + +### Important classes + +* [`PlayerManager`](https://source.chromium.org/chromium/chromium/src/+/master:/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/PlayerManager.java): +Entry point for using the player. When created it initializes the compositor +and populates a hierarchy of player frames based on the paint preview. +* [`PlayerCompositorDelegateImpl`](https://source.chromium.org/chromium/chromium/src/+/master:/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/PlayerCompositorDelegateImpl.java) +: Communicates with the paint preview compositor. It requests bitmaps from the +comopsitor and provides them to palyer frames. +* [`PlayerFrameMediator`](https://source.chromium.org/chromium/chromium/src/+/master:/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameMediator.java) +: Handles the business logic for a single frame in the player, i.e. +maintaining a viewport, updating its sub-frame visibilities, requesting +bitmaps from the compositor delegate, etc. +* [`PlayerFrameView`](https://source.chromium.org/chromium/chromium/src/+/master:/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameView.java) +: The View that displays a single frame.
\ No newline at end of file diff --git a/chromium/components/paint_preview/player/android/BUILD.gn b/chromium/components/paint_preview/player/android/BUILD.gn index 7ffba697206..e539fb04690 100644 --- a/chromium/components/paint_preview/player/android/BUILD.gn +++ b/chromium/components/paint_preview/player/android/BUILD.gn @@ -85,8 +85,10 @@ android_library("java") { "//base:base_java", "//base:jni_java", "//components/paint_preview/browser/android:java", - "//third_party/android_deps:androidx_annotation_annotation_java", + "//components/paint_preview/common/proto:proto_java", + "//content/public/android:content_java", "//third_party/android_swipe_refresh:android_swipe_refresh_java", + "//third_party/androidx:androidx_annotation_annotation_java", "//ui/android:ui_java", "//ui/android:ui_java_resources", "//url:gurl_java", @@ -109,8 +111,8 @@ android_library("player_java_test_support") { "//base:base_java_test_support", "//base:jni_java", "//components/paint_preview/browser/android:java", - "//components/signin/core/browser/android:java", - "//components/signin/core/browser/android:signin_java_test_support", + "//components/signin/public/android:java", + "//components/signin/public/android:signin_java_test_support", "//content/public/android:content_java", "//content/public/test/android:content_java_test_support", "//third_party/android_support_test_runner:rules_java", @@ -129,11 +131,12 @@ android_library("javatests") { ":player_java_test_support", "//base:base_java", "//base:base_java_test_support", + "//components/paint_preview/common/proto:proto_java", "//content/public/android:content_java", "//content/public/test/android:content_java_test_support", - "//third_party/android_deps:androidx_test_runner_java", "//third_party/android_support_test_runner:rules_java", "//third_party/android_support_test_runner:runner_java", + "//third_party/androidx:androidx_test_runner_java", "//third_party/hamcrest:hamcrest_java", "//third_party/junit", "//third_party/ub-uiautomator:ub_uiautomator_java", @@ -183,7 +186,7 @@ junit_binary("paint_preview_junit_tests") { "//base:base_java", "//base:base_java_test_support", "//base:base_junit_test_support", - "//third_party/android_deps:androidx_annotation_annotation_java", + "//third_party/androidx:androidx_annotation_annotation_java", "//ui/android:ui_full_java", "//url:gurl_java", ] diff --git a/chromium/components/paint_preview/player/android/DEPS b/chromium/components/paint_preview/player/android/DEPS index d21e0729f9c..2ece8a03c82 100644 --- a/chromium/components/paint_preview/player/android/DEPS +++ b/chromium/components/paint_preview/player/android/DEPS @@ -1,3 +1,4 @@ include_rules = [ + "+content/public/android/java/src/org/chromium/content_public", "+ui/android", ] diff --git a/chromium/components/paint_preview/player/android/javatests/paint_preview_test_service.cc b/chromium/components/paint_preview/player/android/javatests/paint_preview_test_service.cc index 33c7190a325..f58536adbea 100644 --- a/chromium/components/paint_preview/player/android/javatests/paint_preview_test_service.cc +++ b/chromium/components/paint_preview/player/android/javatests/paint_preview_test_service.cc @@ -5,6 +5,7 @@ #include "components/paint_preview/player/android/javatests/paint_preview_test_service.h" #include <memory> +#include <utility> #include "base/android/jni_android.h" #include "base/android/jni_array.h" @@ -107,15 +108,20 @@ jlong JNI_PaintPreviewTestService_GetInstance( } PaintPreviewTestService::PaintPreviewTestService(const base::FilePath& path) - : PaintPreviewBaseService(path, - kTestDirName, - std::make_unique<TestPaintPreviewPolicy>(), - false), + : PaintPreviewBaseService( + std::make_unique<PaintPreviewFileMixin>(path, kTestDirName), + std::make_unique<TestPaintPreviewPolicy>(), + false), test_data_dir_( path.AppendASCII(kPaintPreviewDir).AppendASCII(kTestDirName)) {} PaintPreviewTestService::~PaintPreviewTestService() = default; +jlong PaintPreviewTestService::GetBaseService(JNIEnv* env) { + return reinterpret_cast<intptr_t>( + static_cast<PaintPreviewBaseService*>(this)); +} + base::android::ScopedJavaLocalRef<jintArray> PaintPreviewTestService::CreateSingleSkp( JNIEnv* env, diff --git a/chromium/components/paint_preview/player/android/javatests/paint_preview_test_service.h b/chromium/components/paint_preview/player/android/javatests/paint_preview_test_service.h index c6c5b2ac3f4..4b925795251 100644 --- a/chromium/components/paint_preview/player/android/javatests/paint_preview_test_service.h +++ b/chromium/components/paint_preview/player/android/javatests/paint_preview_test_service.h @@ -24,6 +24,8 @@ class PaintPreviewTestService : public PaintPreviewBaseService { PaintPreviewTestService(const PaintPreviewTestService&) = delete; PaintPreviewTestService& operator=(const PaintPreviewTestService&) = delete; + jlong GetBaseService(JNIEnv* env); + base::android::ScopedJavaLocalRef<jintArray> CreateSingleSkp( JNIEnv* env, jint j_id, diff --git a/chromium/components/paint_preview/player/android/javatests/src/org/chromium/components/paintpreview/player/PaintPreviewPlayerTest.java b/chromium/components/paint_preview/player/android/javatests/src/org/chromium/components/paintpreview/player/PaintPreviewPlayerTest.java index ce9a7556662..9dfe0c7f31b 100644 --- a/chromium/components/paint_preview/player/android/javatests/src/org/chromium/components/paintpreview/player/PaintPreviewPlayerTest.java +++ b/chromium/components/paint_preview/player/android/javatests/src/org/chromium/components/paintpreview/player/PaintPreviewPlayerTest.java @@ -5,6 +5,7 @@ package org.chromium.components.paintpreview.player; import android.graphics.Rect; +import android.os.Build; import android.os.Build.VERSION_CODES; import android.support.test.InstrumentationRegistry; import android.support.test.uiautomator.By; @@ -152,21 +153,6 @@ public class PaintPreviewPlayerTest extends DummyUiActivityTestCase { final View playerHostView = mPlayerManager.getView(); assertLinkUrl(playerHostView, 220, 220, TEST_IN_VIEWPORT_LINK_URL); assertLinkUrl(playerHostView, 300, 270, TEST_IN_VIEWPORT_LINK_URL); - - // Temporarily commenting out as this is flaky on P. - - // UiDevice device = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation()); - // int deviceHeight = device.getDisplayHeight(); - // int statusBarHeight = statusBarHeight(); - // int navigationBarHeight = navigationBarHeight(); - // int padding = 20; - // int fromY = deviceHeight - navigationBarHeight - padding; - // int toY = statusBarHeight + padding; - // mLinkClickHandler.mUrl = null; - // device.swipe(300, fromY, 300, toY, 10); - - // Manually click as assertLinkUrl() doesn't handle subframe scrolls well. - // assertLinkUrl(playerHostView, 200, 1500, TEST_OUT_OF_VIEWPORT_LINK_URL); } @Test @@ -227,6 +213,11 @@ public class PaintPreviewPlayerTest extends DummyUiActivityTestCase { public void onLinkClick(GURL url) { mLinkClickHandler.onLinkClicked(url); } + + @Override + public boolean isAccessibilityEnabled() { + return false; + } }, 0xffffffff, false); mPlayerManager.setCompressOnClose(false); }); @@ -235,10 +226,33 @@ public class PaintPreviewPlayerTest extends DummyUiActivityTestCase { private void scaleSmokeTest(boolean multiFrame) throws Exception { initPlayerManager(multiFrame); - UiDevice device = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation()); + final UiDevice device = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation()); - // Query all FrameLayout objects as the PlayerFrameView isn't recognized. - List<UiObject2> objects = device.findObjects(By.clazz("android.widget.FrameLayout")); + device.waitForIdle(); + List<UiObject2> objects = null; + boolean failed = false; + try { + // Query all FrameLayout objects as the PlayerFrameView isn't recognized. + // + // This may throw a NullPointerException when an AccessibilityNodeInfo is unexpectedly + // null on P. It appears to be a bug with null checks inside UiAutomator. However, it + // could be exacerbated were the UI state to change mid-invocation (it is unclear + // why/whether that happens). This occurs < 30% of the time. + objects = device.findObjects(By.clazz("android.widget.FrameLayout")); + } catch (NullPointerException e) { + failed = true; + } + if (failed || objects == null) { + // Ignore NullPointerException failures on P (particularly Pixel 2 ARM on the + // waterfall). + if (Build.VERSION.SDK_INT > VERSION_CODES.O_MR1 + && Build.VERSION.SDK_INT < VERSION_CODES.Q) { + return; + } + + // If this fails on any other configuration it is an unexpected issue. + Assert.fail("UiDevice#findObjects() threw an unexpected NullPointerException."); + } int viewAxHashCode = mPlayerManager.getView().createAccessibilityNodeInfo().hashCode(); boolean didPinch = false; @@ -409,6 +423,11 @@ public class PaintPreviewPlayerTest extends DummyUiActivityTestCase { public void onLinkClick(GURL url) { mLinkClickHandler.onLinkClicked(url); } + + @Override + public boolean isAccessibilityEnabled() { + return false; + } }, 0xffffffff, false); mPlayerManager.setCompressOnClose(false); getActivity().setContentView(mPlayerManager.getView()); @@ -479,6 +498,7 @@ public class PaintPreviewPlayerTest extends DummyUiActivityTestCase { int[] locationXY = new int[2]; view.getLocationOnScreen(locationXY); UiDevice device = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation()); + device.waitForIdle(); device.click(scaledX + locationXY[0], scaledY + locationXY[1]); CriteriaHelper.pollUiThread(() -> { diff --git a/chromium/components/paint_preview/player/android/javatests/src/org/chromium/components/paintpreview/player/PaintPreviewTestService.java b/chromium/components/paint_preview/player/android/javatests/src/org/chromium/components/paintpreview/player/PaintPreviewTestService.java index 59a285e8284..2dc733a1e34 100644 --- a/chromium/components/paint_preview/player/android/javatests/src/org/chromium/components/paintpreview/player/PaintPreviewTestService.java +++ b/chromium/components/paint_preview/player/android/javatests/src/org/chromium/components/paintpreview/player/PaintPreviewTestService.java @@ -15,15 +15,18 @@ import org.chromium.components.paintpreview.browser.NativePaintPreviewServicePro @JNINamespace("paint_preview") public class PaintPreviewTestService implements NativePaintPreviewServiceProvider { private static final String TAG = "PPTestService"; + private long mNativePaintPreviewBaseService; private long mNativePaintPreviewTestService; public PaintPreviewTestService(String path) { mNativePaintPreviewTestService = PaintPreviewTestServiceJni.get().getInstance(path); + mNativePaintPreviewBaseService = + PaintPreviewTestServiceJni.get().getBaseService(mNativePaintPreviewTestService); } @Override - public long getNativeService() { - return mNativePaintPreviewTestService; + public long getNativeBaseService() { + return mNativePaintPreviewBaseService; } public boolean createFramesForKey(String key, String url, FrameData rootFrameData) { @@ -59,6 +62,7 @@ public class PaintPreviewTestService implements NativePaintPreviewServiceProvide @NativeMethods interface Natives { long getInstance(String path); + long getBaseService(long nativePaintPreviewTestService); int[] createSingleSkp(long nativePaintPreviewTestService, int id, int width, int height, int[] flattenedLinkRects, String[] links, int[] flattenedChildRects); boolean serializeFrames(long nativePaintPreviewTestService, String key, String url); diff --git a/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/frame/CompressibleBitmapTest.java b/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/frame/CompressibleBitmapTest.java index 11fc73c752a..181c9b9eaa0 100644 --- a/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/frame/CompressibleBitmapTest.java +++ b/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/frame/CompressibleBitmapTest.java @@ -48,7 +48,8 @@ public class CompressibleBitmapTest { } @Implementation - public static Bitmap decodeByteArray(byte[] array, int offset, int length) { + public static Bitmap decodeByteArray( + byte[] array, int offset, int length, BitmapFactory.Options options) { return sBitmap; } } @@ -56,7 +57,9 @@ public class CompressibleBitmapTest { @Test public void testCompressAndDiscard() { Bitmap bitmap = Mockito.mock(Bitmap.class); + Bitmap alphaBitmap = Mockito.mock(Bitmap.class); when(bitmap.compress(any(), anyInt(), any())).thenReturn(true); + when(bitmap.extractAlpha()).thenReturn(alphaBitmap); SequencedTaskRunner taskRunner = Mockito.mock(SequencedTaskRunner.class); doAnswer(invocation -> { @@ -68,6 +71,7 @@ public class CompressibleBitmapTest { CompressibleBitmap compressibleBitmap = new CompressibleBitmap(bitmap, taskRunner, false); verify(bitmap, times(1)).compress(any(), eq(100), any()); + verify(bitmap, times(1)).extractAlpha(); Assert.assertNull(compressibleBitmap.getBitmap()); } @@ -75,7 +79,9 @@ public class CompressibleBitmapTest { @Test public void testCompressAndKeep() { Bitmap bitmap = Mockito.mock(Bitmap.class); + Bitmap alphaBitmap = Mockito.mock(Bitmap.class); when(bitmap.compress(any(), anyInt(), any())).thenReturn(true); + when(bitmap.extractAlpha()).thenReturn(alphaBitmap); SequencedTaskRunner taskRunner = Mockito.mock(SequencedTaskRunner.class); doAnswer(invocation -> { @@ -87,6 +93,7 @@ public class CompressibleBitmapTest { CompressibleBitmap compressibleBitmap = new CompressibleBitmap(bitmap, taskRunner, true); verify(bitmap, times(1)).compress(any(), eq(100), any()); + verify(bitmap, times(1)).extractAlpha(); Assert.assertEquals(compressibleBitmap.getBitmap(), bitmap); compressibleBitmap.discardBitmap(); @@ -99,7 +106,9 @@ public class CompressibleBitmapTest { @Test public void testNoDiscardIfCompressFails() { Bitmap bitmap = Mockito.mock(Bitmap.class); + Bitmap alphaBitmap = Mockito.mock(Bitmap.class); when(bitmap.compress(any(), anyInt(), any())).thenReturn(false); + when(bitmap.extractAlpha()).thenReturn(alphaBitmap); SequencedTaskRunner taskRunner = Mockito.mock(SequencedTaskRunner.class); doAnswer(invocation -> { @@ -111,6 +120,7 @@ public class CompressibleBitmapTest { CompressibleBitmap compressibleBitmap = new CompressibleBitmap(bitmap, taskRunner, false); verify(bitmap, times(1)).compress(any(), eq(100), any()); + verify(bitmap, times(1)).extractAlpha(); // Discarding should fail. Assert.assertEquals(compressibleBitmap.getBitmap(), bitmap); @@ -121,7 +131,9 @@ public class CompressibleBitmapTest { @Test public void testInflate() throws TimeoutException { Bitmap bitmap = Mockito.mock(Bitmap.class); + Bitmap alphaBitmap = Mockito.mock(Bitmap.class); when(bitmap.compress(any(), anyInt(), any())).thenReturn(true); + when(bitmap.extractAlpha()).thenReturn(alphaBitmap); SequencedTaskRunner taskRunner = Mockito.mock(SequencedTaskRunner.class); doAnswer(invocation -> { @@ -133,6 +145,7 @@ public class CompressibleBitmapTest { CompressibleBitmap compressibleBitmap = new CompressibleBitmap(bitmap, taskRunner, false); verify(bitmap, times(1)).compress(any(), eq(100), any()); + verify(bitmap, times(1)).extractAlpha(); Assert.assertNull(compressibleBitmap.getBitmap()); FakeShadowBitmapFactory.setBitmap(bitmap); @@ -157,7 +170,9 @@ public class CompressibleBitmapTest { @Test public void testLocking() throws TimeoutException { Bitmap bitmap = Mockito.mock(Bitmap.class); + Bitmap alphaBitmap = Mockito.mock(Bitmap.class); when(bitmap.compress(any(), anyInt(), any())).thenReturn(true); + when(bitmap.extractAlpha()).thenReturn(alphaBitmap); SequencedTaskRunner taskRunner = Mockito.mock(SequencedTaskRunner.class); doAnswer(invocation -> { ((Runnable) invocation.getArgument(0)).run(); @@ -168,6 +183,7 @@ public class CompressibleBitmapTest { CompressibleBitmap compressibleBitmap = new CompressibleBitmap(bitmap, taskRunner, true); verify(bitmap, times(1)).compress(any(), eq(100), any()); + verify(bitmap, times(1)).extractAlpha(); Assert.assertTrue(compressibleBitmap.lock()); Assert.assertFalse(compressibleBitmap.lock()); Assert.assertEquals(compressibleBitmap.getBitmap(), bitmap); diff --git a/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/frame/PlayerFrameBitmapPainterTest.java b/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/frame/PlayerFrameBitmapPainterTest.java index 3659f74bbcd..2d16f4eb51b 100644 --- a/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/frame/PlayerFrameBitmapPainterTest.java +++ b/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/frame/PlayerFrameBitmapPainterTest.java @@ -64,7 +64,8 @@ public class PlayerFrameBitmapPainterTest { } @Implementation - public static Bitmap decodeByteArray(byte[] array, int offset, int length) { + public static Bitmap decodeByteArray( + byte[] array, int offset, int length, BitmapFactory.Options options) { return sBitmaps.get(fromByteArray(array)); } } @@ -262,11 +263,17 @@ public class PlayerFrameBitmapPainterTest { // Prepare the bitmap matrix. Bitmap bitmap00 = Mockito.mock(Bitmap.class); + when(bitmap00.extractAlpha()).thenReturn(Mockito.mock(Bitmap.class)); Bitmap bitmap10 = Mockito.mock(Bitmap.class); + when(bitmap10.extractAlpha()).thenReturn(Mockito.mock(Bitmap.class)); Bitmap bitmap01 = Mockito.mock(Bitmap.class); + when(bitmap01.extractAlpha()).thenReturn(Mockito.mock(Bitmap.class)); Bitmap bitmap11 = Mockito.mock(Bitmap.class); + when(bitmap11.extractAlpha()).thenReturn(Mockito.mock(Bitmap.class)); Bitmap bitmap20 = Mockito.mock(Bitmap.class); + when(bitmap20.extractAlpha()).thenReturn(Mockito.mock(Bitmap.class)); Bitmap bitmap21 = Mockito.mock(Bitmap.class); + when(bitmap21.extractAlpha()).thenReturn(Mockito.mock(Bitmap.class)); Map<Integer, Bitmap> bitmapMap = new HashMap<>(); bitmapMap.put(bitmap00.hashCode(), bitmap00); bitmapMap.put(bitmap10.hashCode(), bitmap10); diff --git a/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/frame/PlayerFrameCoordinatorTest.java b/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/frame/PlayerFrameCoordinatorTest.java index 6f188b41566..b7980196d3c 100644 --- a/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/frame/PlayerFrameCoordinatorTest.java +++ b/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/frame/PlayerFrameCoordinatorTest.java @@ -30,11 +30,11 @@ public class PlayerFrameCoordinatorTest { PlayerFrameCoordinator rootCoordinator = new PlayerFrameCoordinator( RuntimeEnvironment.systemContext, Mockito.mock(PlayerCompositorDelegate.class), Mockito.mock(UnguessableToken.class), 100, 2000, 0, 0, true, null, - Mockito.mock(PlayerGestureListener.class), null); + Mockito.mock(PlayerGestureListener.class), null, null); PlayerFrameCoordinator childCoordinator = new PlayerFrameCoordinator( RuntimeEnvironment.systemContext, Mockito.mock(PlayerCompositorDelegate.class), Mockito.mock(UnguessableToken.class), 100, 200, 0, 0, true, null, - Mockito.mock(PlayerGestureListener.class), null); + Mockito.mock(PlayerGestureListener.class), null, null); rootCoordinator.addSubFrame(childCoordinator, new Rect(10, 20, 35, 40)); rootCoordinator.getMediator().setLayoutDimensions(100, 200); diff --git a/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/frame/PlayerFrameMediatorTest.java b/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/frame/PlayerFrameMediatorTest.java index fd56112c2c7..200d69446a8 100644 --- a/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/frame/PlayerFrameMediatorTest.java +++ b/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/frame/PlayerFrameMediatorTest.java @@ -194,6 +194,14 @@ public class PlayerFrameMediatorTest { } @Override + public int requestBitmap(Rect clipRect, float scaleFactor, Callback<Bitmap> bitmapCallback, + Runnable errorCallback) { + Assert.fail("The GUIDless version of TestPlayerCompositorDelegate#requestBitmap() " + + "shouldn't be called."); + return 0; + } + + @Override public boolean cancelBitmapRequest(int requestId) { return false; } @@ -233,7 +241,7 @@ public class PlayerFrameMediatorTest { mFrameGuid, contentSize, 0, 0); mScaleController = new PlayerFrameScaleController(mModel.get(PlayerFrameProperties.SCALE_MATRIX), - mMediator, mGestureListener::onScale); + mMediator, null, mGestureListener::onScale); mScrollController = new PlayerFrameScrollController( mScroller, mMediator, mGestureListener::onScroll, mGestureListener::onFling); mBitmapStateController = mMediator.getBitmapStateControllerForTest(); @@ -757,19 +765,19 @@ public class PlayerFrameMediatorTest { List<ClickedPoint> expectedClickedPoints = new ArrayList<>(); // No scrolling has happened yet. - mMediator.onTap(15, 26); + mMediator.onTap(15, 26, false); expectedClickedPoints.add(new ClickedPoint(mFrameGuid, 15, 26)); Assert.assertEquals(expectedClickedPoints, mCompositorDelegate.mClickedPoints); // Scroll, and then click. The call to {@link PlayerFrameMediator} must account for the // scroll offset. mScrollController.scrollBy(90, 100); - mMediator.onTap(70, 50); + mMediator.onTap(70, 50, false); expectedClickedPoints.add(new ClickedPoint(mFrameGuid, 160, 150)); Assert.assertEquals(expectedClickedPoints, mCompositorDelegate.mClickedPoints); mScrollController.scrollBy(-40, -60); - mMediator.onTap(30, 80); + mMediator.onTap(30, 80, false); expectedClickedPoints.add(new ClickedPoint(mFrameGuid, 80, 120)); Assert.assertEquals(expectedClickedPoints, mCompositorDelegate.mClickedPoints); } diff --git a/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/frame/PlayerFrameScaleControllerTest.java b/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/frame/PlayerFrameScaleControllerTest.java index 20897f1c459..e0ff83c4921 100644 --- a/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/frame/PlayerFrameScaleControllerTest.java +++ b/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/frame/PlayerFrameScaleControllerTest.java @@ -65,7 +65,7 @@ public class PlayerFrameScaleControllerTest { when(mMediatorDelegateMock.getContentSize()).thenReturn(contentSize); when(mMediatorDelegateMock.getInitialScaleFactor()).thenReturn(1f); mScaleController = new PlayerFrameScaleController( - mBitmapScaleMatrix, mMediatorDelegateMock, mScaleListener); + mBitmapScaleMatrix, mMediatorDelegateMock, null, mScaleListener); mViewport.setScale(1f); mViewport.setSize(100, 100); } diff --git a/chromium/components/paint_preview/player/android/player_compositor_delegate_android.cc b/chromium/components/paint_preview/player/android/player_compositor_delegate_android.cc index 382e6f17a81..f90b9dd85ee 100644 --- a/chromium/components/paint_preview/player/android/player_compositor_delegate_android.cc +++ b/chromium/components/paint_preview/player/android/player_compositor_delegate_android.cc @@ -64,16 +64,18 @@ jlong JNI_PlayerCompositorDelegateImpl_Initialize( JNIEnv* env, const JavaParamRef<jobject>& j_object, jlong paint_preview_service, + const JavaParamRef<jbyteArray>& j_proto, const JavaParamRef<jstring>& j_url_spec, const JavaParamRef<jstring>& j_directory_key, + jboolean j_main_frame_mode, const JavaParamRef<jobject>& j_compositor_error_callback, jboolean j_is_low_mem) { PlayerCompositorDelegateAndroid* delegate = new PlayerCompositorDelegateAndroid( env, j_object, reinterpret_cast<PaintPreviewBaseService*>(paint_preview_service), - j_url_spec, j_directory_key, j_compositor_error_callback, - j_is_low_mem); + j_proto, j_url_spec, j_directory_key, j_main_frame_mode, + j_compositor_error_callback, j_is_low_mem); return reinterpret_cast<intptr_t>(delegate); } @@ -81,18 +83,33 @@ PlayerCompositorDelegateAndroid::PlayerCompositorDelegateAndroid( JNIEnv* env, const JavaParamRef<jobject>& j_object, PaintPreviewBaseService* paint_preview_service, + const JavaParamRef<jbyteArray>& j_proto, const JavaParamRef<jstring>& j_url_spec, const JavaParamRef<jstring>& j_directory_key, + jboolean j_main_frame_mode, const JavaParamRef<jobject>& j_compositor_error_callback, jboolean j_is_low_mem) : PlayerCompositorDelegate(), request_id_(0), startup_timestamp_(base::TimeTicks::Now()) { + if (j_proto) { + std::string serialized_proto; + base::android::JavaByteArrayToString(env, j_proto, &serialized_proto); + auto proto = std::make_unique<PaintPreviewProto>(); + if (!proto->ParseFromString(serialized_proto)) { + base::android::RunIntCallbackAndroid( + j_compositor_error_callback, + static_cast<int>(CompositorStatus::PROTOBUF_DESERIALIZATION_ERROR)); + return; + } + PlayerCompositorDelegate::SetProto(std::move(proto)); + } PlayerCompositorDelegate::Initialize( paint_preview_service, GURL(base::android::ConvertJavaStringToUTF8(env, j_url_spec)), DirectoryKey{ base::android::ConvertJavaStringToUTF8(env, j_directory_key)}, + static_cast<bool>(j_main_frame_mode), base::BindOnce(&base::android::RunIntCallbackAndroid, ScopedJavaGlobalRef<jobject>(j_compositor_error_callback)), base::TimeDelta::FromSeconds(15), @@ -103,7 +120,8 @@ PlayerCompositorDelegateAndroid::PlayerCompositorDelegateAndroid( void PlayerCompositorDelegateAndroid::OnCompositorReady( CompositorStatus compositor_status, - mojom::PaintPreviewBeginCompositeResponsePtr composite_response) { + mojom::PaintPreviewBeginCompositeResponsePtr composite_response, + std::unique_ptr<ui::AXTreeUpdate> ax_tree) { bool compositor_started = CompositorStatus::OK == compositor_status; base::UmaHistogramBoolean( "Browser.PaintPreview.Player.CompositorProcessStartedCorrectly", @@ -160,7 +178,8 @@ void PlayerCompositorDelegateAndroid::OnCompositorReady( Java_PlayerCompositorDelegateImpl_onCompositorReady( env, java_ref_, j_root_frame_guid, j_all_guids, j_scroll_extents, - j_scroll_offsets, j_subframe_count, j_subframe_ids, j_subframe_rects); + j_scroll_offsets, j_subframe_count, j_subframe_ids, j_subframe_rects, + reinterpret_cast<intptr_t>(ax_tree.release())); } void PlayerCompositorDelegateAndroid::OnMemoryPressure( @@ -224,20 +243,23 @@ jint PlayerCompositorDelegateAndroid::RequestBitmap( TRACE_EVENT_NESTABLE_ASYNC_BEGIN0( "paint_preview", "PlayerCompositorDelegateAndroid::RequestBitmap", TRACE_ID_LOCAL(request_id_)); - - int32_t id = PlayerCompositorDelegate::RequestBitmap( - base::android::UnguessableTokenAndroid::FromJavaUnguessableToken( - env, j_frame_guid), - gfx::Rect(j_clip_x, j_clip_y, j_clip_width, j_clip_height), - j_scale_factor, - base::BindOnce(&PlayerCompositorDelegateAndroid::OnBitmapCallback, - weak_factory_.GetWeakPtr(), - ScopedJavaGlobalRef<jobject>(j_bitmap_callback), - ScopedJavaGlobalRef<jobject>(j_error_callback), - request_id_)); + gfx::Rect rect(j_clip_x, j_clip_y, j_clip_width, j_clip_height); + auto callback = base::BindOnce( + &PlayerCompositorDelegateAndroid::OnBitmapCallback, + weak_factory_.GetWeakPtr(), + ScopedJavaGlobalRef<jobject>(j_bitmap_callback), + ScopedJavaGlobalRef<jobject>(j_error_callback), request_id_); ++request_id_; - return static_cast<jint>(id); + base::Optional<base::UnguessableToken> frame_guid; + if (j_frame_guid) { + frame_guid = + base::android::UnguessableTokenAndroid::FromJavaUnguessableToken( + env, j_frame_guid); + } + + return static_cast<jint>(PlayerCompositorDelegate::RequestBitmap( + frame_guid, rect, j_scale_factor, std::move(callback))); } jboolean PlayerCompositorDelegateAndroid::CancelBitmapRequest( @@ -263,7 +285,8 @@ void PlayerCompositorDelegateAndroid::OnBitmapCallback( sk_bitmap.computeByteSize()); if (status != mojom::PaintPreviewCompositor::BitmapStatus::kSuccess || - sk_bitmap.isNull()) { + sk_bitmap.isNull() || sk_bitmap.info().width() <= 0 || + sk_bitmap.info().height() <= 0) { base::android::RunRunnableAndroid(j_error_callback); return; } diff --git a/chromium/components/paint_preview/player/android/player_compositor_delegate_android.h b/chromium/components/paint_preview/player/android/player_compositor_delegate_android.h index 1b5955221da..e6d6177e0af 100644 --- a/chromium/components/paint_preview/player/android/player_compositor_delegate_android.h +++ b/chromium/components/paint_preview/player/android/player_compositor_delegate_android.h @@ -21,14 +21,17 @@ class PlayerCompositorDelegateAndroid : public PlayerCompositorDelegate { JNIEnv* env, const base::android::JavaParamRef<jobject>& j_object, PaintPreviewBaseService* paint_preview_service, + const base::android::JavaParamRef<jbyteArray>& j_proto, const base::android::JavaParamRef<jstring>& j_url_spec, const base::android::JavaParamRef<jstring>& j_directory_key, + jboolean j_main_frame_mode, const base::android::JavaParamRef<jobject>& j_compositor_error_callback, jboolean j_is_low_mem); void OnCompositorReady( CompositorStatus compositor_status, - mojom::PaintPreviewBeginCompositeResponsePtr composite_response) override; + mojom::PaintPreviewBeginCompositeResponsePtr composite_response, + std::unique_ptr<ui::AXTreeUpdate> ax_tree) override; void OnMemoryPressure(base::MemoryPressureListener::MemoryPressureLevel memory_pressure_level) override; diff --git a/chromium/components/paint_preview/player/bitmap_request.cc b/chromium/components/paint_preview/player/bitmap_request.cc index c15b30c99f2..a09c52654e6 100644 --- a/chromium/components/paint_preview/player/bitmap_request.cc +++ b/chromium/components/paint_preview/player/bitmap_request.cc @@ -6,10 +6,11 @@ namespace paint_preview { -BitmapRequest::BitmapRequest(const base::UnguessableToken& frame_guid, - const gfx::Rect& clip_rect, - float scale_factor, - BitmapRequestCallback callback) +BitmapRequest::BitmapRequest( + const base::Optional<base::UnguessableToken>& frame_guid, + const gfx::Rect& clip_rect, + float scale_factor, + BitmapRequestCallback callback) : frame_guid(frame_guid), clip_rect(clip_rect), scale_factor(scale_factor), diff --git a/chromium/components/paint_preview/player/bitmap_request.h b/chromium/components/paint_preview/player/bitmap_request.h index ef541931c43..b4de5299d3b 100644 --- a/chromium/components/paint_preview/player/bitmap_request.h +++ b/chromium/components/paint_preview/player/bitmap_request.h @@ -6,6 +6,7 @@ #define COMPONENTS_PAINT_PREVIEW_PLAYER_BITMAP_REQUEST_H_ #include "base/callback.h" +#include "base/optional.h" #include "base/unguessable_token.h" #include "components/services/paint_preview_compositor/public/mojom/paint_preview_compositor.mojom.h" #include "third_party/skia/include/core/SkBitmap.h" @@ -18,7 +19,7 @@ struct BitmapRequest { base::OnceCallback<void(mojom::PaintPreviewCompositor::BitmapStatus, const SkBitmap&)>; - BitmapRequest(const base::UnguessableToken& frame_guid, + BitmapRequest(const base::Optional<base::UnguessableToken>& frame_guid, const gfx::Rect& clip_rect, float scale_factor, BitmapRequestCallback callback); @@ -27,7 +28,7 @@ struct BitmapRequest { BitmapRequest& operator=(BitmapRequest&& other) noexcept; BitmapRequest(BitmapRequest&& other) noexcept; - base::UnguessableToken frame_guid; + base::Optional<base::UnguessableToken> frame_guid; gfx::Rect clip_rect; float scale_factor; BitmapRequestCallback callback; diff --git a/chromium/components/paint_preview/player/compositor_status.h b/chromium/components/paint_preview/player/compositor_status.h index 2961495e1c9..d155aa342bc 100644 --- a/chromium/components/paint_preview/player/compositor_status.h +++ b/chromium/components/paint_preview/player/compositor_status.h @@ -28,6 +28,8 @@ enum class CompositorStatus : int { TIMED_OUT, STOPPED_DUE_TO_MEMORY_PRESSURE, SKIPPED_DUE_TO_MEMORY_PRESSURE, + // Used by long screenshots code only when call to requestBitmap fails. + REQUEST_BITMAP_FAILURE, COUNT, }; diff --git a/chromium/components/paint_preview/player/player_compositor_delegate.cc b/chromium/components/paint_preview/player/player_compositor_delegate.cc index 5d653579ee4..4b054152653 100644 --- a/chromium/components/paint_preview/player/player_compositor_delegate.cc +++ b/chromium/components/paint_preview/player/player_compositor_delegate.cc @@ -105,10 +105,11 @@ PlayerCompositorDelegate::PlayerCompositorDelegate() PlayerCompositorDelegate::~PlayerCompositorDelegate() { if (compress_on_close_ && paint_preview_service_) { - paint_preview_service_->GetTaskRunner()->PostTask( + paint_preview_service_->GetFileMixin()->GetTaskRunner()->PostTask( FROM_HERE, base::BindOnce(base::IgnoreResult(&FileManager::CompressDirectory), - paint_preview_service_->GetFileManager(), key_)); + paint_preview_service_->GetFileMixin()->GetFileManager(), + key_)); } } @@ -116,6 +117,7 @@ void PlayerCompositorDelegate::Initialize( PaintPreviewBaseService* paint_preview_service, const GURL& expected_url, const DirectoryKey& key, + bool main_frame_mode, base::OnceCallback<void(int)> compositor_error, base::TimeDelta timeout_duration, size_t max_requests) { @@ -140,7 +142,7 @@ void PlayerCompositorDelegate::Initialize( &PlayerCompositorDelegate::OnCompositorServiceDisconnected, weak_factory_.GetWeakPtr())); - InitializeInternal(paint_preview_service, expected_url, key, + InitializeInternal(paint_preview_service, expected_url, key, main_frame_mode, std::move(compositor_error), timeout_duration, max_requests); } @@ -149,6 +151,7 @@ void PlayerCompositorDelegate::InitializeWithFakeServiceForTest( PaintPreviewBaseService* paint_preview_service, const GURL& expected_url, const DirectoryKey& key, + bool main_frame_mode, base::OnceCallback<void(int)> compositor_error, base::TimeDelta timeout_duration, size_t max_requests, @@ -159,7 +162,7 @@ void PlayerCompositorDelegate::InitializeWithFakeServiceForTest( base::BindOnce(&PlayerCompositorDelegate::OnCompositorServiceDisconnected, weak_factory_.GetWeakPtr())); - InitializeInternal(paint_preview_service, expected_url, key, + InitializeInternal(paint_preview_service, expected_url, key, main_frame_mode, std::move(compositor_error), timeout_duration, max_requests); } @@ -168,17 +171,15 @@ void PlayerCompositorDelegate::InitializeInternal( PaintPreviewBaseService* paint_preview_service, const GURL& expected_url, const DirectoryKey& key, + bool main_frame_mode, base::OnceCallback<void(int)> compositor_error, base::TimeDelta timeout_duration, size_t max_requests) { max_requests_ = max_requests; + main_frame_mode_ = main_frame_mode; compositor_error_ = std::move(compositor_error); paint_preview_service_ = paint_preview_service; key_ = key; - memory_pressure_ = std::make_unique<base::MemoryPressureListener>( - FROM_HERE, - base::BindRepeating(&PlayerCompositorDelegate::OnMemoryPressure, - weak_factory_.GetWeakPtr())); paint_preview_compositor_client_ = paint_preview_compositor_service_->CreateCompositor( @@ -188,6 +189,10 @@ void PlayerCompositorDelegate::InitializeInternal( base::BindOnce(&PlayerCompositorDelegate::OnCompositorClientDisconnected, weak_factory_.GetWeakPtr())); + memory_pressure_ = std::make_unique<base::MemoryPressureListener>( + FROM_HERE, + base::BindRepeating(&PlayerCompositorDelegate::OnMemoryPressure, + weak_factory_.GetWeakPtr())); if (!timeout_duration.is_inf() && !timeout_duration.is_zero()) { timeout_.Reset( base::BindOnce(&PlayerCompositorDelegate::OnCompositorTimeout, @@ -198,12 +203,14 @@ void PlayerCompositorDelegate::InitializeInternal( } int32_t PlayerCompositorDelegate::RequestBitmap( - const base::UnguessableToken& frame_guid, + const base::Optional<base::UnguessableToken>& frame_guid, const gfx::Rect& clip_rect, float scale_factor, base::OnceCallback<void(mojom::PaintPreviewCompositor::BitmapStatus, const SkBitmap&)> callback) { DCHECK(IsInitialized()); + DCHECK((main_frame_mode_ && !frame_guid.has_value()) || + (!main_frame_mode_ && frame_guid.has_value())); const int32_t request_id = next_request_id_; next_request_id_++; if (!paint_preview_compositor_client_) { @@ -296,7 +303,8 @@ void PlayerCompositorDelegate::OnCompositorReadyStatusAdapter( default: NOTREACHED(); } - OnCompositorReady(new_status, std::move(composite_response)); + OnCompositorReady(new_status, std::move(composite_response), + std::move(ax_tree_update_)); } void PlayerCompositorDelegate::OnCompositorServiceDisconnected() { @@ -313,30 +321,35 @@ void PlayerCompositorDelegate::OnCompositorClientCreated( TRACE_EVENT_NESTABLE_ASYNC_END0("paint_preview", "PlayerCompositorDelegate CreateCompositor", TRACE_ID_LOCAL(this)); - paint_preview_service_->GetCapturedPaintPreviewProto( - key, base::nullopt, - base::BindOnce(&PlayerCompositorDelegate::OnProtoAvailable, - weak_factory_.GetWeakPtr(), expected_url)); + if (!proto_) { + paint_preview_service_->GetFileMixin()->GetCapturedPaintPreviewProto( + key, base::nullopt, + base::BindOnce(&PlayerCompositorDelegate::OnProtoAvailable, + weak_factory_.GetWeakPtr(), expected_url)); + } else { + OnProtoAvailable(expected_url, PaintPreviewFileMixin::ProtoReadStatus::kOk, + std::move(proto_)); + } } void PlayerCompositorDelegate::OnProtoAvailable( const GURL& expected_url, - PaintPreviewBaseService::ProtoReadStatus proto_status, + PaintPreviewFileMixin::ProtoReadStatus proto_status, std::unique_ptr<PaintPreviewProto> proto) { - if (proto_status == PaintPreviewBaseService::ProtoReadStatus::kExpired) { - OnCompositorReady(CompositorStatus::CAPTURE_EXPIRED, nullptr); + if (proto_status == PaintPreviewFileMixin::ProtoReadStatus::kExpired) { + OnCompositorReady(CompositorStatus::CAPTURE_EXPIRED, nullptr, nullptr); return; } - if (proto_status == PaintPreviewBaseService::ProtoReadStatus::kNoProto) { - OnCompositorReady(CompositorStatus::NO_CAPTURE, nullptr); + if (proto_status == PaintPreviewFileMixin::ProtoReadStatus::kNoProto) { + OnCompositorReady(CompositorStatus::NO_CAPTURE, nullptr, nullptr); return; } if (proto_status == - PaintPreviewBaseService::ProtoReadStatus::kDeserializationError || + PaintPreviewFileMixin::ProtoReadStatus::kDeserializationError || !proto || !proto->IsInitialized()) { - OnCompositorReady(CompositorStatus::PROTOBUF_DESERIALIZATION_ERROR, + OnCompositorReady(CompositorStatus::PROTOBUF_DESERIALIZATION_ERROR, nullptr, nullptr); return; } @@ -348,30 +361,50 @@ void PlayerCompositorDelegate::OnProtoAvailable( // - The storage structure // In either case, the new code is likely unable to deserialize the result // so we should early abort. - OnCompositorReady(CompositorStatus::OLD_VERSION, nullptr); + OnCompositorReady(CompositorStatus::OLD_VERSION, nullptr, nullptr); return; } else if (version > kPaintPreviewVersion) { // This shouldn't happen hence NOTREACHED(). However, in release we should // treat this as a new failure type to catch any possible regressions. - OnCompositorReady(CompositorStatus::UNEXPECTED_VERSION, nullptr); + OnCompositorReady(CompositorStatus::UNEXPECTED_VERSION, nullptr, nullptr); NOTREACHED(); return; } auto proto_url = GURL(proto->metadata().url()); if (expected_url != proto_url) { - OnCompositorReady(CompositorStatus::URL_MISMATCH, nullptr); + OnCompositorReady(CompositorStatus::URL_MISMATCH, nullptr, nullptr); return; } if (!paint_preview_compositor_client_) { - OnCompositorReady(CompositorStatus::COMPOSITOR_CLIENT_DISCONNECT, nullptr); + OnCompositorReady(CompositorStatus::COMPOSITOR_CLIENT_DISCONNECT, nullptr, + nullptr); return; } paint_preview_compositor_client_->SetRootFrameUrl(proto_url); - proto_ = std::move(proto); + + // If the current Chrome version doesn't match the one in proto, we can't + // use the AXTreeUpdate. + auto chromeVersion = proto_->metadata().chrome_version(); + if (proto_->metadata().has_chrome_version() && + chromeVersion.major() == CHROME_VERSION_MAJOR && + chromeVersion.minor() == CHROME_VERSION_MINOR && + chromeVersion.build() == CHROME_VERSION_BUILD && + chromeVersion.patch() == CHROME_VERSION_PATCH) { + paint_preview_service_->GetFileMixin()->GetAXTreeUpdate( + key_, base::BindOnce(&PlayerCompositorDelegate::OnAXTreeUpdateAvailable, + weak_factory_.GetWeakPtr())); + } else { + PlayerCompositorDelegate::OnAXTreeUpdateAvailable(nullptr); + } +} + +void PlayerCompositorDelegate::OnAXTreeUpdateAvailable( + std::unique_ptr<ui::AXTreeUpdate> update) { + ax_tree_update_ = std::move(update); base::ThreadPool::PostTaskAndReplyWithResult( FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE}, base::BindOnce(&PrepareCompositeRequest, *proto_), @@ -383,20 +416,31 @@ void PlayerCompositorDelegate::SendCompositeRequest( mojom::PaintPreviewBeginCompositeRequestPtr begin_composite_request) { // TODO(crbug.com/1021590): Handle initialization errors. if (!begin_composite_request) { - OnCompositorReady(CompositorStatus::INVALID_REQUEST, nullptr); + OnCompositorReady(CompositorStatus::INVALID_REQUEST, nullptr, nullptr); return; } // It is possible the client was disconnected while loading the proto. if (!paint_preview_compositor_client_) { - OnCompositorReady(CompositorStatus::COMPOSITOR_CLIENT_DISCONNECT, nullptr); + OnCompositorReady(CompositorStatus::COMPOSITOR_CLIENT_DISCONNECT, nullptr, + nullptr); return; } - paint_preview_compositor_client_->BeginSeparatedFrameComposite( - std::move(begin_composite_request), - base::BindOnce(&PlayerCompositorDelegate::OnCompositorReadyStatusAdapter, - weak_factory_.GetWeakPtr())); + if (main_frame_mode_) { + paint_preview_compositor_client_->BeginMainFrameComposite( + std::move(begin_composite_request), + base::BindOnce( + &PlayerCompositorDelegate::OnCompositorReadyStatusAdapter, + weak_factory_.GetWeakPtr())); + + } else { + paint_preview_compositor_client_->BeginSeparatedFrameComposite( + std::move(begin_composite_request), + base::BindOnce( + &PlayerCompositorDelegate::OnCompositorReadyStatusAdapter, + weak_factory_.GetWeakPtr())); + } // Defer building hit testers so it happens in parallel with preparing the // compositor. @@ -436,9 +480,14 @@ void PlayerCompositorDelegate::ProcessBitmapRequestsFromQueue() { if (!paint_preview_compositor_client_) return; - paint_preview_compositor_client_->BitmapForSeparatedFrame( - request.frame_guid, request.clip_rect, request.scale_factor, - std::move(request.callback)); + if (request.frame_guid.has_value()) { + paint_preview_compositor_client_->BitmapForSeparatedFrame( + request.frame_guid.value(), request.clip_rect, request.scale_factor, + std::move(request.callback)); + } else { + paint_preview_compositor_client_->BitmapForMainFrame( + request.clip_rect, request.scale_factor, std::move(request.callback)); + } pending_bitmap_requests_.erase(it); } } diff --git a/chromium/components/paint_preview/player/player_compositor_delegate.h b/chromium/components/paint_preview/player/player_compositor_delegate.h index f08c6272cdf..cf8bebf1814 100644 --- a/chromium/components/paint_preview/player/player_compositor_delegate.h +++ b/chromium/components/paint_preview/player/player_compositor_delegate.h @@ -49,6 +49,7 @@ class PlayerCompositorDelegate { void Initialize(PaintPreviewBaseService* paint_preview_service, const GURL& url, const DirectoryKey& key, + bool main_frame_mode, base::OnceCallback<void(int)> compositor_error, base::TimeDelta timeout_duration, size_t max_requests); @@ -56,6 +57,10 @@ class PlayerCompositorDelegate { // Returns whether initialization has happened. bool IsInitialized() const { return paint_preview_service_; } + void SetProto(std::unique_ptr<PaintPreviewProto> proto) { + proto_ = std::move(proto); + } + // Overrides whether to compress the directory when the player is closed. By // default compression will happen. void SetCompressOnClose(bool compress) { compress_on_close_ = compress; } @@ -64,14 +69,15 @@ class PlayerCompositorDelegate { // situations. virtual void OnCompositorReady( CompositorStatus compositor_status, - mojom::PaintPreviewBeginCompositeResponsePtr composite_response) {} + mojom::PaintPreviewBeginCompositeResponsePtr composite_response, + std::unique_ptr<ui::AXTreeUpdate> update) {} // Called when there is a request for a new bitmap. When the bitmap // is ready, it will be passed to callback. Returns an ID for the request. // Pass this ID to `CancelBitmapRequest(int32_t)` to cancel the request if it // hasn't already been sent. int32_t RequestBitmap( - const base::UnguessableToken& frame_guid, + const base::Optional<base::UnguessableToken>& frame_guid, const gfx::Rect& clip_rect, float scale_factor, base::OnceCallback<void(mojom::PaintPreviewCompositor::BitmapStatus, @@ -100,6 +106,7 @@ class PlayerCompositorDelegate { PaintPreviewBaseService* paint_preview_service, const GURL& expected_url, const DirectoryKey& key, + bool main_frame_mode, base::OnceCallback<void(int)> compositor_error, base::TimeDelta timeout_duration, size_t max_requests, @@ -123,10 +130,13 @@ class PlayerCompositorDelegate { void InitializeInternal(PaintPreviewBaseService* paint_preview_service, const GURL& expected_url, const DirectoryKey& key, + bool main_frame_mode, base::OnceCallback<void(int)> compositor_error, base::TimeDelta timeout_duration, size_t max_requests); + void OnAXTreeUpdateAvailable(std::unique_ptr<ui::AXTreeUpdate> update); + void OnCompositorReadyStatusAdapter( mojom::PaintPreviewCompositor::BeginCompositeStatus status, mojom::PaintPreviewBeginCompositeResponsePtr composite_response); @@ -141,7 +151,7 @@ class PlayerCompositorDelegate { void OnCompositorTimeout(); void OnProtoAvailable(const GURL& expected_url, - PaintPreviewBaseService::ProtoReadStatus proto_status, + PaintPreviewFileMixin::ProtoReadStatus proto_status, std::unique_ptr<PaintPreviewProto> proto); void SendCompositeRequest( @@ -166,10 +176,12 @@ class PlayerCompositorDelegate { base::CancelableOnceClosure timeout_; int max_requests_{1}; + bool main_frame_mode_{false}; base::flat_map<base::UnguessableToken, std::unique_ptr<HitTester>> hit_testers_; std::unique_ptr<PaintPreviewProto> proto_; + std::unique_ptr<ui::AXTreeUpdate> ax_tree_update_; int active_requests_{0}; int32_t next_request_id_{0}; diff --git a/chromium/components/paint_preview/player/player_compositor_delegate_unittest.cc b/chromium/components/paint_preview/player/player_compositor_delegate_unittest.cc index e2728a07ba3..de29398fb9a 100644 --- a/chromium/components/paint_preview/player/player_compositor_delegate_unittest.cc +++ b/chromium/components/paint_preview/player/player_compositor_delegate_unittest.cc @@ -84,7 +84,11 @@ class FakePaintPreviewCompositorClient : public PaintPreviewCompositorClient { mojom::PaintPreviewBeginCompositeRequestPtr request, mojom::PaintPreviewCompositor::BeginMainFrameCompositeCallback callback) override { - NOTREACHED(); + auto response = mojom::PaintPreviewBeginCompositeResponse::New(); + response->root_frame_guid = base::UnguessableToken::Create(); + task_runner_->PostTask(FROM_HERE, + base::BindOnce(std::move(callback), response_status_, + std::move(response))); } void BitmapForMainFrame( @@ -92,7 +96,14 @@ class FakePaintPreviewCompositorClient : public PaintPreviewCompositorClient { float scale_factor, mojom::PaintPreviewCompositor::BitmapForMainFrameCallback callback) override { - NOTREACHED(); + SkBitmap bitmap; + bitmap.allocPixels( + SkImageInfo::MakeN32Premul(clip_rect.width(), clip_rect.height())); + task_runner_->PostTask( + FROM_HERE, + base::BindOnce(std::move(callback), + mojom::PaintPreviewCompositor::BitmapStatus::kSuccess, + bitmap)); } void SetRootFrameUrl(const GURL& url) override { @@ -190,9 +201,10 @@ class PlayerCompositorDelegateImpl : public PlayerCompositorDelegate { bool WasStatusChecked() const { return status_checked_; } - void OnCompositorReady(CompositorStatus compositor_status, - mojom::PaintPreviewBeginCompositeResponsePtr - composite_response) override { + void OnCompositorReady( + CompositorStatus compositor_status, + mojom::PaintPreviewBeginCompositeResponsePtr composite_response, + std::unique_ptr<ui::AXTreeUpdate> update) override { // Cast to int for easier debugging. EXPECT_EQ(static_cast<int>(expected_status_), static_cast<int>(compositor_status)); @@ -220,7 +232,8 @@ class PlayerCompositorDelegateTest : public testing::Test { void SetUp() override { ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); service_ = std::make_unique<PaintPreviewBaseService>( - temp_dir.GetPath(), "test", nullptr, false); + std::make_unique<PaintPreviewFileMixin>(temp_dir.GetPath(), "test"), + nullptr, false); } PaintPreviewBaseService* GetBaseService() { return service_.get(); } @@ -250,14 +263,16 @@ class PlayerCompositorDelegateTest : public testing::Test { } void SerializeProtoAndCreateRootSkp(PaintPreviewProto* proto, - const DirectoryKey& key) { - auto file_manager = GetBaseService()->GetFileManager(); + const DirectoryKey& key, + bool skip_proto_serialization = false) { + auto file_manager = GetBaseService()->GetFileMixin()->GetFileManager(); base::RunLoop loop; file_manager->GetTaskRunner()->PostTask( FROM_HERE, base::BindOnce( [](base::OnceClosure quit, scoped_refptr<FileManager> file_manager, - PaintPreviewProto* proto, const DirectoryKey& key) { + PaintPreviewProto* proto, const DirectoryKey& key, + bool skip_proto_serialization) { auto directory = file_manager->CreateOrGetDirectory(key, true); std::string fake_data = "Hello World!"; @@ -266,10 +281,13 @@ class PlayerCompositorDelegateTest : public testing::Test { root_file.AsUTF8Unsafe()); base::WriteFile(root_file, fake_data.data(), fake_data.size()); - file_manager->SerializePaintPreviewProto(key, *proto, false); + if (!skip_proto_serialization) { + file_manager->SerializePaintPreviewProto(key, *proto, false); + } std::move(quit).Run(); }, - loop.QuitClosure(), file_manager, proto, key)); + loop.QuitClosure(), file_manager, proto, key, + skip_proto_serialization)); loop.Run(); } @@ -283,7 +301,7 @@ class PlayerCompositorDelegateTest : public testing::Test { TEST_F(PlayerCompositorDelegateTest, OnClick) { auto* service = GetBaseService(); - auto file_manager = service->GetFileManager(); + auto file_manager = service->GetFileMixin()->GetFileManager(); auto key = file_manager->CreateKey(1U); GURL url("www.example.com"); @@ -347,8 +365,9 @@ TEST_F(PlayerCompositorDelegateTest, OnClick) { PlayerCompositorDelegateImpl player_compositor_delegate; player_compositor_delegate.SetExpectedStatus(CompositorStatus::OK); player_compositor_delegate.InitializeWithFakeServiceForTest( - service, url, key, base::DoNothing(), base::TimeDelta::Max(), - kMaxParallelRequests, CreateCompositorService()); + service, url, key, /*main_frame_mode=*/false, base::DoNothing(), + base::TimeDelta::Max(), kMaxParallelRequests, + CreateCompositorService()); env.RunUntilIdle(); EXPECT_TRUE(player_compositor_delegate.WasStatusChecked()); @@ -371,7 +390,7 @@ TEST_F(PlayerCompositorDelegateTest, OnClick) { TEST_F(PlayerCompositorDelegateTest, BadProto) { auto* service = GetBaseService(); - auto file_manager = service->GetFileManager(); + auto file_manager = service->GetFileMixin()->GetFileManager(); auto key = file_manager->CreateKey(1U); base::RunLoop loop; file_manager->GetTaskRunner()->PostTask( @@ -391,8 +410,9 @@ TEST_F(PlayerCompositorDelegateTest, BadProto) { player_compositor_delegate.SetExpectedStatus( CompositorStatus::PROTOBUF_DESERIALIZATION_ERROR); player_compositor_delegate.InitializeWithFakeServiceForTest( - service, GURL(), key, base::DoNothing(), base::TimeDelta::Max(), - kMaxParallelRequests, CreateCompositorService()); + service, GURL(), key, /*main_frame_mode=*/false, base::DoNothing(), + base::TimeDelta::Max(), kMaxParallelRequests, + CreateCompositorService()); env.RunUntilIdle(); EXPECT_TRUE(player_compositor_delegate.WasStatusChecked()); } @@ -401,7 +421,7 @@ TEST_F(PlayerCompositorDelegateTest, BadProto) { TEST_F(PlayerCompositorDelegateTest, OldVersion) { auto* service = GetBaseService(); - auto file_manager = service->GetFileManager(); + auto file_manager = service->GetFileMixin()->GetFileManager(); auto key = file_manager->CreateKey(1U); GURL url("https://www.chromium.org/"); auto proto = CreateValidProto(url); @@ -411,8 +431,32 @@ TEST_F(PlayerCompositorDelegateTest, OldVersion) { PlayerCompositorDelegateImpl player_compositor_delegate; player_compositor_delegate.SetExpectedStatus(CompositorStatus::OLD_VERSION); player_compositor_delegate.InitializeWithFakeServiceForTest( - service, url, key, base::DoNothing(), base::TimeDelta::Max(), - kMaxParallelRequests, CreateCompositorService()); + service, url, key, /*main_frame_mode=*/false, base::DoNothing(), + base::TimeDelta::Max(), kMaxParallelRequests, + CreateCompositorService()); + player_compositor_delegate.SetCompressOnClose(false); + env.RunUntilIdle(); + EXPECT_TRUE(player_compositor_delegate.WasStatusChecked()); + } + env.RunUntilIdle(); +} + +TEST_F(PlayerCompositorDelegateTest, InMemoryProto) { + auto* service = GetBaseService(); + auto file_manager = service->GetFileMixin()->GetFileManager(); + auto key = file_manager->CreateKey(1U); + GURL url("https://www.chromium.org/"); + auto proto = CreateValidProto(url); + SerializeProtoAndCreateRootSkp(&proto, key, true); + { + PlayerCompositorDelegateImpl player_compositor_delegate; + player_compositor_delegate.SetProto( + std::make_unique<PaintPreviewProto>(proto)); + player_compositor_delegate.SetExpectedStatus(CompositorStatus::OK); + player_compositor_delegate.InitializeWithFakeServiceForTest( + service, url, key, /*main_frame_mode=*/false, base::DoNothing(), + base::TimeDelta::Max(), kMaxParallelRequests, + CreateCompositorService()); player_compositor_delegate.SetCompressOnClose(false); env.RunUntilIdle(); EXPECT_TRUE(player_compositor_delegate.WasStatusChecked()); @@ -422,7 +466,7 @@ TEST_F(PlayerCompositorDelegateTest, OldVersion) { TEST_F(PlayerCompositorDelegateTest, URLMismatch) { auto* service = GetBaseService(); - auto file_manager = service->GetFileManager(); + auto file_manager = service->GetFileMixin()->GetFileManager(); auto key = file_manager->CreateKey(1U); GURL url("https://www.chromium.org/"); auto proto = CreateValidProto(url); @@ -432,8 +476,9 @@ TEST_F(PlayerCompositorDelegateTest, URLMismatch) { player_compositor_delegate.SetExpectedStatus( CompositorStatus::URL_MISMATCH); player_compositor_delegate.InitializeWithFakeServiceForTest( - service, GURL(), key, base::DoNothing(), base::TimeDelta::Max(), - kMaxParallelRequests, CreateCompositorService()); + service, GURL(), key, /*main_frame_mode=*/false, base::DoNothing(), + base::TimeDelta::Max(), kMaxParallelRequests, + CreateCompositorService()); env.RunUntilIdle(); EXPECT_TRUE(player_compositor_delegate.WasStatusChecked()); } @@ -442,7 +487,7 @@ TEST_F(PlayerCompositorDelegateTest, URLMismatch) { TEST_F(PlayerCompositorDelegateTest, ServiceDisconnect) { auto* service = GetBaseService(); - auto file_manager = service->GetFileManager(); + auto file_manager = service->GetFileMixin()->GetFileManager(); auto key = file_manager->CreateKey(1U); GURL url("https://www.chromium.org/"); auto proto = CreateValidProto(url); @@ -452,7 +497,7 @@ TEST_F(PlayerCompositorDelegateTest, ServiceDisconnect) { player_compositor_delegate.SetExpectedStatus(CompositorStatus::OK); bool called = false; player_compositor_delegate.InitializeWithFakeServiceForTest( - service, url, key, + service, url, key, /*main_frame_mode=*/false, base::BindOnce( [](bool* called, int status) { EXPECT_EQ(static_cast<int>( @@ -474,7 +519,7 @@ TEST_F(PlayerCompositorDelegateTest, ServiceDisconnect) { TEST_F(PlayerCompositorDelegateTest, ClientDisconnect) { auto* service = GetBaseService(); - auto file_manager = service->GetFileManager(); + auto file_manager = service->GetFileMixin()->GetFileManager(); auto key = file_manager->CreateKey(1U); GURL url("https://www.chromium.org/"); auto proto = CreateValidProto(url); @@ -484,7 +529,7 @@ TEST_F(PlayerCompositorDelegateTest, ClientDisconnect) { player_compositor_delegate.SetExpectedStatus(CompositorStatus::OK); bool called = false; player_compositor_delegate.InitializeWithFakeServiceForTest( - service, url, key, + service, url, key, /*main_frame_mode=*/false, base::BindOnce( [](bool* called, int status) { EXPECT_EQ(static_cast<int>( @@ -505,7 +550,7 @@ TEST_F(PlayerCompositorDelegateTest, ClientDisconnect) { TEST_F(PlayerCompositorDelegateTest, InvalidCompositeRequest) { auto* service = GetBaseService(); - auto file_manager = service->GetFileManager(); + auto file_manager = service->GetFileMixin()->GetFileManager(); auto key = file_manager->CreateKey(1U); GURL url("https://www.chromium.org/"); auto proto = CreateValidProto(url); @@ -526,8 +571,9 @@ TEST_F(PlayerCompositorDelegateTest, InvalidCompositeRequest) { player_compositor_delegate.SetExpectedStatus( CompositorStatus::INVALID_REQUEST); player_compositor_delegate.InitializeWithFakeServiceForTest( - service, url, key, base::DoNothing(), base::TimeDelta::Max(), - kMaxParallelRequests, CreateCompositorService()); + service, url, key, /*main_frame_mode=*/false, base::DoNothing(), + base::TimeDelta::Max(), kMaxParallelRequests, + CreateCompositorService()); env.RunUntilIdle(); EXPECT_TRUE(player_compositor_delegate.WasStatusChecked()); } @@ -536,7 +582,7 @@ TEST_F(PlayerCompositorDelegateTest, InvalidCompositeRequest) { TEST_F(PlayerCompositorDelegateTest, CompositorDeserializationError) { auto* service = GetBaseService(); - auto file_manager = service->GetFileManager(); + auto file_manager = service->GetFileMixin()->GetFileManager(); auto key = file_manager->CreateKey(1U); GURL url("https://www.chromium.org/"); auto proto = CreateValidProto(url); @@ -546,8 +592,9 @@ TEST_F(PlayerCompositorDelegateTest, CompositorDeserializationError) { player_compositor_delegate.SetExpectedStatus( CompositorStatus::COMPOSITOR_DESERIALIZATION_ERROR); player_compositor_delegate.InitializeWithFakeServiceForTest( - service, url, key, base::DoNothing(), base::TimeDelta::Max(), - kMaxParallelRequests, CreateCompositorService()); + service, url, key, /*main_frame_mode=*/false, base::DoNothing(), + base::TimeDelta::Max(), kMaxParallelRequests, + CreateCompositorService()); AsFakeClient(player_compositor_delegate.GetClientForTest()) ->SetBeginSeparatedFrameResponseStatus( mojom::PaintPreviewCompositor::BeginCompositeStatus:: @@ -560,7 +607,7 @@ TEST_F(PlayerCompositorDelegateTest, CompositorDeserializationError) { TEST_F(PlayerCompositorDelegateTest, InvalidRootSkp) { auto* service = GetBaseService(); - auto file_manager = service->GetFileManager(); + auto file_manager = service->GetFileMixin()->GetFileManager(); auto key = file_manager->CreateKey(1U); GURL url("https://www.chromium.org/"); auto proto = CreateValidProto(url); @@ -570,8 +617,9 @@ TEST_F(PlayerCompositorDelegateTest, InvalidRootSkp) { player_compositor_delegate.SetExpectedStatus( CompositorStatus::INVALID_ROOT_FRAME_SKP); player_compositor_delegate.InitializeWithFakeServiceForTest( - service, url, key, base::DoNothing(), base::TimeDelta::Max(), - kMaxParallelRequests, CreateCompositorService()); + service, url, key, /*main_frame_mode=*/false, base::DoNothing(), + base::TimeDelta::Max(), kMaxParallelRequests, + CreateCompositorService()); AsFakeClient(player_compositor_delegate.GetClientForTest()) ->SetBeginSeparatedFrameResponseStatus( mojom::PaintPreviewCompositor::BeginCompositeStatus:: @@ -584,7 +632,7 @@ TEST_F(PlayerCompositorDelegateTest, InvalidRootSkp) { TEST_F(PlayerCompositorDelegateTest, CompressOnClose) { auto* service = GetBaseService(); - auto file_manager = service->GetFileManager(); + auto file_manager = service->GetFileMixin()->GetFileManager(); auto key = file_manager->CreateKey(1U); base::FilePath dir; file_manager->GetTaskRunner()->PostTaskAndReplyWithResult( @@ -605,8 +653,9 @@ TEST_F(PlayerCompositorDelegateTest, CompressOnClose) { PlayerCompositorDelegateImpl player_compositor_delegate; player_compositor_delegate.SetExpectedStatus(CompositorStatus::NO_CAPTURE); player_compositor_delegate.InitializeWithFakeServiceForTest( - service, GURL(), key, base::DoNothing(), base::TimeDelta::Max(), - kMaxParallelRequests, CreateCompositorService()); + service, GURL(), key, /*main_frame_mode=*/false, base::DoNothing(), + base::TimeDelta::Max(), kMaxParallelRequests, + CreateCompositorService()); env.RunUntilIdle(); EXPECT_TRUE(player_compositor_delegate.WasStatusChecked()); } @@ -616,7 +665,7 @@ TEST_F(PlayerCompositorDelegateTest, CompressOnClose) { TEST_F(PlayerCompositorDelegateTest, RequestBitmapWithCancel) { auto* service = GetBaseService(); - auto file_manager = service->GetFileManager(); + auto file_manager = service->GetFileMixin()->GetFileManager(); auto key = file_manager->CreateKey(1U); { // This test skips setting up files as the fakes don't use them. In normal @@ -625,8 +674,9 @@ TEST_F(PlayerCompositorDelegateTest, RequestBitmapWithCancel) { PlayerCompositorDelegateImpl player_compositor_delegate; player_compositor_delegate.SetExpectedStatus(CompositorStatus::NO_CAPTURE); player_compositor_delegate.InitializeWithFakeServiceForTest( - service, GURL(), key, base::DoNothing(), base::TimeDelta::Max(), - kMaxParallelRequests, CreateCompositorService()); + service, GURL(), key, /*main_frame_mode=*/false, base::DoNothing(), + base::TimeDelta::Max(), kMaxParallelRequests, + CreateCompositorService()); env.RunUntilIdle(); EXPECT_TRUE(player_compositor_delegate.WasStatusChecked()); @@ -687,7 +737,7 @@ TEST_F(PlayerCompositorDelegateTest, RequestBitmapWithCancel) { TEST_F(PlayerCompositorDelegateTest, RequestBitmapWithCancelAll) { auto* service = GetBaseService(); - auto file_manager = service->GetFileManager(); + auto file_manager = service->GetFileMixin()->GetFileManager(); auto key = file_manager->CreateKey(1U); { // This test skips setting up files as the fakes don't use them. In normal @@ -696,8 +746,9 @@ TEST_F(PlayerCompositorDelegateTest, RequestBitmapWithCancelAll) { PlayerCompositorDelegateImpl player_compositor_delegate; player_compositor_delegate.SetExpectedStatus(CompositorStatus::NO_CAPTURE); player_compositor_delegate.InitializeWithFakeServiceForTest( - service, GURL(), key, base::DoNothing(), base::TimeDelta::Max(), - kMaxParallelRequests, CreateCompositorService()); + service, GURL(), key, /*main_frame_mode=*/false, base::DoNothing(), + base::TimeDelta::Max(), kMaxParallelRequests, + CreateCompositorService()); env.RunUntilIdle(); EXPECT_TRUE(player_compositor_delegate.WasStatusChecked()); @@ -739,7 +790,7 @@ TEST_F(PlayerCompositorDelegateTest, RequestBitmapWithCancelAll) { TEST_F(PlayerCompositorDelegateTest, RequestBitmapSuccessQueued) { auto* service = GetBaseService(); - auto file_manager = service->GetFileManager(); + auto file_manager = service->GetFileMixin()->GetFileManager(); auto key = file_manager->CreateKey(1U); { // This test skips setting up files as the fakes don't use them. In normal @@ -748,8 +799,9 @@ TEST_F(PlayerCompositorDelegateTest, RequestBitmapSuccessQueued) { PlayerCompositorDelegateImpl player_compositor_delegate; player_compositor_delegate.SetExpectedStatus(CompositorStatus::NO_CAPTURE); player_compositor_delegate.InitializeWithFakeServiceForTest( - service, GURL(), key, base::DoNothing(), base::TimeDelta::Max(), - kMaxParallelRequests, CreateCompositorService()); + service, GURL(), key, /*main_frame_mode=*/false, base::DoNothing(), + base::TimeDelta::Max(), kMaxParallelRequests, + CreateCompositorService()); env.RunUntilIdle(); EXPECT_TRUE(player_compositor_delegate.WasStatusChecked()); @@ -770,9 +822,46 @@ TEST_F(PlayerCompositorDelegateTest, RequestBitmapSuccessQueued) { env.RunUntilIdle(); } +TEST_F(PlayerCompositorDelegateTest, RequestMainFrameBitmapSuccess) { + auto* service = GetBaseService(); + auto file_manager = service->GetFileMixin()->GetFileManager(); + auto key = file_manager->CreateKey(1U); + GURL url("https://www.chromium.org/"); + auto proto = CreateValidProto(url); + SerializeProtoAndCreateRootSkp(&proto, key); + { + // This test skips setting up files as the fakes don't use them. In normal + // execution the files are required by the service or no bitmap will be + // created. + PlayerCompositorDelegateImpl player_compositor_delegate; + player_compositor_delegate.SetExpectedStatus(CompositorStatus::OK); + player_compositor_delegate.InitializeWithFakeServiceForTest( + service, url, key, /*main_frame_mode=*/true, base::DoNothing(), + base::TimeDelta::Max(), kMaxParallelRequests, + CreateCompositorService()); + env.RunUntilIdle(); + EXPECT_TRUE(player_compositor_delegate.WasStatusChecked()); + + base::RunLoop loop; + player_compositor_delegate.RequestBitmap( + base::nullopt, gfx::Rect(10, 20, 30, 40), 1.0, + base::BindOnce( + [](base::OnceClosure quit, + mojom::PaintPreviewCompositor::BitmapStatus status, + const SkBitmap& bitmap) { + EXPECT_EQ(mojom::PaintPreviewCompositor::BitmapStatus::kSuccess, + status); + std::move(quit).Run(); + }, + loop.QuitClosure())); + loop.Run(); + } + env.RunUntilIdle(); +} + TEST_F(PlayerCompositorDelegateTest, Timeout) { auto* service = GetBaseService(); - auto file_manager = service->GetFileManager(); + auto file_manager = service->GetFileMixin()->GetFileManager(); auto key = file_manager->CreateKey(1U); { PlayerCompositorDelegateImpl player_compositor_delegate; @@ -780,7 +869,7 @@ TEST_F(PlayerCompositorDelegateTest, Timeout) { AsFakeService(compositor_service.get())->SetTimeout(); base::RunLoop loop; player_compositor_delegate.InitializeWithFakeServiceForTest( - service, GURL(), key, + service, GURL(), key, /*main_frame_mode=*/false, base::BindOnce( [](base::OnceClosure quit, int status) { EXPECT_EQ(static_cast<CompositorStatus>(status), @@ -798,7 +887,7 @@ TEST_F(PlayerCompositorDelegateTest, Timeout) { TEST_F(PlayerCompositorDelegateTest, CriticalMemoryPressure) { auto* service = GetBaseService(); - auto file_manager = service->GetFileManager(); + auto file_manager = service->GetFileMixin()->GetFileManager(); auto key = file_manager->CreateKey(1U); { // This test skips setting up files as the fakes don't use them. In normal @@ -808,7 +897,7 @@ TEST_F(PlayerCompositorDelegateTest, CriticalMemoryPressure) { PlayerCompositorDelegateImpl player_compositor_delegate; player_compositor_delegate.SetExpectedStatus(CompositorStatus::NO_CAPTURE); player_compositor_delegate.InitializeWithFakeServiceForTest( - service, GURL(), key, + service, GURL(), key, /*main_frame_mode=*/false, base::BindOnce( [](base::OnceClosure quit, int compositor_status) { EXPECT_EQ(compositor_status, @@ -831,7 +920,7 @@ TEST_F(PlayerCompositorDelegateTest, CriticalMemoryPressure) { TEST_F(PlayerCompositorDelegateTest, CriticalMemoryPressureBeforeStart) { auto* service = GetBaseService(); - auto file_manager = service->GetFileManager(); + auto file_manager = service->GetFileMixin()->GetFileManager(); auto key = file_manager->CreateKey(1U); { // This test skips setting up files as the fakes don't use them. In normal @@ -845,7 +934,7 @@ TEST_F(PlayerCompositorDelegateTest, CriticalMemoryPressureBeforeStart) { player_compositor_delegate.SetFakeMemoryPressureMonitor( &memory_pressure_monitor); player_compositor_delegate.Initialize( - service, GURL(), key, + service, GURL(), key, /*main_frame_mode=*/false, base::BindOnce( [](base::OnceClosure quit, int compositor_status) { EXPECT_EQ(compositor_status, diff --git a/chromium/components/paint_preview/public/paint_preview_compositor_client.h b/chromium/components/paint_preview/public/paint_preview_compositor_client.h index 4c119038c43..9f49438c501 100644 --- a/chromium/components/paint_preview/public/paint_preview_compositor_client.h +++ b/chromium/components/paint_preview/public/paint_preview_compositor_client.h @@ -29,9 +29,12 @@ class PaintPreviewCompositorClient { // isn't started. virtual const base::Optional<base::UnguessableToken>& Token() const = 0; - // Adds |closure| as a disconnect handler. + // Adds `closure` as a disconnect handler. virtual void SetDisconnectHandler(base::OnceClosure closure) = 0; + // Note the BitmapFor* methods use `clip_rect` values relative to the captured + // content. + // mojom::PaintPreviewCompositor API virtual void BeginSeparatedFrameComposite( mojom::PaintPreviewBeginCompositeRequestPtr request, diff --git a/chromium/components/paint_preview/renderer/BUILD.gn b/chromium/components/paint_preview/renderer/BUILD.gn index f88dd6f4693..c02e8ebf1ba 100644 --- a/chromium/components/paint_preview/renderer/BUILD.gn +++ b/chromium/components/paint_preview/renderer/BUILD.gn @@ -4,52 +4,52 @@ import("//testing/test.gni") -if (!is_ios) { - static_library("renderer") { - sources = [ - "paint_preview_recorder_impl.cc", - "paint_preview_recorder_impl.h", - "paint_preview_recorder_utils.cc", - "paint_preview_recorder_utils.h", - ] - - deps = [ - "//base", - "//cc/paint", - "//content/public/renderer", - "//mojo/public/cpp/base", - "//third_party/blink/public:blink_headers", - "//third_party/blink/public/common", - ] - - public_deps = [ - "//components/paint_preview/common", - "//components/paint_preview/common/mojom", - ] - } - - source_set("unit_tests") { - testonly = true - - sources = [ "paint_preview_recorder_utils_unittest.cc" ] - - deps = [ - ":renderer", - "//base", - "//base/test:test_support", - "//cc/paint", - "//components/paint_preview/common:test_utils", - "//testing/gmock", - "//testing/gtest", - ] - } - - test("paint_preview_renderer_unit_tests") { - deps = [ - ":unit_tests", - "//base", - "//base/test:test_support", - "//components/test:run_all_unittests", - ] - } +assert(!is_ios, "Paint Previews are not supported on iOS.") + +source_set("renderer") { + sources = [ + "paint_preview_recorder_impl.cc", + "paint_preview_recorder_impl.h", + "paint_preview_recorder_utils.cc", + "paint_preview_recorder_utils.h", + ] + + deps = [ + "//base", + "//cc/paint", + "//content/public/renderer", + "//mojo/public/cpp/base", + "//third_party/blink/public:blink_headers", + "//third_party/blink/public/common", + ] + + public_deps = [ + "//components/paint_preview/common", + "//components/paint_preview/common/mojom", + ] +} + +source_set("unit_tests") { + testonly = true + + sources = [ "paint_preview_recorder_utils_unittest.cc" ] + + deps = [ + ":renderer", + "//base", + "//base/test:test_support", + "//cc/paint", + "//components/paint_preview/common:test_utils", + "//testing/gmock", + "//testing/gtest", + ] +} + +test("paint_preview_renderer_unit_tests") { + deps = [ + ":unit_tests", + "//base", + "//base/test:test_support", + "//components/test:run_all_unittests", + ] } diff --git a/chromium/components/paint_preview/renderer/paint_preview_recorder_browsertest.cc b/chromium/components/paint_preview/renderer/paint_preview_recorder_browsertest.cc index 71867bc8c08..59df0a945d8 100644 --- a/chromium/components/paint_preview/renderer/paint_preview_recorder_browsertest.cc +++ b/chromium/components/paint_preview/renderer/paint_preview_recorder_browsertest.cc @@ -255,6 +255,31 @@ TEST_F(PaintPreviewRecorderRenderViewTest, TestCaptureInvalidFile) { content::RunAllTasksUntilIdle(); } +TEST_F(PaintPreviewRecorderRenderViewTest, TestCaptureInvalidXYClip) { + LoadHTML("<body></body>"); + + mojom::PaintPreviewCaptureParamsPtr params = + mojom::PaintPreviewCaptureParams::New(); + auto token = base::UnguessableToken::Create(); + params->guid = token; + params->clip_rect = gfx::Rect(1000000, 1000000, 10, 10); + params->is_main_frame = true; + params->capture_links = true; + params->max_capture_size = 0; + base::FilePath skp_path = MakeTestFilePath("test.skp"); + base::File skp_file(skp_path, + base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE); + params->file = std::move(skp_file); + + content::RenderFrame* frame = GetFrame(); + PaintPreviewRecorderImpl paint_preview_recorder(frame); + paint_preview_recorder.CapturePaintPreview( + std::move(params), + base::BindOnce(&OnCaptureFinished, + mojom::PaintPreviewStatus::kCaptureFailed, nullptr)); + content::RunAllTasksUntilIdle(); +} + TEST_F(PaintPreviewRecorderRenderViewTest, TestCaptureMainFrameAndLocalFrame) { LoadHTML( "<!doctype html>" @@ -339,6 +364,73 @@ TEST_F(PaintPreviewRecorderRenderViewTest, TestCaptureCustomClipRect) { EXPECT_EQ(out_response->links[0]->rect.height(), 30); } +TEST_F(PaintPreviewRecorderRenderViewTest, TestCaptureWithClamp) { + LoadHTML( + "<!doctype html>" + "<body>" + " <div style='width: 600px; height: 600px; background-color: #0000ff;'>" + " <div style='width: 300px; height: 300px; background-color: " + " #ffff00; position: relative; left: 150px; top: 150px'></div>" + " </div>" + " <a style='position: absolute; left: 160px; top: 170px; width: 40px; " + " height: 30px;' href='http://www.example.com'>Foo</a>" + "</body>"); + + auto out_response = mojom::PaintPreviewCaptureResponse::New(); + content::RenderFrame* frame = GetFrame(); + const size_t kLarge = 1000000; + gfx::Rect clip_rect = gfx::Rect(0, 0, kLarge, kLarge); + base::FilePath skp_path = RunCapture(frame, &out_response, true, clip_rect); + + EXPECT_TRUE(out_response->embedding_token.has_value()); + EXPECT_EQ(frame->GetWebFrame()->GetEmbeddingToken(), + out_response->embedding_token.value()); + EXPECT_EQ(out_response->content_id_to_embedding_token.size(), 0U); + + sk_sp<SkPicture> pic; + { + base::ScopedAllowBlockingForTesting scope; + FileRStream rstream(base::File( + skp_path, base::File::FLAG_OPEN_ALWAYS | base::File::FLAG_READ)); + pic = SkPicture::MakeFromStream(&rstream, nullptr); + } + EXPECT_LT(pic->cullRect().height(), kLarge); + EXPECT_LT(pic->cullRect().width(), kLarge); +} + +TEST_F(PaintPreviewRecorderRenderViewTest, TestCaptureFullIfWidthHeightAre0) { + LoadHTML( + "<!doctype html>" + "<body>" + " <div style='width: 600px; height: 600px; background-color: #0000ff;'>" + " <div style='width: 300px; height: 300px; background-color: " + " #ffff00; position: relative; left: 150px; top: 150px'></div>" + " </div>" + " <a style='position: absolute; left: 160px; top: 170px; width: 40px; " + " height: 30px;' href='http://www.example.com'>Foo</a>" + "</body>"); + + auto out_response = mojom::PaintPreviewCaptureResponse::New(); + content::RenderFrame* frame = GetFrame(); + gfx::Rect clip_rect = gfx::Rect(1, 1, 0, 0); + base::FilePath skp_path = RunCapture(frame, &out_response, true, clip_rect); + + EXPECT_TRUE(out_response->embedding_token.has_value()); + EXPECT_EQ(frame->GetWebFrame()->GetEmbeddingToken(), + out_response->embedding_token.value()); + EXPECT_EQ(out_response->content_id_to_embedding_token.size(), 0U); + + sk_sp<SkPicture> pic; + { + base::ScopedAllowBlockingForTesting scope; + FileRStream rstream(base::File( + skp_path, base::File::FLAG_OPEN_ALWAYS | base::File::FLAG_READ)); + pic = SkPicture::MakeFromStream(&rstream, nullptr); + } + EXPECT_GT(pic->cullRect().height(), 0U); + EXPECT_GT(pic->cullRect().width(), 0U); +} + TEST_F(PaintPreviewRecorderRenderViewTest, CaptureWithTranslate) { // URLs should be annotated correctly when a CSS transform is applied. LoadHTML( diff --git a/chromium/components/paint_preview/renderer/paint_preview_recorder_impl.cc b/chromium/components/paint_preview/renderer/paint_preview_recorder_impl.cc index b01d7d51d4c..c3f0315410d 100644 --- a/chromium/components/paint_preview/renderer/paint_preview_recorder_impl.cc +++ b/chromium/components/paint_preview/renderer/paint_preview_recorder_impl.cc @@ -9,6 +9,7 @@ #include "base/auto_reset.h" #include "base/bind.h" +#include "base/bind_post_task.h" #include "base/metrics/histogram_functions.h" #include "base/optional.h" #include "base/task/task_traits.h" @@ -29,6 +30,8 @@ namespace paint_preview { namespace { +// Represents a finished recording of the page represented by the response and +// status of the mojo message. struct FinishedRecording { FinishedRecording(mojom::PaintPreviewStatus status, mojom::PaintPreviewCaptureResponsePtr response) @@ -45,60 +48,122 @@ struct FinishedRecording { mojom::PaintPreviewCaptureResponsePtr response; }; -FinishedRecording FinishRecording( - sk_sp<const cc::PaintRecord> recording, - const gfx::Rect& bounds, +using CapturePaintPreviewCallback = + base::OnceCallback<void(mojom::PaintPreviewStatus, + mojom::PaintPreviewCaptureResponsePtr)>; + +// Finishes building the PaintPreviewCaptureResponse mojo message and sends it +// or sends an error if the status is not `PaintPreviewStatus::kOK` +void BuildAndSendResponse(std::unique_ptr<PaintPreviewTracker> tracker, + FinishedRecording out, + CapturePaintPreviewCallback callback) { + if (out.status == mojom::PaintPreviewStatus::kOk) { + BuildResponse(tracker.get(), out.response.get(), /*log=*/true); + } + std::move(callback).Run(out.status, std::move(out.response)); +} + +// Records `skp` to `skp_file` on the threadpool to avoid blocking the main +// thread. +void RecordToFileOnThreadPool(sk_sp<const SkPicture> skp, + base::File skp_file, + std::unique_ptr<PaintPreviewTracker> tracker, + base::Optional<size_t> max_capture_size, + FinishedRecording out, + CapturePaintPreviewCallback callback) { + TRACE_EVENT0("paint_preview", "RecordToFileOnThreadPool"); + size_t serialized_size = 0; + bool success = RecordToFile(std::move(skp_file), skp, tracker.get(), + max_capture_size, &serialized_size); + out.status = success ? mojom::PaintPreviewStatus::kOk + : mojom::PaintPreviewStatus::kCaptureFailed; + out.response->serialized_size = serialized_size; + + BuildAndSendResponse(std::move(tracker), std::move(out), std::move(callback)); +} + +// Handles file persistence storage. +void SerializeFileRecording(sk_sp<const SkPicture> skp, + base::File skp_file, + std::unique_ptr<PaintPreviewTracker> tracker, + base::Optional<size_t> max_capture_size, + FinishedRecording out, + CapturePaintPreviewCallback callback) { + base::ThreadPool::PostTask( + FROM_HERE, + {base::TaskPriority::USER_VISIBLE, base::MayBlock(), + base::WithBaseSyncPrimitives()}, + BindOnce(&RecordToFileOnThreadPool, skp, std::move(skp_file), + std::move(tracker), max_capture_size, std::move(out), + base::BindPostTask(base::SequencedTaskRunnerHandle::Get(), + std::move(callback)))); +} + +// Handles memory buffer persistence storage. +void SerializeMemoryBufferRecording( + sk_sp<const SkPicture> skp, std::unique_ptr<PaintPreviewTracker> tracker, - RecordingPersistence persistence, - base::File skp_file, base::Optional<size_t> max_capture_size, - mojom::PaintPreviewCaptureResponsePtr response) { - TRACE_EVENT0("paint_preview", "FinishRecording"); - FinishedRecording out(mojom::PaintPreviewStatus::kOk, std::move(response)); + FinishedRecording out, + CapturePaintPreviewCallback callback) { + TRACE_EVENT0("paint_preview", "SerializeMemoryBufferRecording"); + size_t serialized_size = 0; + base::Optional<mojo_base::BigBuffer> buffer = + RecordToBuffer(skp, tracker.get(), max_capture_size, &serialized_size); + out.status = buffer.has_value() ? mojom::PaintPreviewStatus::kOk + : mojom::PaintPreviewStatus::kCaptureFailed; + out.response->skp.emplace(std::move(buffer.value())); + out.response->serialized_size = serialized_size; + + BuildAndSendResponse(std::move(tracker), std::move(out), std::move(callback)); +} + +// Finishes the recording process by converting the `recording` to an SkPicture. +// Serialization is then delegated based on the type of `persistence`. +void FinishRecordingOnUIThread(sk_sp<const cc::PaintRecord> recording, + const gfx::Rect& bounds, + std::unique_ptr<PaintPreviewTracker> tracker, + RecordingPersistence persistence, + base::File skp_file, + base::Optional<size_t> max_capture_size, + mojom::PaintPreviewCaptureResponsePtr response, + CapturePaintPreviewCallback callback) { + TRACE_EVENT0("paint_preview", "FinishRecordingOnUIThread"); DCHECK(tracker); if (!tracker) { - out.status = mojom::PaintPreviewStatus::kCaptureFailed; - return out; + std::move(callback).Run(mojom::PaintPreviewStatus::kCaptureFailed, + std::move(response)); + return; } TRACE_EVENT_BEGIN0("paint_preview", "ParseGlyphsAndLinks"); ParseGlyphsAndLinks(recording.get(), tracker.get()); TRACE_EVENT_END0("paint_preview", "ParseGlyphsAndLinks"); - TRACE_EVENT0("paint_preview", "SerializeAsSkPicture"); - - bool success = false; - size_t serialized_size = 0; - { - auto skp = PaintRecordToSkPicture(recording, tracker.get(), bounds); - recording.reset(); - if (!skp) { - out.status = mojom::PaintPreviewStatus::kCaptureFailed; - return out; - } - - switch (persistence) { - case RecordingPersistence::kFileSystem: - success = RecordToFile(std::move(skp_file), skp, tracker.get(), - max_capture_size, &serialized_size); - break; - case RecordingPersistence::kMemoryBuffer: - base::Optional<mojo_base::BigBuffer> buffer = RecordToBuffer( - skp, tracker.get(), max_capture_size, &serialized_size); - success = buffer.has_value(); - out.response->skp.emplace(std::move(buffer.value())); - break; - } + // This cannot be done async if the recording contains a GPU accelerated + // image. + TRACE_EVENT_BEGIN0("paint_preview", "ConvertToSkPicture"); + auto skp = PaintRecordToSkPicture(recording, tracker.get(), bounds); + recording.reset(); + if (!skp) { + std::move(callback).Run(mojom::PaintPreviewStatus::kCaptureFailed, + std::move(response)); + return; } + TRACE_EVENT_BEGIN0("paint_preview", "ConvertToSkPicture"); - if (!success) { - out.status = mojom::PaintPreviewStatus::kCaptureFailed; - return out; + FinishedRecording out(mojom::PaintPreviewStatus::kOk, std::move(response)); + switch (persistence) { + case RecordingPersistence::kFileSystem: + SerializeFileRecording(skp, std::move(skp_file), std::move(tracker), + max_capture_size, std::move(out), + std::move(callback)); + break; + case RecordingPersistence::kMemoryBuffer: + SerializeMemoryBufferRecording(skp, std::move(tracker), max_capture_size, + std::move(out), std::move(callback)); + break; } - - BuildResponse(tracker.get(), out.response.get(), /*log=*/true); - out.response->serialized_size = serialized_size; - return out; } } // namespace @@ -145,6 +210,9 @@ void PaintPreviewRecorderImpl::OnDestruct() { void PaintPreviewRecorderImpl::BindPaintPreviewRecorder( mojo::PendingAssociatedReceiver<mojom::PaintPreviewRecorder> receiver) { + // Capture requests can occur multiple times on the same frame. If the browser + // has released its endpoint and creates a new one this needs to be reset. + paint_preview_recorder_receiver_.reset(); paint_preview_recorder_receiver_.Bind(std::move(receiver)); } @@ -164,10 +232,11 @@ void PaintPreviewRecorderImpl::CapturePaintPreviewInternal( DCHECK_EQ(is_main_frame_, params->is_main_frame); // Default to using the clip rect. gfx::Rect bounds = params->clip_rect; + auto document_size = frame->DocumentSize(); + gfx::Rect document_rect = + gfx::Rect(0, 0, document_size.width, document_size.height); if (bounds.IsEmpty() || params->clip_rect_is_hint) { // If the clip rect is empty or only a hint try to use the document size. - auto size = frame->DocumentSize(); - gfx::Rect document_rect = gfx::Rect(0, 0, size.width, size.height); if (!document_rect.IsEmpty()) bounds = document_rect; @@ -182,6 +251,30 @@ void PaintPreviewRecorderImpl::CapturePaintPreviewInternal( std::move(response)); return; } + } else { + // Overflow check and confirm that that the document has a size. + if (document_rect.IsEmpty() || bounds.x() >= document_rect.width() || + bounds.y() >= document_rect.height() || bounds.x() < 0 || + bounds.y() < 0) { + std::move(callback).Run(mojom::PaintPreviewStatus::kCaptureFailed, + std::move(response)); + return; + } + + // If either width or height is 0 capture the full extent in that dimension + // while still respecting the provided x and y. + if (bounds.width() == 0) { + bounds.set_width(document_rect.width()); + } + if (bounds.height() == 0) { + bounds.set_height(document_rect.height()); + } + + // Clamp width and height to be the document size. + bounds.set_width( + std::min(document_rect.width() - bounds.x(), bounds.width())); + bounds.set_height( + std::min(document_rect.height() - bounds.y(), bounds.height())); } auto tracker = std::make_unique<PaintPreviewTracker>( @@ -248,13 +341,10 @@ void PaintPreviewRecorderImpl::CapturePaintPreviewInternal( image_ctx->max_representation_size = params->max_capture_size; } - // This cannot be done async if the recording contains a GPU accelerated - // image. - FinishedRecording recording = FinishRecording( - recorder.finishRecordingAsPicture(), bounds, std::move(tracker), - params->persistence, std::move(params->file), max_capture_size, - std::move(response)); - std::move(callback).Run(recording.status, std::move(recording.response)); + FinishRecordingOnUIThread(recorder.finishRecordingAsPicture(), bounds, + std::move(tracker), params->persistence, + std::move(params->file), max_capture_size, + std::move(response), std::move(callback)); } } // namespace paint_preview diff --git a/chromium/components/paint_preview/renderer/paint_preview_recorder_utils.cc b/chromium/components/paint_preview/renderer/paint_preview_recorder_utils.cc index 372f53e559b..9ae75412c0f 100644 --- a/chromium/components/paint_preview/renderer/paint_preview_recorder_utils.cc +++ b/chromium/components/paint_preview/renderer/paint_preview_recorder_utils.cc @@ -67,12 +67,12 @@ void ParseGlyphsAndLinks(const cc::PaintOpBuffer* buffer, } case cc::PaintOpType::SetMatrix: { auto* matrix_op = static_cast<cc::SetMatrixOp*>(*it); - tracker->SetMatrix(matrix_op->matrix); + tracker->SetMatrix(matrix_op->matrix.asM33()); break; } case cc::PaintOpType::Concat: { auto* concat_op = static_cast<cc::ConcatOp*>(*it); - tracker->Concat(concat_op->matrix); + tracker->Concat(concat_op->matrix.asM33()); break; } case cc::PaintOpType::Scale: { |