diff options
author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2020-07-16 11:45:35 +0200 |
---|---|---|
committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2020-07-17 08:59:23 +0000 |
commit | 552906b0f222c5d5dd11b9fd73829d510980461a (patch) | |
tree | 3a11e6ed0538a81dd83b20cf3a4783e297f26d91 /chromium/components/paint_preview | |
parent | 1b05827804eaf047779b597718c03e7d38344261 (diff) | |
download | qtwebengine-chromium-552906b0f222c5d5dd11b9fd73829d510980461a.tar.gz |
BASELINE: Update Chromium to 83.0.4103.122
Change-Id: Ie3a82f5bb0076eec2a7c6a6162326b4301ee291e
Reviewed-by: Michael BrĂ¼ning <michael.bruning@qt.io>
Diffstat (limited to 'chromium/components/paint_preview')
88 files changed, 3855 insertions, 788 deletions
diff --git a/chromium/components/paint_preview/DEPS b/chromium/components/paint_preview/DEPS index e05a98faa08..933597a5a10 100644 --- a/chromium/components/paint_preview/DEPS +++ b/chromium/components/paint_preview/DEPS @@ -1,6 +1,7 @@ include_rules = [ "+mojo/public/cpp", "+third_party/harfbuzz-ng/src/src", + "+third_party/harfbuzz-ng/utils/hb_scoped.h", "+third_party/skia/include/core", "+ui/gfx/geometry", ] diff --git a/chromium/components/paint_preview/browser/BUILD.gn b/chromium/components/paint_preview/browser/BUILD.gn index bc8e48867c5..90ebaf83615 100644 --- a/chromium/components/paint_preview/browser/BUILD.gn +++ b/chromium/components/paint_preview/browser/BUILD.gn @@ -6,10 +6,12 @@ import("//testing/test.gni") assert(!is_ios, "Paint Previews are not supported on iOS.") -static_library("browser") { +source_set("browser") { sources = [ "compositor_utils.cc", "compositor_utils.h", + "directory_key.cc", + "directory_key.h", "file_manager.cc", "file_manager.h", "hit_tester.cc", @@ -18,16 +20,25 @@ static_library("browser") { "paint_preview_base_service.h", "paint_preview_client.cc", "paint_preview_client.h", + "paint_preview_compositor_client_impl.cc", + "paint_preview_compositor_client_impl.h", + "paint_preview_compositor_service_impl.cc", + "paint_preview_compositor_service_impl.h", + "paint_preview_policy.h", ] deps = [ "//base", "//cc/base", "//components/discardable_memory/service", - "//components/strings:components_strings", + "//components/keyed_service/core", + "//components/strings:components_strings_grit", + "//components/ukm/content", "//content/public/browser", "//mojo/public/cpp/base", "//mojo/public/cpp/bindings", + "//services/metrics/public/cpp:metrics_cpp", + "//services/metrics/public/cpp:ukm_builders", "//third_party/blink/public:blink_headers", "//third_party/blink/public/common", "//third_party/zlib/google:zip", @@ -39,10 +50,29 @@ static_library("browser") { "//components/paint_preview/common", "//components/paint_preview/common/mojom", "//components/paint_preview/common/proto", + "//components/paint_preview/public", "//components/services/paint_preview_compositor/public/mojom", ] } +source_set("test_support") { + testonly = true + sources = [ + "paint_preview_base_service_test_factory.cc", + "paint_preview_base_service_test_factory.h", + "test_paint_preview_policy.cc", + "test_paint_preview_policy.h", + ] + + deps = [ + "//base", + "//components/keyed_service/core", + "//content/public/browser", + ] + + public_deps = [ ":browser" ] +} + source_set("unit_tests") { testonly = true @@ -55,6 +85,7 @@ source_set("unit_tests") { deps = [ ":browser", + ":test_support", "//base", "//base/test:test_support", "//components/paint_preview/common:test_utils", diff --git a/chromium/components/paint_preview/browser/DEPS b/chromium/components/paint_preview/browser/DEPS index fcca51dccf8..aecb2ff99a9 100644 --- a/chromium/components/paint_preview/browser/DEPS +++ b/chromium/components/paint_preview/browser/DEPS @@ -1,11 +1,14 @@ include_rules = [ "+cc/base", "+components/discardable_memory/service", + "+components/keyed_service/core", "+components/services/paint_preview_compositor/public/mojom", "+components/strings/grit/components_strings.h", - "+components/keyed_service/core", + "+components/ukm/content", + "+components/ukm/test_ukm_recorder.h", "+content/public/browser", "+content/public/test", + "+services/metrics/public/cpp", "+third_party/blink/public/common", "+third_party/zlib/google", ] diff --git a/chromium/components/paint_preview/browser/android/BUILD.gn b/chromium/components/paint_preview/browser/android/BUILD.gn new file mode 100644 index 00000000000..5885b049a34 --- /dev/null +++ b/chromium/components/paint_preview/browser/android/BUILD.gn @@ -0,0 +1,67 @@ +# 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. + +import("//build/config/android/rules.gni") + +assert(is_android, "This directory should only be compiled for Android.") + +generate_jni("jni_headers") { + sources = [ "java/src/org/chromium/components/paintpreview/browser/PaintPreviewUtils.java" ] +} + +android_library("java") { + annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ] + + sources = [ + "java/src/org/chromium/components/paintpreview/browser/NativePaintPreviewServiceProvider.java", + "java/src/org/chromium/components/paintpreview/browser/PaintPreviewUtils.java", + ] + + deps = [ + "//base:base_java", + "//base:jni_java", + "//content/public/android:content_java", + ] +} + +source_set("android") { + sources = [ + "paint_preview_utils.cc", + "paint_preview_utils.h", + ] + + deps = [ + ":jni_headers", + "//base", + "//components/paint_preview/browser", + "//components/paint_preview/buildflags", + "//components/ukm/content", + "//content/public/browser", + "//services/metrics/public/cpp:ukm_builders", + ] +} + +source_set("unit_tests") { + testonly = true + + sources = [ "paint_preview_utils_unittest.cc" ] + + deps = [ + ":android", + "//base", + "//base/test:test_support", + "//components/paint_preview/common:test_utils", + "//components/paint_preview/common/mojom", + "//components/ukm:test_support", + "//components/ukm/content", + "//content/public/browser", + "//content/test:test_support", + "//services/metrics/public/cpp:ukm_builders", + "//testing/gmock", + "//testing/gtest", + "//third_party/blink/public:blink_headers", + "//third_party/blink/public/common", + "//third_party/zlib/google:zip", + ] +} diff --git a/chromium/components/paint_preview/browser/android/java/DEPS b/chromium/components/paint_preview/browser/android/java/DEPS new file mode 100644 index 00000000000..5beb9bb3398 --- /dev/null +++ b/chromium/components/paint_preview/browser/android/java/DEPS @@ -0,0 +1,3 @@ +include_rules = [ + "+content/public/android/java/src/org/chromium/content_public/browser/WebContents.java", +] diff --git a/chromium/components/paint_preview/browser/android/java/src/org/chromium/components/paintpreview/browser/NativePaintPreviewServiceProvider.java b/chromium/components/paint_preview/browser/android/java/src/org/chromium/components/paintpreview/browser/NativePaintPreviewServiceProvider.java new file mode 100644 index 00000000000..ffbf946db24 --- /dev/null +++ b/chromium/components/paint_preview/browser/android/java/src/org/chromium/components/paintpreview/browser/NativePaintPreviewServiceProvider.java @@ -0,0 +1,11 @@ +// Copyright 2019 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. + +package org.chromium.components.paintpreview.browser; + +/** + * The Java-side implementations of paint_preview_base_service.cc should implement this interface. + * Provides a method for accessing the native PaintPreviewBaseService. + */ +public interface NativePaintPreviewServiceProvider { long getNativeService(); }
\ No newline at end of file diff --git a/chromium/components/paint_preview/browser/android/java/src/org/chromium/components/paintpreview/browser/PaintPreviewUtils.java b/chromium/components/paint_preview/browser/android/java/src/org/chromium/components/paintpreview/browser/PaintPreviewUtils.java new file mode 100644 index 00000000000..5e4fcdcc528 --- /dev/null +++ b/chromium/components/paint_preview/browser/android/java/src/org/chromium/components/paintpreview/browser/PaintPreviewUtils.java @@ -0,0 +1,26 @@ +// 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. + +package org.chromium.components.paintpreview.browser; + +import org.chromium.base.annotations.NativeMethods; +import org.chromium.content_public.browser.WebContents; + +/** + * Helper to capture paint previews via native. + */ +public class PaintPreviewUtils { + /** + * Captures a paint preview of the passed contents. + * @param contents The WebContents of the page to capture. + */ + public static void capturePaintPreview(WebContents contents) { + PaintPreviewUtilsJni.get().capturePaintPreview(contents); + } + + @NativeMethods + interface Natives { + void capturePaintPreview(WebContents webContents); + } +} diff --git a/chromium/components/paint_preview/browser/android/paint_preview_utils.cc b/chromium/components/paint_preview/browser/android/paint_preview_utils.cc new file mode 100644 index 00000000000..873464a03b0 --- /dev/null +++ b/chromium/components/paint_preview/browser/android/paint_preview_utils.cc @@ -0,0 +1,210 @@ +// 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/android/paint_preview_utils.h" + +#include <jni.h> + +#include <utility> + +#include "base/bind.h" +#include "base/bind_helpers.h" +#include "base/callback.h" +#include "base/files/file_util.h" +#include "base/logging.h" +#include "base/metrics/histogram_functions.h" +#include "base/task/post_task.h" +#include "base/task/thread_pool.h" +#include "base/unguessable_token.h" +#include "components/paint_preview/browser/android/jni_headers/PaintPreviewUtils_jni.h" +#include "components/paint_preview/browser/file_manager.h" +#include "components/paint_preview/browser/paint_preview_client.h" +#include "components/paint_preview/buildflags/buildflags.h" +#include "components/ukm/content/source_url_recorder.h" +#include "content/public/browser/browser_context.h" +#include "content/public/browser/web_contents.h" +#include "content/public/browser/web_contents_observer.h" +#include "content/public/browser/web_contents_user_data.h" +#include "services/metrics/public/cpp/metrics_utils.h" +#include "services/metrics/public/cpp/ukm_builders.h" +#include "services/metrics/public/cpp/ukm_recorder.h" + +namespace paint_preview { + +namespace { + +const char kPaintPreviewTestTag[] = "PaintPreviewTest "; +const char kPaintPreviewDir[] = "paint_preview"; +const char kCaptureTestDir[] = "capture_test"; + +struct CaptureMetrics { + base::TimeDelta capture_time; + ukm::SourceId source_id; +}; + +void CleanupOnFailure(const base::FilePath& root_dir, + FinishedCallback finished) { + VLOG(1) << kPaintPreviewTestTag << "Capture Failed\n"; + base::DeleteFileRecursively(root_dir); + std::move(finished).Run(base::nullopt); +} + +void CleanupAndLogResult(const base::FilePath& zip_path, + const CaptureMetrics& metrics, + FinishedCallback finished, + bool keep_zip, + size_t compressed_size_bytes) { + VLOG(1) << kPaintPreviewTestTag << "Capture Finished Successfully:\n" + << "Compressed size " << compressed_size_bytes << " bytes\n" + << "Time taken in native " << metrics.capture_time.InMilliseconds() + << " ms"; + + if (!keep_zip) + base::DeleteFileRecursively(zip_path.DirName()); + + base::UmaHistogramMemoryKB( + "Browser.PaintPreview.CaptureExperiment.CompressedOnDiskSize", + compressed_size_bytes / 1000); + if (metrics.source_id != ukm::kInvalidSourceId) { + ukm::builders::PaintPreviewCapture(metrics.source_id) + .SetCompressedOnDiskSize( + ukm::GetExponentialBucketMinForBytes(compressed_size_bytes)) + .Record(ukm::UkmRecorder::Get()); + } + std::move(finished).Run(zip_path); +} + +void MeasureSize(scoped_refptr<FileManager> manager, + const DirectoryKey& key, + const base::FilePath& root_dir, + CaptureMetrics metrics, + FinishedCallback finished, + bool keep_zip, + bool success) { + if (!success) { + CleanupOnFailure(root_dir, std::move(finished)); + return; + } + manager->GetTaskRunner()->PostTaskAndReplyWithResult( + FROM_HERE, base::BindOnce(&FileManager::GetSizeOfArtifacts, manager, key), + base::BindOnce( + &CleanupAndLogResult, + root_dir.AppendASCII(key.AsciiDirname()).AddExtensionASCII("zip"), + metrics, std::move(finished), keep_zip)); +} + +void OnCaptured(scoped_refptr<FileManager> manager, + const DirectoryKey& key, + base::TimeTicks start_time, + const base::FilePath& root_dir, + ukm::SourceId source_id, + FinishedCallback finished, + bool keep_zip, + base::UnguessableToken guid, + mojom::PaintPreviewStatus status, + std::unique_ptr<PaintPreviewProto> proto) { + base::TimeDelta time_delta = base::TimeTicks::Now() - start_time; + + bool success = (status == mojom::PaintPreviewStatus::kOk); + base::UmaHistogramBoolean("Browser.PaintPreview.CaptureExperiment.Success", + success); + if (!success || !proto) { + base::ThreadPool::PostTask( + FROM_HERE, {base::MayBlock()}, + base::BindOnce(&CleanupOnFailure, root_dir, std::move(finished))); + return; + } + + CaptureMetrics result = {time_delta, source_id}; + manager->GetTaskRunner()->PostTaskAndReplyWithResult( + FROM_HERE, + base::BindOnce(&FileManager::SerializePaintPreviewProto, manager, key, + *proto, true), + base::BindOnce(&MeasureSize, manager, key, root_dir, result, + std::move(finished), keep_zip)); +} + +void InitiateCapture(scoped_refptr<FileManager> manager, + const DirectoryKey& key, + int frame_tree_node_id, + FinishedCallback finished, + bool keep_zip, + const base::Optional<base::FilePath>& url_path) { + auto* contents = + content::WebContents::FromFrameTreeNodeId(frame_tree_node_id); + if (!url_path.has_value() || !contents) { + std::move(finished).Run(base::nullopt); + return; + } + + auto* client = PaintPreviewClient::FromWebContents(contents); + if (!client) { + VLOG(1) << kPaintPreviewTestTag << "Failure: client could not be created."; + CleanupOnFailure(url_path->DirName(), std::move(finished)); + return; + } + + PaintPreviewClient::PaintPreviewParams params; + params.document_guid = base::UnguessableToken::Create(); + params.is_main_frame = true; + params.root_dir = url_path.value(); + + ukm::SourceId source_id = ukm::GetSourceIdForWebContentsDocument(contents); + auto start_time = base::TimeTicks::Now(); + client->CapturePaintPreview( + params, contents->GetMainFrame(), + base::BindOnce(&OnCaptured, manager, key, start_time, + params.root_dir.DirName(), source_id, std::move(finished), + keep_zip)); +} + +} // namespace + +void Capture(content::WebContents* contents, + FinishedCallback finished, + bool keep_zip) { + PaintPreviewClient::CreateForWebContents(contents); + base::FilePath root_path = contents->GetBrowserContext() + ->GetPath() + .AppendASCII(kPaintPreviewDir) + .AppendASCII(kCaptureTestDir); + auto manager = base::MakeRefCounted<FileManager>( + root_path, base::ThreadPool::CreateSequencedTaskRunner( + {base::MayBlock(), base::TaskPriority::USER_VISIBLE, + base::TaskShutdownBehavior::BLOCK_SHUTDOWN, + base::ThreadPolicy::MUST_USE_FOREGROUND})); + + auto key = manager->CreateKey(contents->GetLastCommittedURL()); + manager->GetTaskRunner()->PostTaskAndReplyWithResult( + FROM_HERE, + base::BindOnce(&FileManager::CreateOrGetDirectory, manager, key, true), + base::BindOnce(&InitiateCapture, manager, key, + contents->GetMainFrame()->GetFrameTreeNodeId(), + std::move(finished), keep_zip)); +} + +} // namespace paint_preview + +// If the ENABLE_PAINT_PREVIEW buildflags is set this method will trigger a +// series of actions; +// 1. Capture a paint preview via the client and measure the time taken. +// 2. Zip a folder containing the artifacts and measure the size of the zip. +// 3. Delete the resulting zip archive. +// 4. Log the results. +// If the buildflag is not set this is just a stub. +static void JNI_PaintPreviewUtils_CapturePaintPreview( + JNIEnv* env, + const base::android::JavaParamRef<jobject>& jweb_contents) { +#if BUILDFLAG(ENABLE_PAINT_PREVIEW) + auto* contents = content::WebContents::FromJavaWebContents(jweb_contents); + paint_preview::Capture(contents, base::DoNothing(), /* keep_zip= */ false); +#else + // In theory this is unreachable as the codepath to reach here is only exposed + // if the buildflag for ENABLE_PAINT_PREVIEW is set. However, this function + // will still be compiled as it is called from JNI so this is here as a + // placeholder. + VLOG(1) << paint_preview::kPaintPreviewTestTag + << "Failure: compiled without buildflag ENABLE_PAINT_PREVIEW."; +#endif // BUILDFLAG(ENABLE_PAINT_PREVIEW) +} diff --git a/chromium/components/paint_preview/browser/android/paint_preview_utils.h b/chromium/components/paint_preview/browser/android/paint_preview_utils.h new file mode 100644 index 00000000000..d5176805e62 --- /dev/null +++ b/chromium/components/paint_preview/browser/android/paint_preview_utils.h @@ -0,0 +1,31 @@ +// 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_ANDROID_PAINT_PREVIEW_UTILS_H_ +#define COMPONENTS_PAINT_PREVIEW_BROWSER_ANDROID_PAINT_PREVIEW_UTILS_H_ + +#include "base/callback_forward.h" +#include "base/files/file_path.h" +#include "base/optional.h" + +namespace content { +class WebContents; +} // namespace content + +namespace paint_preview { + +using FinishedCallback = + base::OnceCallback<void(const base::Optional<base::FilePath>&)>; + +// Captures a paint preview of |contents|. On completion returns the path of the +// zip archive in which the paint preview is stored via |finished| (or an empty +// path if the capture failed). The zip archive will exist only if |keep_zip| is +// true. +void Capture(content::WebContents* contents, + FinishedCallback finished, + bool keep_zip); + +} // namespace paint_preview + +#endif // COMPONENTS_PAINT_PREVIEW_BROWSER_ANDROID_PAINT_PREVIEW_UTILS_H_ diff --git a/chromium/components/paint_preview/browser/android/paint_preview_utils_unittest.cc b/chromium/components/paint_preview/browser/android/paint_preview_utils_unittest.cc new file mode 100644 index 00000000000..48b26b44a97 --- /dev/null +++ b/chromium/components/paint_preview/browser/android/paint_preview_utils_unittest.cc @@ -0,0 +1,211 @@ +// 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/android/paint_preview_utils.h" + +#include <utility> + +#include "base/files/file_enumerator.h" +#include "base/files/file_path.h" +#include "base/files/file_util.h" +#include "components/paint_preview/common/mojom/paint_preview_recorder.mojom.h" +#include "components/ukm/content/source_url_recorder.h" +#include "components/ukm/test_ukm_recorder.h" +#include "content/public/browser/render_frame_host.h" +#include "content/public/browser/web_contents.h" +#include "content/public/test/navigation_simulator.h" +#include "content/public/test/test_renderer_host.h" +#include "mojo/public/cpp/bindings/associated_receiver.h" +#include "services/metrics/public/cpp/ukm_builders.h" +#include "services/metrics/public/cpp/ukm_recorder.h" +#include "services/metrics/public/cpp/ukm_source.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h" +#include "third_party/zlib/google/zip.h" + +namespace paint_preview { + +namespace { + +class MockPaintPreviewRecorder : public mojom::PaintPreviewRecorder { + public: + MockPaintPreviewRecorder() = default; + ~MockPaintPreviewRecorder() override = default; + + MockPaintPreviewRecorder(const MockPaintPreviewRecorder&) = delete; + MockPaintPreviewRecorder& operator=(const MockPaintPreviewRecorder&) = delete; + + void CapturePaintPreview( + mojom::PaintPreviewCaptureParamsPtr params, + mojom::PaintPreviewRecorder::CapturePaintPreviewCallback callback) + override { + std::move(callback).Run(status_, mojom::PaintPreviewCaptureResponse::New()); + } + + void SetResponseStatus(mojom::PaintPreviewStatus status) { status_ = status; } + + void BindRequest(mojo::ScopedInterfaceEndpointHandle handle) { + binding_.Bind(mojo::PendingAssociatedReceiver<mojom::PaintPreviewRecorder>( + std::move(handle))); + } + + private: + mojom::PaintPreviewStatus status_; + mojo::AssociatedReceiver<mojom::PaintPreviewRecorder> binding_{this}; +}; + +base::Optional<base::FilePath> Unzip(const base::FilePath& zip) { + base::FilePath dst_path = zip.RemoveExtension(); + if (!base::CreateDirectory(dst_path)) + return base::nullopt; + if (!zip::Unzip(zip, dst_path)) + return base::nullopt; + base::DeleteFileRecursively(zip); + return dst_path; +} + +} // namespace + +class PaintPreviewUtilsRenderViewHostTest + : public content::RenderViewHostTestHarness { + public: + PaintPreviewUtilsRenderViewHostTest() {} + + PaintPreviewUtilsRenderViewHostTest( + const PaintPreviewUtilsRenderViewHostTest&) = delete; + PaintPreviewUtilsRenderViewHostTest& operator=( + const PaintPreviewUtilsRenderViewHostTest&) = delete; + + protected: + void SetUp() override { + RenderViewHostTestHarness::SetUp(); + content::RenderFrameHostTester::For(main_rfh()) + ->InitializeRenderFrameIfNeeded(); + ukm::InitializeSourceUrlRecorderForWebContents(web_contents()); + } + + void OverrideInterface(MockPaintPreviewRecorder* service) { + blink::AssociatedInterfaceProvider* remote_interfaces = + web_contents()->GetMainFrame()->GetRemoteAssociatedInterfaces(); + remote_interfaces->OverrideBinderForTesting( + mojom::PaintPreviewRecorder::Name_, + base::BindRepeating(&MockPaintPreviewRecorder::BindRequest, + base::Unretained(service))); + } +}; + +TEST_F(PaintPreviewUtilsRenderViewHostTest, CaptureSingleFrameAndKeep) { + content::NavigationSimulator::NavigateAndCommitFromBrowser( + web_contents(), GURL("http://www.example.com")); + auto* contents = content::WebContents::FromRenderFrameHost(main_rfh()); + ukm::TestAutoSetUkmRecorder ukm_recorder; + + MockPaintPreviewRecorder service; + service.SetResponseStatus(mojom::PaintPreviewStatus::kOk); + OverrideInterface(&service); + + base::RunLoop loop; + Capture(contents, + base::BindOnce( + [](base::OnceClosure quit, + const base::Optional<base::FilePath>& maybe_zip_path) { + EXPECT_TRUE(maybe_zip_path.has_value()); + + const base::FilePath& zip_path = maybe_zip_path.value(); + EXPECT_EQ(".zip", zip_path.Extension()); + { + base::ScopedAllowBlockingForTesting scope; + EXPECT_TRUE(base::PathExists(zip_path)); + auto unzipped_path = Unzip(zip_path); + EXPECT_TRUE(unzipped_path.has_value()); + + base::FileEnumerator enumerate(unzipped_path.value(), false, + base::FileEnumerator::FILES); + size_t count = 0; + bool has_proto = false; + bool has_skp = false; + for (base::FilePath name = enumerate.Next(); !name.empty(); + name = enumerate.Next(), ++count) { + if (name.Extension() == ".skp") + has_skp = true; + if (name.BaseName().AsUTF8Unsafe() == "proto.pb") + has_proto = true; + } + EXPECT_EQ(2U, count); + EXPECT_TRUE(has_skp); + EXPECT_TRUE(has_proto); + base::DeleteFileRecursively(zip_path.DirName()); + } + std::move(quit).Run(); + }, + loop.QuitClosure()), + true); + loop.Run(); + + auto entries = ukm_recorder.GetEntriesByName( + ukm::builders::PaintPreviewCapture::kEntryName); + EXPECT_EQ(2U, entries.size()); +} + +TEST_F(PaintPreviewUtilsRenderViewHostTest, CaptureSingleFrameAndDelete) { + content::NavigationSimulator::NavigateAndCommitFromBrowser( + web_contents(), GURL("http://www.example.com")); + auto* contents = content::WebContents::FromRenderFrameHost(main_rfh()); + ukm::TestAutoSetUkmRecorder ukm_recorder; + + MockPaintPreviewRecorder service; + service.SetResponseStatus(mojom::PaintPreviewStatus::kOk); + OverrideInterface(&service); + + base::RunLoop loop; + Capture(contents, + base::BindOnce( + [](base::OnceClosure quit, + const base::Optional<base::FilePath>& maybe_zip_path) { + EXPECT_TRUE(maybe_zip_path.has_value()); + const base::FilePath& zip_path = maybe_zip_path.value(); + { + base::ScopedAllowBlockingForTesting scope; + EXPECT_FALSE(base::PathExists(zip_path)); + EXPECT_FALSE(base::DirectoryExists(zip_path.DirName())); + } + std::move(quit).Run(); + }, + loop.QuitClosure()), + false); + loop.Run(); + + auto entries = ukm_recorder.GetEntriesByName( + ukm::builders::PaintPreviewCapture::kEntryName); + EXPECT_EQ(2U, entries.size()); +} + +TEST_F(PaintPreviewUtilsRenderViewHostTest, SingleFrameFailure) { + content::NavigationSimulator::NavigateAndCommitFromBrowser( + web_contents(), GURL("http://www.example.com")); + auto* contents = content::WebContents::FromRenderFrameHost(main_rfh()); + ukm::TestAutoSetUkmRecorder ukm_recorder; + + MockPaintPreviewRecorder service; + service.SetResponseStatus(mojom::PaintPreviewStatus::kFailed); + OverrideInterface(&service); + + base::RunLoop loop; + Capture(contents, + base::BindOnce( + [](base::OnceClosure quit, + const base::Optional<base::FilePath>& path) { + EXPECT_FALSE(path.has_value()); + std::move(quit).Run(); + }, + loop.QuitClosure()), + false); + loop.Run(); + + auto entries = ukm_recorder.GetEntriesByName( + ukm::builders::PaintPreviewCapture::kEntryName); + EXPECT_EQ(0U, entries.size()); +} + +} // namespace paint_preview diff --git a/chromium/components/paint_preview/browser/compositor_utils.cc b/chromium/components/paint_preview/browser/compositor_utils.cc index 1c163eb80f0..6e8af2d5a14 100644 --- a/chromium/components/paint_preview/browser/compositor_utils.cc +++ b/chromium/components/paint_preview/browser/compositor_utils.cc @@ -8,6 +8,7 @@ #include "base/bind.h" #include "base/task/post_task.h" +#include "build/build_config.h" #include "components/discardable_memory/service/discardable_shared_memory_manager.h" #include "components/services/paint_preview_compositor/public/mojom/paint_preview_compositor.mojom.h" #include "components/strings/grit/components_strings.h" @@ -36,7 +37,7 @@ CreateCompositorCollection() { mojom::PaintPreviewCompositorCollection>( content::ServiceProcessHost::Options() .WithDisplayName(IDS_PAINT_PREVIEW_COMPOSITOR_SERVICE_DISPLAY_NAME) - .WithSandboxType(service_manager::SANDBOX_TYPE_PDF_COMPOSITOR) + .WithSandboxType(service_manager::SandboxType::kPrintCompositor) .Pass()); mojo::PendingRemote<discardable_memory::mojom::DiscardableSharedMemoryManager> discardable_memory_manager; diff --git a/chromium/components/paint_preview/browser/directory_key.cc b/chromium/components/paint_preview/browser/directory_key.cc new file mode 100644 index 00000000000..819e98c4bb9 --- /dev/null +++ b/chromium/components/paint_preview/browser/directory_key.cc @@ -0,0 +1,13 @@ +// 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/directory_key.h" + +namespace paint_preview { + +bool operator<(const DirectoryKey& a, const DirectoryKey& b) { + return a.AsciiDirname() < b.AsciiDirname(); +} + +} // namespace paint_preview diff --git a/chromium/components/paint_preview/browser/directory_key.h b/chromium/components/paint_preview/browser/directory_key.h new file mode 100644 index 00000000000..a24b66afa29 --- /dev/null +++ b/chromium/components/paint_preview/browser/directory_key.h @@ -0,0 +1,32 @@ +// 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_DIRECTORY_KEY_H_ +#define COMPONENTS_PAINT_PREVIEW_BROWSER_DIRECTORY_KEY_H_ + +#include <string> + +#include "base/strings/string_piece.h" + +namespace paint_preview { + +// A key for a specific subdirectory (== captured paint preview). +class DirectoryKey { + public: + DirectoryKey() = default; + explicit DirectoryKey(base::StringPiece ascii_dirname) + : ascii_dirname_(ascii_dirname) {} + ~DirectoryKey() = default; + + const std::string& AsciiDirname() const { return ascii_dirname_; } + + private: + std::string ascii_dirname_; +}; + +bool operator<(const DirectoryKey& a, const DirectoryKey& b); + +} // namespace paint_preview + +#endif // COMPONENTS_PAINT_PREVIEW_BROWSER_DIRECTORY_KEY_H_ diff --git a/chromium/components/paint_preview/browser/file_manager.cc b/chromium/components/paint_preview/browser/file_manager.cc index f209f8925c7..70fc611b3f2 100644 --- a/chromium/components/paint_preview/browser/file_manager.cc +++ b/chromium/components/paint_preview/browser/file_manager.cc @@ -7,34 +7,43 @@ #include "base/files/file_enumerator.h" #include "base/files/file_util.h" #include "base/hash/hash.h" -#include "base/strings/strcat.h" #include "base/strings/string_number_conversions.h" +#include "components/paint_preview/common/file_utils.h" #include "third_party/zlib/google/zip.h" namespace paint_preview { namespace { +constexpr char kProtoName[] = "proto.pb"; constexpr char kZipExt[] = ".zip"; -std::string HashToHex(const GURL& url) { - uint32_t hash = base::PersistentHash(url.spec()); - return base::HexEncode(&hash, sizeof(uint32_t)); -} - } // namespace -FileManager::FileManager(const base::FilePath& root_directory) - : root_directory_(root_directory) {} +FileManager::FileManager( + const base::FilePath& root_directory, + scoped_refptr<base::SequencedTaskRunner> io_task_runner) + : root_directory_(root_directory), io_task_runner_(io_task_runner) {} + FileManager::~FileManager() = default; -size_t FileManager::GetSizeOfArtifactsFor(const GURL& url) { +DirectoryKey FileManager::CreateKey(const GURL& url) const { + uint32_t hash = base::PersistentHash(url.spec()); + return DirectoryKey{base::HexEncode(&hash, sizeof(uint32_t))}; +} + +DirectoryKey FileManager::CreateKey(uint64_t tab_id) const { + return DirectoryKey{base::NumberToString(tab_id)}; +} + +size_t FileManager::GetSizeOfArtifacts(const DirectoryKey& key) const { + DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); base::FilePath path; - StorageType storage_type = GetPathForUrl(url, &path); + StorageType storage_type = GetPathForKey(key, &path); switch (storage_type) { case kDirectory: { return base::ComputeDirectorySize( - root_directory_.AppendASCII(HashToHex(url))); + root_directory_.AppendASCII(key.AsciiDirname())); } case kZip: { int64_t file_size = 0; @@ -48,75 +57,86 @@ size_t FileManager::GetSizeOfArtifactsFor(const GURL& url) { } } -bool FileManager::GetCreatedTime(const GURL& url, base::Time* created_time) { +base::Optional<base::File::Info> FileManager::GetInfo( + const DirectoryKey& key) const { + DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); base::FilePath path; - StorageType storage_type = GetPathForUrl(url, &path); + StorageType storage_type = GetPathForKey(key, &path); if (storage_type == FileManager::StorageType::kNone) - return false; + return base::nullopt; base::File::Info info; if (!base::GetFileInfo(path, &info)) - return false; - *created_time = info.creation_time; - return true; + return base::nullopt; + return info; } -bool FileManager::GetLastModifiedTime(const GURL& url, - base::Time* last_modified_time) { +bool FileManager::DirectoryExists(const DirectoryKey& key) const { + DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); base::FilePath path; - StorageType storage_type = GetPathForUrl(url, &path); - if (storage_type == FileManager::StorageType::kNone) - return false; - base::File::Info info; - if (!base::GetFileInfo(path, &info)) - return false; - *last_modified_time = info.last_modified; - return true; + return GetPathForKey(key, &path) != StorageType::kNone; } -bool FileManager::CreateOrGetDirectoryFor(const GURL& url, - base::FilePath* directory) { +bool FileManager::CaptureExists(const DirectoryKey& key) const { + DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); base::FilePath path; - StorageType storage_type = GetPathForUrl(url, &path); + StorageType storage_type = GetPathForKey(key, &path); + switch (storage_type) { + case kDirectory: + return base::PathExists(path.AppendASCII(kProtoName)); + case kZip: + return true; + case kNone: // fallthrough; + default: + return false; + } +} + +base::Optional<base::FilePath> FileManager::CreateOrGetDirectory( + const DirectoryKey& key, + bool clear) const { + DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); + if (clear) + DeleteArtifactSet(key); + + base::FilePath path; + StorageType storage_type = GetPathForKey(key, &path); switch (storage_type) { case kNone: { - base::FilePath new_path = root_directory_.AppendASCII(HashToHex(url)); + base::FilePath new_path = root_directory_.AppendASCII(key.AsciiDirname()); base::File::Error error = base::File::FILE_OK; if (base::CreateDirectoryAndGetError(new_path, &error)) { - *directory = new_path; - return true; + return new_path; } DVLOG(1) << "ERROR: failed to create directory: " << path << " with error code " << error; - return false; - } - case kDirectory: { - *directory = path; - return true; + return base::nullopt; } + case kDirectory: + return path; case kZip: { - base::FilePath dst_path = root_directory_.AppendASCII(HashToHex(url)); + base::FilePath dst_path = root_directory_.AppendASCII(key.AsciiDirname()); base::File::Error error = base::File::FILE_OK; if (!base::CreateDirectoryAndGetError(dst_path, &error)) { DVLOG(1) << "ERROR: failed to create directory: " << path << " with error code " << error; - return false; + return base::nullopt; } if (!zip::Unzip(path, dst_path)) { DVLOG(1) << "ERROR: failed to unzip: " << path << " to " << dst_path; - return false; + return base::nullopt; } - base::DeleteFile(path, true); - *directory = dst_path; - return true; + base::DeleteFileRecursively(path); + return dst_path; } default: - return false; + return base::nullopt; } } -bool FileManager::CompressDirectoryFor(const GURL& url) { +bool FileManager::CompressDirectory(const DirectoryKey& key) const { + DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); base::FilePath path; - StorageType storage_type = GetPathForUrl(url, &path); + StorageType storage_type = GetPathForKey(key, &path); switch (storage_type) { case kDirectory: { // If there are no files in the directory, zip will succeed, but unzip @@ -126,7 +146,7 @@ bool FileManager::CompressDirectoryFor(const GURL& url) { base::FilePath dst_path = path.AddExtensionASCII(kZipExt); if (!zip::Zip(path, dst_path, /* hidden files */ true)) return false; - base::DeleteFile(path, true); + base::DeleteFileRecursively(path); return true; } case kZip: @@ -137,23 +157,66 @@ bool FileManager::CompressDirectoryFor(const GURL& url) { } } -void FileManager::DeleteArtifactsFor(const std::vector<GURL>& urls) { - for (const auto& url : urls) { - base::FilePath path; - StorageType storage_type = GetPathForUrl(url, &path); - if (storage_type == FileManager::StorageType::kNone) - continue; - base::DeleteFile(path, true); - } +void FileManager::DeleteArtifactSet(const DirectoryKey& key) const { + DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); + base::FilePath path; + StorageType storage_type = GetPathForKey(key, &path); + if (storage_type == FileManager::StorageType::kNone) + return; + base::DeleteFileRecursively(path); +} + +void FileManager::DeleteArtifactSets( + const std::vector<DirectoryKey>& keys) const { + DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); + for (const auto& key : keys) + DeleteArtifactSet(key); } -void FileManager::DeleteAll() { +void FileManager::DeleteAll() const { + DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); base::DeleteFileRecursively(root_directory_); } -FileManager::StorageType FileManager::GetPathForUrl(const GURL& url, - base::FilePath* path) { - base::FilePath directory_path = root_directory_.AppendASCII(HashToHex(url)); +bool FileManager::SerializePaintPreviewProto(const DirectoryKey& key, + const PaintPreviewProto& proto, + bool compress) const { + DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); + auto path = CreateOrGetDirectory(key, false); + if (!path.has_value()) + return false; + return WriteProtoToFile(path->AppendASCII(kProtoName), proto) && + (!compress || CompressDirectory(key)); +} + +std::unique_ptr<PaintPreviewProto> FileManager::DeserializePaintPreviewProto( + const DirectoryKey& key) const { + DCHECK(io_task_runner_->RunsTasksInCurrentSequence()); + auto path = CreateOrGetDirectory(key, false); + if (!path.has_value()) + return nullptr; + return ReadProtoFromFile(path->AppendASCII(kProtoName)); +} + +base::flat_set<DirectoryKey> FileManager::ListUsedKeys() const { + base::FileEnumerator enumerator( + root_directory_, + /*recursive=*/false, + base::FileEnumerator::FILES | base::FileEnumerator::DIRECTORIES); + std::vector<DirectoryKey> keys; + for (base::FilePath name = enumerator.Next(); !name.empty(); + name = enumerator.Next()) { + keys.push_back( + DirectoryKey{name.BaseName().RemoveExtension().MaybeAsASCII()}); + } + return base::flat_set<DirectoryKey>(std::move(keys)); +} + +FileManager::StorageType FileManager::GetPathForKey( + const DirectoryKey& key, + base::FilePath* path) const { + base::FilePath directory_path = + root_directory_.AppendASCII(key.AsciiDirname()); if (base::PathExists(directory_path)) { *path = directory_path; return kDirectory; diff --git a/chromium/components/paint_preview/browser/file_manager.h b/chromium/components/paint_preview/browser/file_manager.h index f64d94f066a..3d9173e4ce4 100644 --- a/chromium/components/paint_preview/browser/file_manager.h +++ b/chromium/components/paint_preview/browser/file_manager.h @@ -5,57 +5,103 @@ #ifndef COMPONENTS_PAINT_PREVIEW_BROWSER_FILE_MANAGER_H_ #define COMPONENTS_PAINT_PREVIEW_BROWSER_FILE_MANAGER_H_ +#include "base/containers/flat_set.h" +#include "base/files/file.h" #include "base/files/file_path.h" +#include "base/memory/ref_counted.h" +#include "base/sequenced_task_runner.h" #include "base/time/time.h" +#include "components/paint_preview/browser/directory_key.h" +#include "components/paint_preview/common/proto/paint_preview.pb.h" #include "url/gurl.h" namespace paint_preview { -// Manages paint preview files associated with a root directory (typically a -// user profile). -class FileManager { +// FileManager manages paint preview files associated with a root directory. +// Typically the root directory is <profile_dir>/paint_previews/<feature>. +// +// This class is refcounted so scheduled tasks may continue during shutdown. +class FileManager : public base::RefCountedThreadSafe<FileManager> { public: // Create a file manager for |root_directory|. Top level items in // |root_directoy| should be exclusively managed by this class. Items within - // the subdirectories it creates can be freely modified. - explicit FileManager(const base::FilePath& root_directory); - ~FileManager(); + // the subdirectories it creates can be freely modified. All methods will be + // should be posted to the |io_task_runner| thread. + FileManager(const base::FilePath& root_directory, + scoped_refptr<base::SequencedTaskRunner> io_task_runner); + + FileManager(const FileManager&) = delete; + FileManager& operator=(const FileManager&) = delete; + + scoped_refptr<base::SequencedTaskRunner> GetTaskRunner() { + return io_task_runner_; + } + + // Creates a DirectoryKey from keying material. + // TODO(crbug/1056226): implement collision resolution. At present collisions + // result in overwriting data. + DirectoryKey CreateKey(const GURL& url) const; + DirectoryKey CreateKey(uint64_t tab_id) const; // Get statistics about the time of creation and size of artifacts. - size_t GetSizeOfArtifactsFor(const GURL& url); - bool GetCreatedTime(const GURL& url, base::Time* created_time); - bool GetLastModifiedTime(const GURL& url, base::Time* last_modified_time); + size_t GetSizeOfArtifacts(const DirectoryKey& key) const; + base::Optional<base::File::Info> GetInfo(const DirectoryKey& key) const; + + // Returns true if the directory for |key| exists. + bool DirectoryExists(const DirectoryKey& key) const; + + // Returns true if there is a capture for |key| i.e. there exists a proto or + // zip file. + bool CaptureExists(const DirectoryKey& key) const; - // Creates or gets a subdirectory under |root_directory|/ for |url| and - // assigns it to |directory|. Returns true on success. If the directory was - // compressed then it is uncompressed automatically. - bool CreateOrGetDirectoryFor(const GURL& url, base::FilePath* directory); + // Creates or gets a subdirectory under |root_directory| for |key| and + // assigns it to |directory|. The directory will be wiped if |clear| is true. + // Returns a path on success or nullopt on failure. If the directory was + // compressed then it will be uncompressed automatically. + base::Optional<base::FilePath> CreateOrGetDirectory(const DirectoryKey& key, + bool clear) const; - // Compresses the directory associated with |url|. Returns true on success or - // if the directory was already compressed. - // NOTE: an empty directory or a directory containing only empty - // files/directories will not compress. - bool CompressDirectoryFor(const GURL& url); + // Compresses the directory associated with |key|. Returns true on success or + // if the directory was already compressed. NOTE: an empty directory or a + // directory containing only empty files/directories will not be compressed. + bool CompressDirectory(const DirectoryKey& key) const; - // Deletes artifacts associated with |urls|. - void DeleteArtifactsFor(const std::vector<GURL>& urls); + // Deletes artifacts associated with |key| or |keys|. + void DeleteArtifactSet(const DirectoryKey& key) const; + void DeleteArtifactSets(const std::vector<DirectoryKey>& keys) const; - // Deletes all stored paint previews stored in the |profile_directory_|. - void DeleteAll(); + // Deletes all stored paint previews stored in the |root_directory_|. + void DeleteAll() const; + + // Serializes |proto| to the directory for |key| also compresses is |compress| + // is true. Returns true on success. + bool SerializePaintPreviewProto(const DirectoryKey& key, + const PaintPreviewProto& proto, + bool compress) const; + + // Deserializes PaintPreviewProto stored in |key|. Returns nullptr if not + // found or the proto cannot be parsed. + std::unique_ptr<PaintPreviewProto> DeserializePaintPreviewProto( + const DirectoryKey& key) const; + + // Lists the current set of in-use DirectoryKeys. + base::flat_set<DirectoryKey> ListUsedKeys() const; private: + friend class base::RefCountedThreadSafe<FileManager>; + ~FileManager(); + enum StorageType { kNone = 0, kDirectory = 1, kZip = 2, }; - StorageType GetPathForUrl(const GURL& url, base::FilePath* path); + StorageType GetPathForKey(const DirectoryKey& key, + base::FilePath* path) const; base::FilePath root_directory_; - - FileManager(const FileManager&) = delete; - FileManager& operator=(const FileManager&) = delete; + scoped_refptr<base::SequencedTaskRunner> io_task_runner_; }; } // namespace paint_preview diff --git a/chromium/components/paint_preview/browser/file_manager_unittest.cc b/chromium/components/paint_preview/browser/file_manager_unittest.cc index d90b9e28896..cacb0ce55e8 100644 --- a/chromium/components/paint_preview/browser/file_manager_unittest.cc +++ b/chromium/components/paint_preview/browser/file_manager_unittest.cc @@ -4,184 +4,281 @@ #include "components/paint_preview/browser/file_manager.h" +#include <memory> + #include "base/files/file.h" #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/files/scoped_temp_dir.h" +#include "base/memory/ref_counted.h" +#include "base/run_loop.h" +#include "base/test/task_environment.h" +#include "base/threading/sequenced_task_runner_handle.h" +#include "base/updateable_sequenced_task_runner.h" +#include "components/paint_preview/common/proto/paint_preview.pb.h" +#include "components/paint_preview/common/test_utils.h" #include "testing/gtest/include/gtest/gtest.h" +#include "url/gurl.h" namespace paint_preview { -TEST(FileManagerTest, TestStats) { - base::Time now = base::Time::Now(); +class FileManagerTest : public ::testing::Test { + public: + FileManagerTest() = default; + ~FileManagerTest() override = default; + + void SetUp() override { + EXPECT_TRUE(temp_dir.CreateUniqueTempDir()); + secondary_runner_ = base::ThreadPool::CreateUpdateableSequencedTaskRunner( + {base::MayBlock(), base::TaskPriority::BEST_EFFORT, + base::TaskShutdownBehavior::BLOCK_SHUTDOWN, + base::ThreadPolicy::MUST_USE_FOREGROUND}); + } + + const base::FilePath& Dir() const { return temp_dir.GetPath(); } + + scoped_refptr<base::SequencedTaskRunner> MainTaskRunner() { + return base::SequencedTaskRunnerHandle::Get(); + } + + scoped_refptr<base::UpdateableSequencedTaskRunner> SecondaryTaskRunner() { + return secondary_runner_; + } + + void RunUntilIdle() { task_environment_.RunUntilIdle(); } + + private: + scoped_refptr<base::UpdateableSequencedTaskRunner> secondary_runner_; base::ScopedTempDir temp_dir; - ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); - FileManager manager(temp_dir.GetPath()); - GURL url("https://www.chromium.org"); - GURL missing_url("https://www.muimorhc.org"); - base::FilePath directory; - EXPECT_TRUE(manager.CreateOrGetDirectoryFor(url, &directory)); - EXPECT_FALSE(directory.empty()); + base::test::TaskEnvironment task_environment_; +}; + +TEST_F(FileManagerTest, TestStats) { + auto manager = base::MakeRefCounted<FileManager>(Dir(), MainTaskRunner()); + auto valid_key = manager->CreateKey(GURL("https://www.chromium.org")); + auto missing_key = manager->CreateKey(GURL("https://www.muimorhc.org")); + base::FilePath out = manager->CreateOrGetDirectory(valid_key, false) + .value_or(base::FilePath()); + EXPECT_FALSE(out.empty()); + EXPECT_TRUE(manager->DirectoryExists(valid_key)); + EXPECT_FALSE(manager->DirectoryExists(missing_key)); - base::Time created_time; - EXPECT_FALSE(manager.GetCreatedTime(missing_url, &created_time)); - EXPECT_TRUE(manager.GetCreatedTime(url, &created_time)); - - base::TouchFile(directory, now - base::TimeDelta::FromSeconds(1), - now - base::TimeDelta::FromSeconds(1)); - base::Time accessed_time; - EXPECT_TRUE(manager.GetLastModifiedTime(url, &accessed_time)); - base::FilePath proto_path = directory.AppendASCII("paint_preview.pb"); - base::File file(proto_path, - base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE); - const size_t kSize = 50; - std::string zeros(kSize, '0'); - file.Write(0, zeros.data(), zeros.size()); - file.Close(); - base::TouchFile(directory, now + base::TimeDelta::FromSeconds(1), - now + base::TimeDelta::FromSeconds(1)); - base::Time later_accessed_time; - EXPECT_FALSE(manager.GetLastModifiedTime(missing_url, &created_time)); - EXPECT_TRUE(manager.GetLastModifiedTime(url, &later_accessed_time)); - EXPECT_GT(later_accessed_time, accessed_time); - - EXPECT_GE(manager.GetSizeOfArtifactsFor(url), kSize); + EXPECT_FALSE(manager->GetInfo(missing_key).has_value()); + EXPECT_TRUE(manager->GetInfo(valid_key).has_value()); + + base::FilePath file_path = out.AppendASCII("test"); + std::string test_str = "Hello World!"; + EXPECT_EQ(static_cast<int>(test_str.length()), + base::WriteFile(file_path, test_str.data(), test_str.length())); + + EXPECT_EQ(manager->GetSizeOfArtifacts(valid_key), test_str.length()); } -TEST(FileManagerTest, TestCreateOrGetDirectory) { - base::ScopedTempDir temp_dir; - ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); - FileManager manager(temp_dir.GetPath()); - GURL url("https://www.chromium.org"); +TEST_F(FileManagerTest, TestCreateOrGetDirectory) { + auto manager = + base::MakeRefCounted<FileManager>(Dir(), SecondaryTaskRunner()); - // Create a new directory. - base::FilePath new_directory; - EXPECT_TRUE(manager.CreateOrGetDirectoryFor(url, &new_directory)); - EXPECT_FALSE(new_directory.empty()); - EXPECT_TRUE(base::PathExists(new_directory)); + auto key = manager->CreateKey(1U); + base::RunLoop loop; + manager->GetTaskRunner()->PostTask( + FROM_HERE, + base::BindOnce( + [](base::OnceClosure quit, scoped_refptr<FileManager> manager, + const DirectoryKey& key) { + // Create a new directory. + base::FilePath directory = manager->CreateOrGetDirectory(key, false) + .value_or(base::FilePath()); + EXPECT_FALSE(directory.empty()); + base::FilePath test_file = directory.AppendASCII("test"); + std::string test_str = "Hello World!"; + EXPECT_EQ( + static_cast<int>(test_str.length()), + base::WriteFile(test_file, test_str.data(), test_str.length())); - // Open an existing directory. - base::FilePath existing_directory; - EXPECT_TRUE(manager.CreateOrGetDirectoryFor(url, &existing_directory)); - EXPECT_FALSE(existing_directory.empty()); - EXPECT_EQ(existing_directory, new_directory); - EXPECT_TRUE(base::PathExists(existing_directory)); + // Open an existing directory and don't clear. + base::FilePath existing_directory = + manager->CreateOrGetDirectory(key, false) + .value_or(base::FilePath()); + EXPECT_FALSE(directory.empty()); + EXPECT_EQ(existing_directory, directory); + EXPECT_TRUE(base::PathExists(test_file)); + // Open an existing directory and clear. + base::FilePath cleared_existing_directory = + manager->CreateOrGetDirectory(key, true).value_or( + base::FilePath()); + EXPECT_FALSE(directory.empty()); + EXPECT_EQ(cleared_existing_directory, directory); + EXPECT_FALSE(base::PathExists(test_file)); + std::move(quit).Run(); + }, + loop.QuitClosure(), manager, key)); + loop.Run(); +} + +TEST_F(FileManagerTest, TestCompression) { + auto manager = base::MakeRefCounted<FileManager>(Dir(), MainTaskRunner()); + auto key = manager->CreateKey(1U); + base::FilePath directory = + manager->CreateOrGetDirectory(key, false).value_or(base::FilePath()); + EXPECT_FALSE(directory.empty()); // A file needs to exist for compression to work. - base::FilePath test_file_path = existing_directory.AppendASCII("foo.txt"); - { - base::File file(test_file_path, - base::File::FLAG_CREATE | base::File::FLAG_WRITE); - const std::string data = "foobarbaz"; - file.WriteAtCurrentPos(data.data(), data.size()); - } - EXPECT_TRUE(base::PathExists(test_file_path)); - base::FilePath test_file_path_empty = - existing_directory.AppendASCII("bar.txt"); + base::FilePath test_file = directory.AppendASCII("test"); + std::string test_str = "Hello World!"; + EXPECT_EQ(static_cast<int>(test_str.length()), + base::WriteFile(test_file, test_str.data(), test_str.length())); + EXPECT_TRUE(base::PathExists(test_file)); + base::FilePath test_file_empty = directory.AppendASCII("foo.txt"); { - base::File file(test_file_path_empty, + base::File file(test_file_empty, base::File::FLAG_CREATE | base::File::FLAG_WRITE); } - EXPECT_TRUE(base::PathExists(test_file_path_empty)); + EXPECT_TRUE(base::PathExists(test_file_empty)); // Compress. - base::FilePath zip_path = existing_directory.AddExtensionASCII(".zip"); - EXPECT_TRUE(manager.CompressDirectoryFor(url)); - EXPECT_FALSE(base::PathExists(existing_directory)); - EXPECT_FALSE(base::PathExists(test_file_path)); - EXPECT_FALSE(base::PathExists(test_file_path_empty)); + base::FilePath zip_path = directory.AddExtensionASCII(".zip"); + EXPECT_TRUE(manager->CompressDirectory(key)); + EXPECT_GT(manager->GetSizeOfArtifacts(key), 0U); + EXPECT_FALSE(base::PathExists(directory)); + EXPECT_FALSE(base::PathExists(test_file)); + EXPECT_FALSE(base::PathExists(test_file_empty)); EXPECT_TRUE(base::PathExists(zip_path)); - EXPECT_GT(manager.GetSizeOfArtifactsFor(url), 0U); - existing_directory.clear(); // Open a compressed file. - EXPECT_TRUE(manager.CreateOrGetDirectoryFor(url, &existing_directory)); - EXPECT_EQ(existing_directory, new_directory); - EXPECT_TRUE(base::PathExists(existing_directory)); - EXPECT_TRUE(base::PathExists(test_file_path)); - EXPECT_TRUE(base::PathExists(test_file_path_empty)); + base::FilePath existing_directory = + manager->CreateOrGetDirectory(key, false).value_or(base::FilePath()); + EXPECT_FALSE(existing_directory.empty()); + EXPECT_EQ(existing_directory, directory); + EXPECT_TRUE(base::PathExists(directory)); + EXPECT_TRUE(base::PathExists(test_file)); + EXPECT_TRUE(base::PathExists(test_file_empty)); EXPECT_FALSE(base::PathExists(zip_path)); } -TEST(FileManagerTest, TestCompressDirectory) { - base::ScopedTempDir temp_dir; - ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); - FileManager manager(temp_dir.GetPath()); - GURL url("https://www.chromium.org"); +TEST_F(FileManagerTest, TestCompressDirectoryFail) { + auto manager = base::MakeRefCounted<FileManager>(Dir(), MainTaskRunner()); + auto key = manager->CreateKey(GURL("https://www.chromium.org")); - base::FilePath new_directory; - EXPECT_TRUE(manager.CreateOrGetDirectoryFor(url, &new_directory)); + base::FilePath new_directory = + manager->CreateOrGetDirectory(key, true).value_or(base::FilePath()); EXPECT_FALSE(new_directory.empty()); - EXPECT_TRUE(base::PathExists(new_directory)); // Compression fails without valid contents. base::FilePath zip_path = new_directory.AddExtensionASCII(".zip"); - EXPECT_FALSE(manager.CompressDirectoryFor(url)); + EXPECT_FALSE(manager->CompressDirectory(key)); EXPECT_TRUE(base::PathExists(new_directory)); EXPECT_FALSE(base::PathExists(zip_path)); +} - // Create a file with contents for compression to work. - { - base::File file(new_directory.AppendASCII("foo.txt"), - base::File::FLAG_CREATE | base::File::FLAG_WRITE); - const std::string data = "foobarbaz"; - file.WriteAtCurrentPos(data.data(), data.size()); - } +TEST_F(FileManagerTest, TestDeleteArtifacts) { + auto manager = + base::MakeRefCounted<FileManager>(Dir(), SecondaryTaskRunner()); - EXPECT_TRUE(manager.CompressDirectoryFor(url)); - EXPECT_FALSE(base::PathExists(new_directory)); - EXPECT_TRUE(base::PathExists(zip_path)); + manager->GetTaskRunner()->PostTask( + FROM_HERE, + base::BindOnce( + [](scoped_refptr<FileManager> manager) { + auto cr_key = manager->CreateKey(GURL("https://www.chromium.org")); + base::FilePath cr_directory = + manager->CreateOrGetDirectory(cr_key, true) + .value_or(base::FilePath()); + EXPECT_FALSE(cr_directory.empty()); + + auto w3_key = manager->CreateKey(GURL("https://www.w3.org")); + base::FilePath w3_directory = + manager->CreateOrGetDirectory(w3_key, true) + .value_or(base::FilePath()); + EXPECT_FALSE(w3_directory.empty()); + + manager->DeleteArtifactSet(cr_key); + EXPECT_FALSE(base::PathExists(cr_directory)); + EXPECT_TRUE(base::PathExists(w3_directory)); + + base::FilePath new_cr_directory = + manager->CreateOrGetDirectory(cr_key, true) + .value_or(base::FilePath()); + EXPECT_EQ(cr_directory, new_cr_directory); + + manager->DeleteArtifactSets( + std::vector<DirectoryKey>({cr_key, w3_key})); + EXPECT_FALSE(base::PathExists(new_cr_directory)); + EXPECT_FALSE(base::PathExists(w3_directory)); + }, + manager)); + RunUntilIdle(); } -TEST(FileManagerTest, TestDeleteArtifactsFor) { +TEST_F(FileManagerTest, TestDeleteAll) { base::ScopedTempDir temp_dir; ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); - FileManager manager(temp_dir.GetPath()); + auto manager = + base::MakeRefCounted<FileManager>(temp_dir.GetPath(), MainTaskRunner()); - GURL cr_url("https://www.chromium.org"); - base::FilePath cr_directory; - EXPECT_TRUE(manager.CreateOrGetDirectoryFor(cr_url, &cr_directory)); + auto cr_key = manager->CreateKey(GURL("https://www.chromium.org")); + base::FilePath cr_directory = + manager->CreateOrGetDirectory(cr_key, true).value_or(base::FilePath()); EXPECT_FALSE(cr_directory.empty()); - EXPECT_TRUE(base::PathExists(cr_directory)); - GURL w3_url("https://www.w3.org"); - base::FilePath w3_directory; - EXPECT_TRUE(manager.CreateOrGetDirectoryFor(w3_url, &w3_directory)); + auto w3_key = manager->CreateKey(GURL("https://www.w3.org")); + base::FilePath w3_directory = + manager->CreateOrGetDirectory(w3_key, true).value_or(base::FilePath()); EXPECT_FALSE(w3_directory.empty()); - EXPECT_TRUE(base::PathExists(w3_directory)); - manager.DeleteArtifactsFor(std::vector<GURL>({cr_url})); + manager->DeleteAll(); EXPECT_FALSE(base::PathExists(cr_directory)); - EXPECT_TRUE(base::PathExists(w3_directory)); + EXPECT_FALSE(base::PathExists(w3_directory)); +} + +TEST_F(FileManagerTest, HandleProto) { + base::ScopedTempDir temp_dir; + ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); + auto manager = + base::MakeRefCounted<FileManager>(temp_dir.GetPath(), MainTaskRunner()); + auto key = manager->CreateKey(1U); + base::FilePath path = + manager->CreateOrGetDirectory(key, true).value_or(base::FilePath()); + EXPECT_FALSE(path.empty()); - base::FilePath new_cr_directory; - EXPECT_TRUE(manager.CreateOrGetDirectoryFor(cr_url, &new_cr_directory)); - EXPECT_FALSE(new_cr_directory.empty()); - EXPECT_TRUE(base::PathExists(new_cr_directory)); - EXPECT_EQ(cr_directory, new_cr_directory); + PaintPreviewProto original_proto; + auto* root_frame = original_proto.mutable_root_frame(); + root_frame->set_embedding_token_low(0); + root_frame->set_embedding_token_high(0); + root_frame->set_is_main_frame(true); + root_frame->set_file_path("0.skp"); + auto* metadata = original_proto.mutable_metadata(); + metadata->set_url(GURL("www.chromium.org").spec()); - manager.DeleteArtifactsFor(std::vector<GURL>({cr_url, w3_url})); - EXPECT_FALSE(base::PathExists(new_cr_directory)); - EXPECT_FALSE(base::PathExists(w3_directory)); + EXPECT_TRUE(manager->SerializePaintPreviewProto(key, original_proto, false)); + EXPECT_TRUE(base::PathExists(path.AppendASCII("proto.pb"))); + auto out_proto = manager->DeserializePaintPreviewProto(key); + EXPECT_NE(out_proto, nullptr); + EXPECT_THAT(*out_proto, EqualsProto(original_proto)); } -TEST(FileManagerTest, TestDeleteAll) { +TEST_F(FileManagerTest, HandleProtoCompressed) { base::ScopedTempDir temp_dir; ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); - FileManager manager(temp_dir.GetPath()); - GURL cr_url("https://www.chromium.org"); - base::FilePath cr_directory; - EXPECT_TRUE(manager.CreateOrGetDirectoryFor(cr_url, &cr_directory)); - EXPECT_FALSE(cr_directory.empty()); - EXPECT_TRUE(base::PathExists(cr_directory)); - GURL w3_url("https://www.w3.org"); - base::FilePath w3_directory; - EXPECT_TRUE(manager.CreateOrGetDirectoryFor(w3_url, &w3_directory)); - EXPECT_FALSE(w3_directory.empty()); - EXPECT_TRUE(base::PathExists(w3_directory)); - manager.DeleteAll(); - EXPECT_FALSE(base::PathExists(cr_directory)); - EXPECT_FALSE(base::PathExists(w3_directory)); + auto manager = + base::MakeRefCounted<FileManager>(temp_dir.GetPath(), MainTaskRunner()); + auto key = manager->CreateKey(1U); + base::FilePath path = + manager->CreateOrGetDirectory(key, true).value_or(base::FilePath()); + EXPECT_FALSE(path.empty()); + + PaintPreviewProto original_proto; + auto* root_frame = original_proto.mutable_root_frame(); + root_frame->set_embedding_token_low(0); + root_frame->set_embedding_token_high(0); + root_frame->set_is_main_frame(true); + root_frame->set_file_path("0.skp"); + auto* metadata = original_proto.mutable_metadata(); + metadata->set_url(GURL("www.chromium.org").spec()); + + EXPECT_TRUE(manager->SerializePaintPreviewProto(key, original_proto, true)); + EXPECT_TRUE(base::PathExists(path.AddExtensionASCII(".zip"))); + auto out_proto = manager->DeserializePaintPreviewProto(key); + EXPECT_NE(out_proto, nullptr); + EXPECT_THAT(*out_proto, EqualsProto(original_proto)); } } // namespace paint_preview diff --git a/chromium/components/paint_preview/browser/hit_tester.h b/chromium/components/paint_preview/browser/hit_tester.h index 0fdc0b0c7d8..1e1f29343cf 100644 --- a/chromium/components/paint_preview/browser/hit_tester.h +++ b/chromium/components/paint_preview/browser/hit_tester.h @@ -26,6 +26,9 @@ class HitTester { HitTester(); ~HitTester(); + HitTester(const HitTester&) = delete; + HitTester& operator=(const HitTester&) = delete; + // Builds a R-Tree from the underlying data. void Build(const PaintPreviewFrameProto& proto); void Build(const std::vector<LinkData>& links); @@ -43,9 +46,6 @@ class HitTester { private: cc::RTree<GURL> rtree_; - - HitTester(const HitTester&) = delete; - HitTester& operator=(const HitTester&) = delete; }; } // namespace paint_preview 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 be35e126d1c..a426d3cea12 100644 --- a/chromium/components/paint_preview/browser/paint_preview_base_service.cc +++ b/chromium/components/paint_preview/browser/paint_preview_base_service.cc @@ -7,15 +7,22 @@ #include <memory> #include <utility> +#include "base/bind.h" #include "base/callback.h" #include "base/files/file_path.h" #include "base/logging.h" -#include "base/task/post_task.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/file_manager.h" +#include "components/paint_preview/browser/compositor_utils.h" #include "components/paint_preview/browser/paint_preview_client.h" +#include "components/paint_preview/browser/paint_preview_compositor_service_impl.h" #include "components/paint_preview/common/mojom/paint_preview_recorder.mojom.h" +#include "content/public/browser/browser_thread.h" #include "content/public/browser/web_contents.h" +#include "ui/gfx/geometry/rect.h" namespace paint_preview { @@ -27,13 +34,31 @@ const char kPaintPreviewDir[] = "paint_preview"; PaintPreviewBaseService::PaintPreviewBaseService( const base::FilePath& path, - const std::string& ascii_feature_name, + base::StringPiece ascii_feature_name, + std::unique_ptr<PaintPreviewPolicy> policy, bool is_off_the_record) - : file_manager_( - path.AppendASCII(kPaintPreviewDir).AppendASCII(ascii_feature_name)), + : 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_)), is_off_the_record_(is_off_the_record) {} + PaintPreviewBaseService::~PaintPreviewBaseService() = default; +void PaintPreviewBaseService::GetCapturedPaintPreviewProto( + const DirectoryKey& key, + OnReadProtoCallback on_read_proto_callback) { + task_runner_->PostTaskAndReplyWithResult( + FROM_HERE, + base::BindOnce(&FileManager::DeserializePaintPreviewProto, file_manager_, + key), + std::move(on_read_proto_callback)); +} + void PaintPreviewBaseService::CapturePaintPreview( content::WebContents* web_contents, const base::FilePath& root_dir, @@ -49,6 +74,12 @@ void PaintPreviewBaseService::CapturePaintPreview( const base::FilePath& root_dir, gfx::Rect clip_rect, OnCapturedCallback callback) { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + if (policy_ && !policy_->SupportedForContents(web_contents)) { + std::move(callback).Run(kContentUnsupported, nullptr); + return; + } + PaintPreviewClient::CreateForWebContents(web_contents); // Is a singleton. auto* client = PaintPreviewClient::FromWebContents(web_contents); if (!client) { @@ -62,23 +93,47 @@ void PaintPreviewBaseService::CapturePaintPreview( params.is_main_frame = (render_frame_host == web_contents->GetMainFrame()); params.root_dir = root_dir; + // 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 + // lifecycle. + web_contents->IncrementCapturerCount(gfx::Size(), true); + + auto start_time = base::TimeTicks::Now(); client->CapturePaintPreview( params, render_frame_host, base::BindOnce(&PaintPreviewBaseService::OnCaptured, - weak_ptr_factory_.GetWeakPtr(), std::move(callback))); + weak_ptr_factory_.GetWeakPtr(), + web_contents->GetMainFrame()->GetFrameTreeNodeId(), + start_time, std::move(callback))); +} + +std::unique_ptr<PaintPreviewCompositorService> +PaintPreviewBaseService::StartCompositorService( + base::OnceClosure disconnect_handler) { + return std::make_unique<PaintPreviewCompositorServiceImpl>( + CreateCompositorCollection(), std::move(disconnect_handler)); } void PaintPreviewBaseService::OnCaptured( + int frame_tree_node_id, + base::TimeTicks start_time, OnCapturedCallback callback, base::UnguessableToken guid, mojom::PaintPreviewStatus status, std::unique_ptr<PaintPreviewProto> proto) { - if (status != mojom::PaintPreviewStatus::kOk) { + auto* web_contents = + content::WebContents::FromFrameTreeNodeId(frame_tree_node_id); + if (web_contents) + web_contents->DecrementCapturerCount(true); + + if (status != mojom::PaintPreviewStatus::kOk || !proto) { DVLOG(1) << "ERROR: Paint Preview failed to capture for document " << guid.ToString() << " with error " << status; std::move(callback).Run(kCaptureFailed, nullptr); return; } + base::UmaHistogramTimes("Browser.PaintPreview.Capture.TotalCaptureDuration", + base::TimeTicks::Now() - start_time); std::move(callback).Run(kOk, std::move(proto)); } 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 390fba111c9..2cc2d1e8fc7 100644 --- a/chromium/components/paint_preview/browser/paint_preview_base_service.h +++ b/chromium/components/paint_preview/browser/paint_preview_base_service.h @@ -7,13 +7,23 @@ #include <memory> +#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" +#include "base/sequenced_task_runner.h" +#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_policy.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/public/paint_preview_compositor_service.h" #include "content/public/browser/web_contents.h" namespace paint_preview { @@ -33,6 +43,7 @@ class PaintPreviewBaseService : public KeyedService { public: enum CaptureStatus { kOk = 0, + kContentUnsupported, kClientCreationFailed, kCaptureFailed, }; @@ -41,21 +52,45 @@ class PaintPreviewBaseService : public KeyedService { base::OnceCallback<void(CaptureStatus, std::unique_ptr<PaintPreviewProto>)>; + using OnReadProtoCallback = + base::OnceCallback<void(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|. + // 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, - const std::string& ascii_feature_name, + base::StringPiece ascii_feature_name, + std::unique_ptr<PaintPreviewPolicy> policy, bool is_off_the_record); ~PaintPreviewBaseService() override; // Returns the file manager for the directory associated with the service. - FileManager* GetFileManager() { return &file_manager_; } + 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_; + } // 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(). Derived classes may + // override this function; for example, the proto is cached in memory. + virtual void GetCapturedPaintPreviewProto( + const DirectoryKey& key, + 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 @@ -64,12 +99,11 @@ class PaintPreviewBaseService : public KeyedService { // individual subframes and should be used for only a few use cases. // // NOTE: |root_dir| in the following methods should be created using - // GetFileManager()->CreateOrGetDirectoryFor(<GURL>). However, to provide + // 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: - // - paint_preview.pb (the metadata proto) // - a number of SKPs listed as <guid>.skp (one per frame) // // Captures the main frame of |web_contents| (an observer for capturing Paint @@ -89,18 +123,27 @@ class PaintPreviewBaseService : public KeyedService { gfx::Rect clip_rect, OnCapturedCallback callback); + // Starts the compositor service in a utility process. + std::unique_ptr<PaintPreviewCompositorService> StartCompositorService( + base::OnceClosure disconnect_handler); + private: - void OnCaptured(OnCapturedCallback callback, + void OnCaptured(int frame_tree_node_id, + base::TimeTicks start_time, + OnCapturedCallback callback, base::UnguessableToken guid, mojom::PaintPreviewStatus status, std::unique_ptr<PaintPreviewProto> proto); - FileManager file_manager_; + 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}; - DISALLOW_COPY_AND_ASSIGN(PaintPreviewBaseService); + PaintPreviewBaseService(const PaintPreviewBaseService&) = delete; + PaintPreviewBaseService& operator=(const PaintPreviewBaseService&) = delete; }; } // namespace paint_preview 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 new file mode 100644 index 00000000000..8ae8532e3b1 --- /dev/null +++ b/chromium/components/paint_preview/browser/paint_preview_base_service_test_factory.cc @@ -0,0 +1,64 @@ +// Copyright 2019 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_base_service_test_factory.h" + +#include <memory> +#include <utility> + +#include "base/no_destructor.h" +#include "components/keyed_service/core/keyed_service.h" +#include "components/keyed_service/core/simple_dependency_manager.h" +#include "components/keyed_service/core/simple_factory_key.h" +#include "components/keyed_service/core/simple_keyed_service_factory.h" +#include "components/paint_preview/browser/paint_preview_base_service.h" + +namespace paint_preview { + +const char kTestFeatureDir[] = "test_feature"; + +PaintPreviewBaseServiceTestFactory* +PaintPreviewBaseServiceTestFactory::GetInstance() { + // Use NoDestructor rather than a singleton due to lifetime behavior in + // tests. + static base::NoDestructor<PaintPreviewBaseServiceTestFactory> factory; + return factory.get(); +} + +PaintPreviewBaseService* PaintPreviewBaseServiceTestFactory::GetForKey( + SimpleFactoryKey* key) { + return static_cast<PaintPreviewBaseService*>( + GetInstance()->GetServiceForKey(key, true)); +} + +void PaintPreviewBaseServiceTestFactory::Destroy(SimpleFactoryKey* key) { + SimpleContextShutdown(key); + SimpleContextDestroyed(key); +} + +std::unique_ptr<KeyedService> PaintPreviewBaseServiceTestFactory::Build( + SimpleFactoryKey* key) { + return std::make_unique<PaintPreviewBaseService>( + key->GetPath(), kTestFeatureDir, nullptr, key->IsOffTheRecord()); +} + +PaintPreviewBaseServiceTestFactory::~PaintPreviewBaseServiceTestFactory() = + default; + +PaintPreviewBaseServiceTestFactory::PaintPreviewBaseServiceTestFactory() + : SimpleKeyedServiceFactory("PaintPreviewBaseService", + SimpleDependencyManager::GetInstance()) {} + +std::unique_ptr<KeyedService> +PaintPreviewBaseServiceTestFactory::BuildServiceInstanceFor( + SimpleFactoryKey* key) const { + return Build(key); +} + +SimpleFactoryKey* PaintPreviewBaseServiceTestFactory::GetKeyToUse( + SimpleFactoryKey* key) const { + return key; +} + +} // namespace paint_preview diff --git a/chromium/components/paint_preview/browser/paint_preview_base_service_test_factory.h b/chromium/components/paint_preview/browser/paint_preview_base_service_test_factory.h new file mode 100644 index 00000000000..42c9629e3b4 --- /dev/null +++ b/chromium/components/paint_preview/browser/paint_preview_base_service_test_factory.h @@ -0,0 +1,52 @@ +// 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_BASE_SERVICE_TEST_FACTORY_H_ +#define COMPONENTS_PAINT_PREVIEW_BROWSER_PAINT_PREVIEW_BASE_SERVICE_TEST_FACTORY_H_ + +#include <memory> + +#include "components/keyed_service/core/keyed_service.h" +#include "components/keyed_service/core/simple_factory_key.h" +#include "components/keyed_service/core/simple_keyed_service_factory.h" +#include "components/paint_preview/browser/paint_preview_base_service.h" + +namespace paint_preview { + +// An approximation of a SimpleKeyedServiceFactory for a keyed +// PaintPerviewBaseService. This is different than a "real" version as it +// uses base::NoDestructor rather than base::Singleton due to testing object +// lifecycle. +class PaintPreviewBaseServiceTestFactory : public SimpleKeyedServiceFactory { + public: + static PaintPreviewBaseServiceTestFactory* GetInstance(); + + static PaintPreviewBaseService* GetForKey(SimpleFactoryKey* key); + + // The following are unusual methods for an implementer of a + // KeyedServiceFactory. They are intended to help with testing. + + static std::unique_ptr<KeyedService> Build(SimpleFactoryKey* key); + + void Destroy(SimpleFactoryKey* key); + + // These are public due to using base::NoDestructor. Don't call directly. + PaintPreviewBaseServiceTestFactory(); + ~PaintPreviewBaseServiceTestFactory() override; + + PaintPreviewBaseServiceTestFactory( + const PaintPreviewBaseServiceTestFactory&) = delete; + PaintPreviewBaseServiceTestFactory& operator=( + const PaintPreviewBaseServiceTestFactory&) = delete; + + private: + std::unique_ptr<KeyedService> BuildServiceInstanceFor( + SimpleFactoryKey* key) const override; + + SimpleFactoryKey* GetKeyToUse(SimpleFactoryKey* key) const override; +}; + +} // namespace paint_preview + +#endif // COMPONENTS_PAINT_PREVIEW_BROWSER_PAINT_PREVIEW_BASE_SERVICE_TEST_FACTORY_H_ 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 cd1084c3195..255b090ee8c 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 @@ -7,11 +7,9 @@ #include "base/files/scoped_temp_dir.h" #include "base/macros.h" #include "base/no_destructor.h" +#include "base/strings/strcat.h" #include "build/build_config.h" -#include "components/keyed_service/core/simple_dependency_manager.h" -#include "components/keyed_service/core/simple_factory_key.h" -#include "components/keyed_service/core/simple_key_map.h" -#include "components/keyed_service/core/simple_keyed_service_factory.h" +#include "components/paint_preview/browser/paint_preview_base_service_test_factory.h" #include "components/paint_preview/common/mojom/paint_preview_recorder.mojom.h" #include "components/paint_preview/common/test_utils.h" #include "content/public/browser/render_process_host.h" @@ -27,16 +25,47 @@ namespace { const char kTestFeatureDir[] = "test_feature"; -// Builds a PaintPreviewBaseService associated with |key|. -std::unique_ptr<KeyedService> BuildService(SimpleFactoryKey* key) { +class RejectionPaintPreviewPolicy : public PaintPreviewPolicy { + public: + RejectionPaintPreviewPolicy() = default; + ~RejectionPaintPreviewPolicy() override = default; + + RejectionPaintPreviewPolicy(const RejectionPaintPreviewPolicy&) = delete; + RejectionPaintPreviewPolicy& operator=(const RejectionPaintPreviewPolicy&) = + delete; + + bool SupportedForContents(content::WebContents* web_contents) override { + return false; + } +}; + +// Builds a PaintPreviewBaseService associated with |key| which will never +// permit paint previews. +std::unique_ptr<KeyedService> BuildServiceWithRejectionPolicy( + SimpleFactoryKey* key) { return std::make_unique<PaintPreviewBaseService>( - key->GetPath(), kTestFeatureDir, key->IsOffTheRecord()); + key->GetPath(), kTestFeatureDir, + std::make_unique<RejectionPaintPreviewPolicy>(), key->IsOffTheRecord()); } -// Returns the GUID corresponding to |rfh|. -uint64_t FrameGuid(content::RenderFrameHost* rfh) { - return static_cast<uint64_t>(rfh->GetProcess()->GetID()) << 32 | - rfh->GetRoutingID(); +base::FilePath CreateDir(scoped_refptr<FileManager> manager, + const DirectoryKey& key) { + base::FilePath out; + base::RunLoop loop; + manager->GetTaskRunner()->PostTaskAndReplyWithResult( + FROM_HERE, + base::BindOnce(&FileManager::CreateOrGetDirectory, manager, key, false), + base::BindOnce( + [](base::OnceClosure quit, base::FilePath* out, + const base::Optional<base::FilePath>& path) { + EXPECT_TRUE(path.has_value()); + EXPECT_FALSE(path->empty()); + *out = path.value(); + std::move(quit).Run(); + }, + loop.QuitClosure(), &out)); + loop.Run(); + return out; } } // namespace @@ -72,6 +101,9 @@ class MockPaintPreviewRecorder : public mojom::PaintPreviewRecorder { std::move(handle))); } + MockPaintPreviewRecorder(const MockPaintPreviewRecorder&) = delete; + MockPaintPreviewRecorder& operator=(const MockPaintPreviewRecorder&) = delete; + private: void CheckParams(mojom::PaintPreviewCaptureParamsPtr input_params) { // Ignore GUID and File as this is internal information not known by the @@ -84,41 +116,6 @@ class MockPaintPreviewRecorder : public mojom::PaintPreviewRecorder { mojom::PaintPreviewStatus status_; mojom::PaintPreviewCaptureResponsePtr response_; mojo::AssociatedReceiver<mojom::PaintPreviewRecorder> binding_{this}; - - DISALLOW_COPY_AND_ASSIGN(MockPaintPreviewRecorder); -}; - -// An approximation of a SimpleKeyedServiceFactory for a keyed -// PaintPerviewBaseService. This is different than a "real" version as it -// uses base::NoDestructor rather than base::Singleton. -class PaintPreviewBaseServiceFactory : public SimpleKeyedServiceFactory { - public: - static PaintPreviewBaseServiceFactory* GetInstance() { - // Use NoDestructor rather than a singleton due to lifetime behavior in - // tests. - static base::NoDestructor<PaintPreviewBaseServiceFactory> factory; - return factory.get(); - } - - static PaintPreviewBaseService* GetForKey(SimpleFactoryKey* key) { - return static_cast<PaintPreviewBaseService*>( - GetInstance()->GetServiceForKey(key, true)); - } - - PaintPreviewBaseServiceFactory() - : SimpleKeyedServiceFactory("PaintPreviewBaseService", - SimpleDependencyManager::GetInstance()) {} - ~PaintPreviewBaseServiceFactory() override = default; - - private: - std::unique_ptr<KeyedService> BuildServiceInstanceFor( - SimpleFactoryKey* key) const override { - return BuildService(key); - } - - SimpleFactoryKey* GetKeyToUse(SimpleFactoryKey* key) const override { - return key; - } }; class PaintPreviewBaseServiceTest : public content::RenderViewHostTestHarness { @@ -126,13 +123,24 @@ class PaintPreviewBaseServiceTest : public content::RenderViewHostTestHarness { PaintPreviewBaseServiceTest() = default; ~PaintPreviewBaseServiceTest() override = default; + PaintPreviewBaseServiceTest(const PaintPreviewBaseService&) = delete; + PaintPreviewBaseServiceTest& operator=(const PaintPreviewBaseService&) = + delete; + protected: void SetUp() override { content::RenderViewHostTestHarness::SetUp(); key_ = std::make_unique<SimpleFactoryKey>( browser_context()->GetPath(), browser_context()->IsOffTheRecord()); - PaintPreviewBaseServiceFactory::GetInstance()->SetTestingFactory( - key_.get(), base::BindRepeating(&BuildService)); + PaintPreviewBaseServiceTestFactory::GetInstance()->SetTestingFactory( + key_.get(), + base::BindRepeating(&PaintPreviewBaseServiceTestFactory::Build)); + + rejection_policy_key_ = std::make_unique<SimpleFactoryKey>( + browser_context()->GetPath(), browser_context()->IsOffTheRecord()); + PaintPreviewBaseServiceTestFactory::GetInstance()->SetTestingFactory( + rejection_policy_key_.get(), + base::BindRepeating(&BuildServiceWithRejectionPolicy)); } void OverrideInterface(MockPaintPreviewRecorder* service) { @@ -144,14 +152,18 @@ class PaintPreviewBaseServiceTest : public content::RenderViewHostTestHarness { base::Unretained(service))); } - PaintPreviewBaseService* CreateService() { - return PaintPreviewBaseServiceFactory::GetForKey(key_.get()); + PaintPreviewBaseService* GetService() { + return PaintPreviewBaseServiceTestFactory::GetForKey(key_.get()); + } + + PaintPreviewBaseService* GetServiceWithRejectionPolicy() { + return PaintPreviewBaseServiceTestFactory::GetForKey( + rejection_policy_key_.get()); } private: std::unique_ptr<SimpleFactoryKey> key_ = nullptr; - - DISALLOW_COPY_AND_ASSIGN(PaintPreviewBaseServiceTest); + std::unique_ptr<SimpleFactoryKey> rejection_policy_key_ = nullptr; }; TEST_F(PaintPreviewBaseServiceTest, CaptureMainFrame) { @@ -161,15 +173,15 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureMainFrame) { params->is_main_frame = true; recorder.SetExpectedParams(std::move(params)); auto response = mojom::PaintPreviewCaptureResponse::New(); - response->id = main_rfh()->GetRoutingID(); + response->embedding_token = base::nullopt; recorder.SetResponse(mojom::PaintPreviewStatus::kOk, std::move(response)); OverrideInterface(&recorder); - auto* service = CreateService(); + auto* service = GetService(); EXPECT_FALSE(service->IsOffTheRecord()); - base::FilePath path; - ASSERT_TRUE(service->GetFileManager()->CreateOrGetDirectoryFor( - web_contents()->GetLastCommittedURL(), &path)); + auto manager = service->GetFileManager(); + base::FilePath path = CreateDir( + manager, manager->CreateKey(web_contents()->GetLastCommittedURL())); base::RunLoop loop; service->CapturePaintPreview( @@ -177,26 +189,33 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureMainFrame) { base::BindOnce( [](base::OnceClosure quit_closure, PaintPreviewBaseService::CaptureStatus expected_status, - const base::FilePath& expected_path, uint64_t expected_guid, + const base::FilePath& expected_path, PaintPreviewBaseService::CaptureStatus status, std::unique_ptr<PaintPreviewProto> proto) { EXPECT_EQ(status, expected_status); EXPECT_TRUE(proto->has_root_frame()); EXPECT_EQ(proto->subframes_size(), 0); EXPECT_TRUE(proto->root_frame().is_main_frame()); + auto token = base::UnguessableToken::Deserialize( + proto->root_frame().embedding_token_high(), + proto->root_frame().embedding_token_low()); #if defined(OS_WIN) base::FilePath path = base::FilePath( base::UTF8ToUTF16(proto->root_frame().file_path())); + base::FilePath name( + base::UTF8ToUTF16(base::StrCat({token.ToString(), ".skp"}))); #else base::FilePath path = base::FilePath(proto->root_frame().file_path()); + base::FilePath name(base::StrCat({token.ToString(), ".skp"})); #endif EXPECT_EQ(path.DirName(), expected_path); - EXPECT_EQ(proto->root_frame().id(), expected_guid); + LOG(ERROR) << expected_path; + EXPECT_EQ(path.BaseName(), name); std::move(quit_closure).Run(); }, - loop.QuitClosure(), PaintPreviewBaseService::CaptureStatus::kOk, path, - FrameGuid(main_rfh()))); + loop.QuitClosure(), PaintPreviewBaseService::CaptureStatus::kOk, + path)); loop.Run(); } @@ -207,15 +226,15 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureFailed) { params->is_main_frame = true; recorder.SetExpectedParams(std::move(params)); auto response = mojom::PaintPreviewCaptureResponse::New(); - response->id = main_rfh()->GetRoutingID(); + response->embedding_token = base::nullopt; recorder.SetResponse(mojom::PaintPreviewStatus::kFailed, std::move(response)); OverrideInterface(&recorder); - auto* service = CreateService(); + auto* service = GetService(); EXPECT_FALSE(service->IsOffTheRecord()); - base::FilePath path; - ASSERT_TRUE(service->GetFileManager()->CreateOrGetDirectoryFor( - web_contents()->GetLastCommittedURL(), &path)); + auto manager = service->GetFileManager(); + base::FilePath path = CreateDir( + manager, manager->CreateKey(web_contents()->GetLastCommittedURL())); base::RunLoop loop; service->CapturePaintPreview( @@ -234,4 +253,38 @@ TEST_F(PaintPreviewBaseServiceTest, CaptureFailed) { loop.Run(); } +TEST_F(PaintPreviewBaseServiceTest, CaptureDisallowed) { + MockPaintPreviewRecorder recorder; + auto params = mojom::PaintPreviewCaptureParams::New(); + params->clip_rect = gfx::Rect(0, 0, 0, 0); + params->is_main_frame = true; + recorder.SetExpectedParams(std::move(params)); + auto response = mojom::PaintPreviewCaptureResponse::New(); + response->embedding_token = base::nullopt; + recorder.SetResponse(mojom::PaintPreviewStatus::kFailed, std::move(response)); + OverrideInterface(&recorder); + + auto* service = GetServiceWithRejectionPolicy(); + EXPECT_FALSE(service->IsOffTheRecord()); + auto manager = service->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), + base::BindOnce( + [](base::OnceClosure quit_closure, + PaintPreviewBaseService::CaptureStatus expected_status, + PaintPreviewBaseService::CaptureStatus status, + std::unique_ptr<PaintPreviewProto> proto) { + EXPECT_EQ(status, expected_status); + EXPECT_EQ(proto, nullptr); + std::move(quit_closure).Run(); + }, + loop.QuitClosure(), + PaintPreviewBaseService::CaptureStatus::kContentUnsupported)); + loop.Run(); +} + } // 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 fc3b5a479b8..3728efa1f5f 100644 --- a/chromium/components/paint_preview/browser/paint_preview_client.cc +++ b/chromium/components/paint_preview/browser/paint_preview_client.cc @@ -6,26 +6,38 @@ #include <utility> +#include "base/metrics/histogram_functions.h" #include "base/strings/strcat.h" #include "base/strings/string_number_conversions.h" #include "base/task/post_task.h" +#include "base/task/thread_pool.h" #include "base/threading/scoped_blocking_call.h" +#include "components/ukm/content/source_url_recorder.h" #include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_process_host.h" +#include "content/public/browser/web_contents.h" +#include "services/metrics/public/cpp/metrics_utils.h" +#include "services/metrics/public/cpp/ukm_builders.h" +#include "services/metrics/public/cpp/ukm_recorder.h" #include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h" namespace paint_preview { namespace { -// A frame's GUID is (ProcessID || Routing ID) where || is a bitwise -// concatenation. -uint64_t GenerateFrameGuid(content::RenderFrameHost* render_frame_host) { - int process_id = render_frame_host->GetProcess()->GetID(); - int frame_id = render_frame_host->GetRoutingID(); - return static_cast<uint64_t>(process_id) << 32 | frame_id; +// Creates an old style id of Process ID || Routing ID. This should only be used +// for looking up the main frame's filler GUID in cases where only the +// RenderFrameHost is available (such as in RenderFrameDeleted()). +uint64_t MakeOldStyleId(content::RenderFrameHost* render_frame_host) { + return (static_cast<uint64_t>(render_frame_host->GetProcess()->GetID()) + << 32) | + render_frame_host->GetRoutingID(); +} + +uint64_t MakeOldStyleId(const content::GlobalFrameRoutingId& id) { + return (static_cast<uint64_t>(id.child_id) << 32) | id.frame_routing_id; } // Converts gfx::Rect to its RectProto form. @@ -38,28 +50,24 @@ void RectToRectProto(const gfx::Rect& rect, RectProto* proto) { // Converts |response| into |proto|. Returns a list of the frame GUIDs // referenced by the response. -std::vector<uint64_t> PaintPreviewCaptureResponseToPaintPreviewFrameProto( +std::vector<base::UnguessableToken> +PaintPreviewCaptureResponseToPaintPreviewFrameProto( mojom::PaintPreviewCaptureResponsePtr response, - content::RenderFrameHost* render_frame_host, + base::UnguessableToken frame_guid, PaintPreviewFrameProto* proto) { - int process_id = render_frame_host->GetProcess()->GetID(); - proto->set_id(static_cast<uint64_t>(process_id) << 32 | response->id); - - std::vector<uint64_t> frame_guids; - auto* proto_content_id_proxy_map = proto->mutable_content_id_proxy_id_map(); - for (const auto& id_pair : response->content_id_proxy_id_map) { - content::RenderFrameHost* rfh = - content::RenderFrameHost::FromPlaceholderId(process_id, id_pair.second); - if (!rfh) { - // The render frame host doesn't exist. We won't be able to infill this - // content. 0 is an invalid content_id so an empty picture will be used - // instead. - proto_content_id_proxy_map->insert({id_pair.first, 0}); - continue; - } - auto guid = GenerateFrameGuid(rfh); - frame_guids.push_back(guid); - proto_content_id_proxy_map->insert({id_pair.first, guid}); + proto->set_embedding_token_high(frame_guid.GetHighForSerialization()); + proto->set_embedding_token_low(frame_guid.GetLowForSerialization()); + + std::vector<base::UnguessableToken> frame_guids; + for (const auto& id_pair : response->content_id_to_embedding_token) { + auto* content_id_embedding_token_pair = + proto->add_content_id_to_embedding_tokens(); + content_id_embedding_token_pair->set_content_id(id_pair.first); + content_id_embedding_token_pair->set_embedding_token_low( + id_pair.second.GetLowForSerialization()); + content_id_embedding_token_pair->set_embedding_token_high( + id_pair.second.GetHighForSerialization()); + frame_guids.push_back(id_pair.second); } for (const auto& link : response->links) { @@ -71,28 +79,49 @@ std::vector<uint64_t> PaintPreviewCaptureResponseToPaintPreviewFrameProto( return frame_guids; } +// Records UKM data for the capture. +// TODO(crbug/1038390): Add more metrics; +// - Peak memory during capture (bucketized). +// - Compressed on disk size (bucketized). +void RecordUkmCaptureData(ukm::SourceId source_id, + base::TimeDelta blink_recording_time) { + if (source_id == ukm::kInvalidSourceId) + return; + ukm::builders::PaintPreviewCapture(source_id) + .SetBlinkCaptureTime(blink_recording_time.InMilliseconds()) + .Record(ukm::UkmRecorder::Get()); +} + } // namespace PaintPreviewClient::PaintPreviewParams::PaintPreviewParams() = default; + PaintPreviewClient::PaintPreviewParams::~PaintPreviewParams() = default; PaintPreviewClient::PaintPreviewData::PaintPreviewData() = default; + PaintPreviewClient::PaintPreviewData::~PaintPreviewData() = default; + PaintPreviewClient::PaintPreviewData& PaintPreviewClient::PaintPreviewData:: -operator=(PaintPreviewData&& rhs) noexcept = default; +operator=(PaintPreviewData&& rhs) = default; + PaintPreviewClient::PaintPreviewData::PaintPreviewData( PaintPreviewData&& other) noexcept = default; PaintPreviewClient::CreateResult::CreateResult(base::File file, base::File::Error error) : file(std::move(file)), error(error) {} + PaintPreviewClient::CreateResult::~CreateResult() = default; + PaintPreviewClient::CreateResult::CreateResult(CreateResult&& other) = default; + PaintPreviewClient::CreateResult& PaintPreviewClient::CreateResult::operator=( CreateResult&& other) = default; PaintPreviewClient::PaintPreviewClient(content::WebContents* web_contents) : content::WebContentsObserver(web_contents) {} + PaintPreviewClient::~PaintPreviewClient() = default; void PaintPreviewClient::CapturePaintPreview( @@ -104,11 +133,13 @@ void PaintPreviewClient::CapturePaintPreview( mojom::PaintPreviewStatus::kGuidCollision, nullptr); return; } - all_document_data_.insert({params.document_guid, PaintPreviewData()}); - auto* document_data = &all_document_data_[params.document_guid]; - document_data->root_dir = params.root_dir; - document_data->callback = std::move(callback); - document_data->root_url = render_frame_host->GetLastCommittedURL(); + PaintPreviewData document_data; + document_data.root_dir = params.root_dir; + document_data.callback = std::move(callback); + document_data.root_url = render_frame_host->GetLastCommittedURL(); + document_data.source_id = + ukm::GetSourceIdForWebContentsDocument(web_contents()); + all_document_data_.insert({params.document_guid, std::move(document_data)}); CapturePaintPreviewInternal(params, render_frame_host); } @@ -125,7 +156,19 @@ void PaintPreviewClient::CaptureSubframePaintPreview( void PaintPreviewClient::RenderFrameDeleted( content::RenderFrameHost* render_frame_host) { - uint64_t frame_guid = GenerateFrameGuid(render_frame_host); + // TODO(crbug/1044983): Investigate possible issues with cleanup if just + // a single subframe gets deleted. + auto maybe_token = render_frame_host->GetEmbeddingToken(); + bool is_main_frame = false; + if (!maybe_token.has_value()) { + uint64_t old_style_id = MakeOldStyleId(render_frame_host); + auto it = main_frame_guids_.find(old_style_id); + if (it == main_frame_guids_.end()) + return; + maybe_token = it->second; + is_main_frame = true; + } + base::UnguessableToken frame_guid = maybe_token.value(); auto it = pending_previews_on_subframe_.find(frame_guid); if (it == pending_previews_on_subframe_.end()) return; @@ -133,12 +176,21 @@ void PaintPreviewClient::RenderFrameDeleted( auto data_it = all_document_data_.find(document_guid); if (data_it == all_document_data_.end()) continue; - data_it->second.awaiting_subframes.erase(frame_guid); - data_it->second.finished_subframes.insert(frame_guid); - data_it->second.had_error = true; - if (data_it->second.awaiting_subframes.empty()) { + auto* document_data = &data_it->second; + document_data->awaiting_subframes.erase(frame_guid); + document_data->finished_subframes.insert(frame_guid); + document_data->had_error = true; + if (document_data->awaiting_subframes.empty() || is_main_frame) { + if (is_main_frame) { + for (const auto& subframe_guid : document_data->awaiting_subframes) { + auto subframe_docs = pending_previews_on_subframe_[subframe_guid]; + subframe_docs.erase(document_guid); + if (subframe_docs.empty()) + pending_previews_on_subframe_.erase(subframe_guid); + } + } interface_ptrs_.erase(frame_guid); - OnFinished(document_guid, &data_it->second); + OnFinished(document_guid, document_data); } } pending_previews_on_subframe_.erase(frame_guid); @@ -146,8 +198,6 @@ void PaintPreviewClient::RenderFrameDeleted( PaintPreviewClient::CreateResult PaintPreviewClient::CreateFileHandle( const base::FilePath& path) { - base::ScopedBlockingCall scoped_blocking_call(FROM_HERE, - base::BlockingType::MAY_BLOCK); base::File file(path, base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE); return CreateResult(std::move(file), file.error_details()); @@ -168,45 +218,82 @@ mojom::PaintPreviewCaptureParamsPtr PaintPreviewClient::CreateMojoParams( void PaintPreviewClient::CapturePaintPreviewInternal( const PaintPreviewParams& params, content::RenderFrameHost* render_frame_host) { - uint64_t frame_guid = GenerateFrameGuid(render_frame_host); - auto* document_data = &all_document_data_[params.document_guid]; + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + // Use a frame's embedding token as its GUID. Note that we create a GUID for + // the main frame so that we can treat it the same as other frames. + auto token = render_frame_host->GetEmbeddingToken(); + if (params.is_main_frame && !token.has_value()) { + token = base::UnguessableToken::Create(); + main_frame_guids_.insert( + {MakeOldStyleId(render_frame_host), token.value()}); + } + + // This should be impossible, but if it happens in a release build just abort. + if (!token.has_value()) { + DVLOG(1) << "Error: Attempted to capture a non-main frame without an " + "embedding token."; + NOTREACHED(); + return; + } + + auto it = all_document_data_.find(params.document_guid); + if (it == all_document_data_.end()) + return; + auto* document_data = &it->second; + + base::UnguessableToken frame_guid = token.value(); + if (params.is_main_frame) + document_data->root_frame_token = frame_guid; // Deduplicate data if a subframe is required multiple times. if (base::Contains(document_data->awaiting_subframes, frame_guid) || base::Contains(document_data->finished_subframes, frame_guid)) return; base::FilePath file_path = document_data->root_dir.AppendASCII( - base::StrCat({base::NumberToString(frame_guid), ".skp"})); - base::PostTaskAndReplyWithResult( - FROM_HERE, - {base::ThreadPool(), base::MayBlock(), base::TaskPriority::USER_VISIBLE}, + base::StrCat({frame_guid.ToString(), ".skp"})); + base::ThreadPool::PostTaskAndReplyWithResult( + FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE}, base::BindOnce(&CreateFileHandle, file_path), base::BindOnce(&PaintPreviewClient::RequestCaptureOnUIThread, weak_ptr_factory_.GetWeakPtr(), params, frame_guid, - base::Unretained(render_frame_host), file_path)); + content::GlobalFrameRoutingId( + render_frame_host->GetProcess()->GetID(), + render_frame_host->GetRoutingID()), + file_path)); } void PaintPreviewClient::RequestCaptureOnUIThread( const PaintPreviewParams& params, - uint64_t frame_guid, - content::RenderFrameHost* render_frame_host, + const base::UnguessableToken& frame_guid, + const content::GlobalFrameRoutingId& render_frame_id, const base::FilePath& file_path, CreateResult result) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); - auto* document_data = &all_document_data_[params.document_guid]; + auto it = all_document_data_.find(params.document_guid); + if (it == all_document_data_.end()) + return; + auto* document_data = &it->second; + if (!document_data->callback) + return; + if (result.error != base::File::FILE_OK) { - // Don't block up the UI thread and answer the callback on a different - // thread. - base::PostTask( - FROM_HERE, - base::BindOnce(std::move(document_data->callback), params.document_guid, - mojom::PaintPreviewStatus::kFileCreationError, nullptr)); + std::move(document_data->callback) + .Run(params.document_guid, + mojom::PaintPreviewStatus::kFileCreationError, nullptr); + return; + } + + auto* render_frame_host = content::RenderFrameHost::FromID(render_frame_id); + if (!render_frame_host) { + std::move(document_data->callback) + .Run(params.document_guid, mojom::PaintPreviewStatus::kCaptureFailed, + nullptr); return; } document_data->awaiting_subframes.insert(frame_guid); - auto it = pending_previews_on_subframe_.find(frame_guid); - if (it != pending_previews_on_subframe_.end()) { - it->second.insert(params.document_guid); + auto subframe_it = pending_previews_on_subframe_.find(frame_guid); + if (subframe_it != pending_previews_on_subframe_.end()) { + subframe_it->second.insert(params.document_guid); } else { pending_previews_on_subframe_.insert(std::make_pair( frame_guid, @@ -224,15 +311,15 @@ void PaintPreviewClient::RequestCaptureOnUIThread( base::BindOnce(&PaintPreviewClient::OnPaintPreviewCapturedCallback, weak_ptr_factory_.GetWeakPtr(), params.document_guid, frame_guid, params.is_main_frame, file_path, - base::Unretained(render_frame_host))); + render_frame_id)); } void PaintPreviewClient::OnPaintPreviewCapturedCallback( - base::UnguessableToken guid, - uint64_t frame_guid, + const base::UnguessableToken& guid, + const base::UnguessableToken& frame_guid, bool is_main_frame, const base::FilePath& filename, - content::RenderFrameHost* render_frame_host, + const content::GlobalFrameRoutingId& render_frame_id, mojom::PaintPreviewStatus status, mojom::PaintPreviewCaptureResponsePtr response) { // There is no retry logic so always treat a frame as processed regardless of @@ -241,8 +328,11 @@ void PaintPreviewClient::OnPaintPreviewCapturedCallback( if (status == mojom::PaintPreviewStatus::kOk) status = RecordFrame(guid, frame_guid, is_main_frame, filename, - render_frame_host, std::move(response)); - auto* document_data = &all_document_data_[guid]; + render_frame_id, std::move(response)); + auto it = all_document_data_.find(guid); + if (it == all_document_data_.end()) + return; + auto* document_data = &it->second; if (status != mojom::PaintPreviewStatus::kOk) document_data->had_error = true; @@ -250,73 +340,93 @@ void PaintPreviewClient::OnPaintPreviewCapturedCallback( OnFinished(guid, document_data); } -void PaintPreviewClient::MarkFrameAsProcessed(base::UnguessableToken guid, - uint64_t frame_guid) { +void PaintPreviewClient::MarkFrameAsProcessed( + base::UnguessableToken guid, + const base::UnguessableToken& frame_guid) { pending_previews_on_subframe_[frame_guid].erase(guid); if (pending_previews_on_subframe_[frame_guid].empty()) interface_ptrs_.erase(frame_guid); - all_document_data_[guid].finished_subframes.insert(frame_guid); - all_document_data_[guid].awaiting_subframes.erase(frame_guid); + auto it = all_document_data_.find(guid); + if (it == all_document_data_.end()) + return; + auto* document_data = &it->second; + document_data->finished_subframes.insert(frame_guid); + document_data->awaiting_subframes.erase(frame_guid); } mojom::PaintPreviewStatus PaintPreviewClient::RecordFrame( - base::UnguessableToken guid, - uint64_t frame_guid, + const base::UnguessableToken& guid, + const base::UnguessableToken& frame_guid, bool is_main_frame, const base::FilePath& filename, - content::RenderFrameHost* render_frame_host, + const content::GlobalFrameRoutingId& render_frame_id, mojom::PaintPreviewCaptureResponsePtr response) { auto it = all_document_data_.find(guid); - if (!it->second.proto) { - it->second.proto = std::make_unique<PaintPreviewProto>(); - it->second.proto->mutable_metadata()->set_url( - all_document_data_[guid].root_url.spec()); + if (it == all_document_data_.end()) + return mojom::PaintPreviewStatus::kCaptureFailed; + auto* document_data = &it->second; + if (!document_data->proto) { + document_data->proto = std::make_unique<PaintPreviewProto>(); + document_data->proto->mutable_metadata()->set_url( + document_data->root_url.spec()); } - PaintPreviewProto* proto_ptr = it->second.proto.get(); + PaintPreviewProto* proto_ptr = document_data->proto.get(); PaintPreviewFrameProto* frame_proto; if (is_main_frame) { + document_data->main_frame_blink_recording_time = + response->blink_recording_time; frame_proto = proto_ptr->mutable_root_frame(); frame_proto->set_is_main_frame(true); + uint64_t old_style_id = MakeOldStyleId(render_frame_id); + main_frame_guids_.erase(old_style_id); } else { frame_proto = proto_ptr->add_subframes(); frame_proto->set_is_main_frame(false); } - // Safe since always <#>.skp. + // Safe since always HEX.skp. frame_proto->set_file_path(filename.AsUTF8Unsafe()); - std::vector<uint64_t> remote_frame_guids = + std::vector<base::UnguessableToken> remote_frame_guids = PaintPreviewCaptureResponseToPaintPreviewFrameProto( - std::move(response), render_frame_host, frame_proto); + std::move(response), frame_guid, frame_proto); for (const auto& remote_frame_guid : remote_frame_guids) { - if (!base::Contains(it->second.finished_subframes, remote_frame_guid)) - it->second.awaiting_subframes.insert(remote_frame_guid); + if (!base::Contains(document_data->finished_subframes, remote_frame_guid)) + document_data->awaiting_subframes.insert(remote_frame_guid); } return mojom::PaintPreviewStatus::kOk; } void PaintPreviewClient::OnFinished(base::UnguessableToken guid, PaintPreviewData* document_data) { - if (!document_data) + if (!document_data || !document_data->callback) return; + + base::UmaHistogramBoolean("Browser.PaintPreview.Capture.Success", + document_data->proto != nullptr); if (document_data->proto) { + base::UmaHistogramCounts100( + "Browser.PaintPreview.Capture.NumberOfFramesCaptured", + document_data->finished_subframes.size()); + + RecordUkmCaptureData(document_data->source_id, + document_data->main_frame_blink_recording_time); + // At a minimum one frame was captured successfully, it is up to the // caller to decide if a partial success is acceptable based on what is // contained in the proto. - base::PostTask( - FROM_HERE, - base::BindOnce(std::move(document_data->callback), guid, - document_data->had_error - ? mojom::PaintPreviewStatus::kPartialSuccess - : mojom::PaintPreviewStatus::kOk, - std::move(document_data->proto))); + std::move(document_data->callback) + .Run(guid, + document_data->had_error + ? mojom::PaintPreviewStatus::kPartialSuccess + : mojom::PaintPreviewStatus::kOk, + std::move(document_data->proto)); } else { // A proto could not be created indicating all frames failed to capture. - base::PostTask(FROM_HERE, - base::BindOnce(std::move(document_data->callback), guid, - mojom::PaintPreviewStatus::kFailed, nullptr)); + std::move(document_data->callback) + .Run(guid, mojom::PaintPreviewStatus::kFailed, nullptr); } all_document_data_.erase(guid); } diff --git a/chromium/components/paint_preview/browser/paint_preview_client.h b/chromium/components/paint_preview/browser/paint_preview_client.h index ad921163c8b..593e2f20630 100644 --- a/chromium/components/paint_preview/browser/paint_preview_client.h +++ b/chromium/components/paint_preview/browser/paint_preview_client.h @@ -27,6 +27,9 @@ namespace paint_preview { // Client responsible for making requests to the mojom::PaintPreviewService. A // client coordinates between multiple frames and handles capture and // aggreagation of data from both the main frame and subframes. +// +// Should be created and accessed from the UI thread as WebContentsUserData +// requires this behavior. class PaintPreviewClient : public content::WebContentsUserData<PaintPreviewClient>, public content::WebContentsObserver { @@ -57,6 +60,8 @@ class PaintPreviewClient ~PaintPreviewClient() override; + // IMPORTANT: The Capture* methods must be called on the UI thread! + // Captures a paint preview corresponding to the content of // |render_frame_host|. This will work for capturing entire documents if // passed the main frame or for just a specific subframe depending on @@ -91,17 +96,25 @@ class PaintPreviewClient // Root directory to store artifacts to. base::FilePath root_dir; + base::UnguessableToken root_frame_token; + // URL of the root frame. GURL root_url; + // UKM Source ID of the WebContent. + ukm::SourceId source_id; + + // Main frame capture time. + base::TimeDelta main_frame_blink_recording_time; + // Callback that is invoked on completion of data. PaintPreviewCallback callback; // All the render frames that are still required. - base::flat_set<uint64_t> awaiting_subframes; + base::flat_set<base::UnguessableToken> awaiting_subframes; // All the render frames that have finished. - base::flat_set<uint64_t> finished_subframes; + base::flat_set<base::UnguessableToken> finished_subframes; // Data proto that is returned via callback. std::unique_ptr<PaintPreviewProto> proto; @@ -148,34 +161,36 @@ class PaintPreviewClient // |render_frame_host| using |params| to configure the request. |frame_guid| // is the GUID associated with the frame. |path| is file path associated with // the File stored in |result| (base::File isn't aware of its file path). - void RequestCaptureOnUIThread(const PaintPreviewParams& params, - uint64_t frame_guid, - content::RenderFrameHost* render_frame_host, - const base::FilePath& path, - CreateResult result); + void RequestCaptureOnUIThread( + const PaintPreviewParams& params, + const base::UnguessableToken& frame_guid, + const content::GlobalFrameRoutingId& render_frame_id, + const base::FilePath& path, + CreateResult result); // Handles recording the frame and updating client state when capture is // complete. void OnPaintPreviewCapturedCallback( - base::UnguessableToken guid, - uint64_t frame_guid, + const base::UnguessableToken& guid, + const base::UnguessableToken& frame_guid, bool is_main_frame, const base::FilePath& filename, - content::RenderFrameHost* render_frame_host, + const content::GlobalFrameRoutingId& render_frame_id, mojom::PaintPreviewStatus status, mojom::PaintPreviewCaptureResponsePtr response); // Marks a frame as having been processed, this should occur regardless of // whether the processed frame is valid as there is no retry. - void MarkFrameAsProcessed(base::UnguessableToken guid, uint64_t frame_guid); + void MarkFrameAsProcessed(base::UnguessableToken guid, + const base::UnguessableToken& frame_guid); // Records the data from a processed frame if it was captured successfully. mojom::PaintPreviewStatus RecordFrame( - base::UnguessableToken guid, - uint64_t frame_guid, + const base::UnguessableToken& guid, + const base::UnguessableToken& frame_guid, bool is_main_frame, const base::FilePath& filename, - content::RenderFrameHost* render_frame_host, + const content::GlobalFrameRoutingId& render_frame_id, mojom::PaintPreviewCaptureResponsePtr response); // Handles finishing the capture once all frames are received. @@ -183,12 +198,17 @@ class PaintPreviewClient // Storage ------------------------------------------------------------------ + // Mapping of Process ID || Routing ID to unguessable tokens for the main + // frame. + base::flat_map<uint64_t, base::UnguessableToken> main_frame_guids_; + // Maps a RenderFrameHost and document to a remote interface. - base::flat_map<uint64_t, mojo::AssociatedRemote<mojom::PaintPreviewRecorder>> + base::flat_map<base::UnguessableToken, + mojo::AssociatedRemote<mojom::PaintPreviewRecorder>> interface_ptrs_; // Maps render frame's GUID and document cookies that requested the frame. - base::flat_map<uint64_t, base::flat_set<base::UnguessableToken>> + base::flat_map<base::UnguessableToken, base::flat_set<base::UnguessableToken>> pending_previews_on_subframe_; // Maps a document GUID to its data. 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 76f9ca34f97..91962004fd7 100644 --- a/chromium/components/paint_preview/browser/paint_preview_client_unittest.cc +++ b/chromium/components/paint_preview/browser/paint_preview_client_unittest.cc @@ -80,12 +80,6 @@ class MockPaintPreviewRecorder : public mojom::PaintPreviewRecorder { MockPaintPreviewRecorder& operator=(const MockPaintPreviewRecorder&) = delete; }; -// Returns the GUID corresponding to |rfh|. -uint64_t FrameGuid(content::RenderFrameHost* rfh) { - return static_cast<uint64_t>(rfh->GetProcess()->GetID()) << 32 | - rfh->GetRoutingID(); -} - // Convert |params| to the mojo::PaintPreviewServiceParams format. NOTE: this // does not set the file parameter as the file is created in the client // internals and should be treated as an opaque file (with an unknown path) in @@ -140,32 +134,41 @@ TEST_F(PaintPreviewClientRenderViewHostTest, CaptureMainFrameMock) { params.is_main_frame = true; content::RenderFrameHost* rfh = main_rfh(); - uint64_t frame_guid = FrameGuid(rfh); GURL expected_url = rfh->GetLastCommittedURL(); auto response = mojom::PaintPreviewCaptureResponse::New(); - response->id = rfh->GetRoutingID(); + response->embedding_token = base::nullopt; PaintPreviewProto expected_proto; expected_proto.mutable_metadata()->set_url(expected_url.spec()); PaintPreviewFrameProto* main_frame = expected_proto.mutable_root_frame(); main_frame->set_is_main_frame(true); - main_frame->set_id(frame_guid); - main_frame->set_file_path( - temp_dir_.GetPath() - .AppendASCII( - base::StrCat({base::NumberToString(main_frame->id()), ".skp"})) - .AsUTF8Unsafe()); base::RunLoop loop; auto callback = base::BindOnce( [](base::RepeatingClosure quit, base::UnguessableToken expected_guid, - PaintPreviewProto expected_proto, base::UnguessableToken returned_guid, - mojom::PaintPreviewStatus status, + base::FilePath temp_dir, PaintPreviewProto expected_proto, + base::UnguessableToken returned_guid, mojom::PaintPreviewStatus status, std::unique_ptr<PaintPreviewProto> proto) { EXPECT_EQ(returned_guid, expected_guid); EXPECT_EQ(status, mojom::PaintPreviewStatus::kOk); + auto token = base::UnguessableToken::Deserialize( + proto->root_frame().embedding_token_high(), + proto->root_frame().embedding_token_low()); + EXPECT_NE(token, base::UnguessableToken::Null()); + + // The token for the main frame is set internally since the render frame + // host won't have one. To simplify the proto comparison using + // EqualsProto copy the generated one into |expected_proto|. + PaintPreviewFrameProto* main_frame = + expected_proto.mutable_root_frame(); + main_frame->set_embedding_token_low(token.GetLowForSerialization()); + main_frame->set_embedding_token_high(token.GetHighForSerialization()); + main_frame->set_file_path( + temp_dir.AppendASCII(base::StrCat({token.ToString(), ".skp"})) + .AsUTF8Unsafe()); + EXPECT_THAT(*proto, EqualsProto(expected_proto)); { base::ScopedAllowBlockingForTesting scope; @@ -179,7 +182,8 @@ TEST_F(PaintPreviewClientRenderViewHostTest, CaptureMainFrameMock) { } quit.Run(); }, - loop.QuitClosure(), params.document_guid, expected_proto); + loop.QuitClosure(), params.document_guid, temp_dir_.GetPath(), + expected_proto); MockPaintPreviewRecorder service; service.SetExpectedParams(ToMojoParams(params)); service.SetResponse(mojom::PaintPreviewStatus::kOk, std::move(response)); @@ -239,7 +243,7 @@ TEST_F(PaintPreviewClientRenderViewHostTest, RenderFrameDeletedDuringCapture) { content::RenderFrameHost* rfh = main_rfh(); auto response = mojom::PaintPreviewCaptureResponse::New(); - response->id = rfh->GetRoutingID(); + response->embedding_token = base::nullopt; base::RunLoop loop; auto callback = base::BindOnce( 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 new file mode 100644 index 00000000000..24d11c3faf3 --- /dev/null +++ b/chromium/components/paint_preview/browser/paint_preview_compositor_client_impl.cc @@ -0,0 +1,92 @@ +// 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_compositor_client_impl.h" + +#include <utility> + +#include "base/callback.h" + +namespace paint_preview { + +PaintPreviewCompositorClientImpl::PaintPreviewCompositorClientImpl( + base::WeakPtr<PaintPreviewCompositorServiceImpl> service) + : service_(service) {} + +PaintPreviewCompositorClientImpl::~PaintPreviewCompositorClientImpl() { + NotifyServiceOfInvalidation(); +} + +const base::Optional<base::UnguessableToken>& +PaintPreviewCompositorClientImpl::Token() const { + return token_; +} + +void PaintPreviewCompositorClientImpl::SetDisconnectHandler( + base::OnceClosure closure) { + user_disconnect_closure_ = std::move(closure); +} + +void PaintPreviewCompositorClientImpl::BeginComposite( + mojom::PaintPreviewBeginCompositeRequestPtr request, + mojom::PaintPreviewCompositor::BeginCompositeCallback callback) { + compositor_->BeginComposite(std::move(request), std::move(callback)); +} + +void PaintPreviewCompositorClientImpl::BitmapForFrame( + const base::UnguessableToken& frame_guid, + const gfx::Rect& clip_rect, + float scale_factor, + mojom::PaintPreviewCompositor::BitmapForFrameCallback callback) { + compositor_->BitmapForFrame(frame_guid, clip_rect, scale_factor, + std::move(callback)); +} + +void PaintPreviewCompositorClientImpl::SetRootFrameUrl(const GURL& url) { + compositor_->SetRootFrameUrl(url); +} + +bool PaintPreviewCompositorClientImpl::IsBoundAndConnected() const { + return compositor_.is_bound() && compositor_.is_connected(); +} + +mojo::PendingReceiver<mojom::PaintPreviewCompositor> +PaintPreviewCompositorClientImpl::BindNewPipeAndPassReceiver() { + return compositor_.BindNewPipeAndPassReceiver(); +} + +PaintPreviewCompositorClientImpl::OnCompositorCreatedCallback +PaintPreviewCompositorClientImpl::BuildCompositorCreatedCallback( + base::OnceClosure user_closure, + OnCompositorCreatedCallback service_callback) { + return base::BindOnce(&PaintPreviewCompositorClientImpl::OnCompositorCreated, + weak_ptr_factory_.GetWeakPtr(), std::move(user_closure), + std::move(service_callback)); +} + +void PaintPreviewCompositorClientImpl::OnCompositorCreated( + base::OnceClosure user_closure, + OnCompositorCreatedCallback service_callback, + const base::UnguessableToken& token) { + token_ = token; + std::move(user_closure).Run(); + std::move(service_callback).Run(token); + compositor_.set_disconnect_handler( + base::BindOnce(&PaintPreviewCompositorClientImpl::DisconnectHandler, + weak_ptr_factory_.GetWeakPtr())); +} + +void PaintPreviewCompositorClientImpl::NotifyServiceOfInvalidation() { + if (service_ && token_.has_value()) + service_->MarkCompositorAsDeleted(token_.value()); +} + +void PaintPreviewCompositorClientImpl::DisconnectHandler() { + if (user_disconnect_closure_) + std::move(user_disconnect_closure_).Run(); + NotifyServiceOfInvalidation(); + compositor_.reset(); +} + +} // namespace paint_preview 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 new file mode 100644 index 00000000000..d8bbf797e63 --- /dev/null +++ b/chromium/components/paint_preview/browser/paint_preview_compositor_client_impl.h @@ -0,0 +1,77 @@ +// 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_COMPOSITOR_CLIENT_IMPL_H_ +#define COMPONENTS_PAINT_PREVIEW_BROWSER_PAINT_PREVIEW_COMPOSITOR_CLIENT_IMPL_H_ + +#include "base/callback_forward.h" +#include "base/optional.h" +#include "base/unguessable_token.h" +#include "components/paint_preview/browser/paint_preview_compositor_service_impl.h" +#include "components/paint_preview/public/paint_preview_compositor_client.h" +#include "components/services/paint_preview_compositor/public/mojom/paint_preview_compositor.mojom.h" +#include "mojo/public/cpp/bindings/remote.h" +#include "ui/gfx/geometry/rect.h" +#include "url/gurl.h" + +namespace paint_preview { + +class PaintPreviewCompositorClientImpl : public PaintPreviewCompositorClient { + public: + using OnCompositorCreatedCallback = + base::OnceCallback<void(const base::UnguessableToken&)>; + + explicit PaintPreviewCompositorClientImpl( + base::WeakPtr<PaintPreviewCompositorServiceImpl> service); + ~PaintPreviewCompositorClientImpl() override; + + // PaintPreviewCompositorClient implementation. + const base::Optional<base::UnguessableToken>& Token() const override; + void SetDisconnectHandler(base::OnceClosure closure) override; + void BeginComposite( + mojom::PaintPreviewBeginCompositeRequestPtr request, + mojom::PaintPreviewCompositor::BeginCompositeCallback callback) override; + void BitmapForFrame( + const base::UnguessableToken& frame_guid, + const gfx::Rect& clip_rect, + float scale_factor, + mojom::PaintPreviewCompositor::BitmapForFrameCallback callback) override; + void SetRootFrameUrl(const GURL& url) override; + + // Exposes underlying BindNewPipeAndPassReceiver method of |compositor_|. + mojo::PendingReceiver<mojom::PaintPreviewCompositor> + BindNewPipeAndPassReceiver(); + + bool IsBoundAndConnected() const; + + OnCompositorCreatedCallback BuildCompositorCreatedCallback( + base::OnceClosure user_closure, + OnCompositorCreatedCallback service_callback); + + PaintPreviewCompositorClientImpl(const PaintPreviewCompositorClientImpl&) = + delete; + PaintPreviewCompositorClientImpl& operator=( + const PaintPreviewCompositorClientImpl&) = delete; + + private: + void OnCompositorCreated(base::OnceClosure user_closure, + OnCompositorCreatedCallback service_callback, + const base::UnguessableToken& token); + + void NotifyServiceOfInvalidation(); + + void DisconnectHandler(); + + base::Optional<base::UnguessableToken> token_; + base::WeakPtr<PaintPreviewCompositorServiceImpl> service_; + mojo::Remote<mojom::PaintPreviewCompositor> compositor_; + base::OnceClosure user_disconnect_closure_; + + base::WeakPtrFactory<PaintPreviewCompositorClientImpl> weak_ptr_factory_{ + this}; +}; + +} // namespace paint_preview + +#endif // COMPONENTS_PAINT_PREVIEW_BROWSER_PAINT_PREVIEW_COMPOSITOR_CLIENT_IMPL_H_ 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 new file mode 100644 index 00000000000..ea75e7ef592 --- /dev/null +++ b/chromium/components/paint_preview/browser/paint_preview_compositor_service_impl.cc @@ -0,0 +1,70 @@ +// 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_compositor_service_impl.h" + +#include "components/paint_preview/browser/paint_preview_compositor_client_impl.h" +#include "components/paint_preview/public/paint_preview_compositor_client.h" + +namespace paint_preview { + +PaintPreviewCompositorServiceImpl::PaintPreviewCompositorServiceImpl( + mojo::Remote<mojom::PaintPreviewCompositorCollection> remote, + base::OnceClosure disconnect_handler) + : compositor_service_(std::move(remote)), + user_disconnect_closure_(std::move(disconnect_handler)) { + compositor_service_.set_disconnect_handler( + base::BindOnce(&PaintPreviewCompositorServiceImpl::DisconnectHandler, + weak_ptr_factory_.GetWeakPtr())); +} + +// The destructor for the |compositor_service_| will automatically result in any +// active compositors being killed. +PaintPreviewCompositorServiceImpl::~PaintPreviewCompositorServiceImpl() = + default; + +std::unique_ptr<PaintPreviewCompositorClient> +PaintPreviewCompositorServiceImpl::CreateCompositor( + base::OnceClosure connected_closure) { + auto compositor = std::make_unique<PaintPreviewCompositorClientImpl>( + weak_ptr_factory_.GetWeakPtr()); + compositor_service_->CreateCompositor( + compositor->BindNewPipeAndPassReceiver(), + compositor->BuildCompositorCreatedCallback( + std::move(connected_closure), + base::BindOnce( + &PaintPreviewCompositorServiceImpl::OnCompositorCreated, + weak_ptr_factory_.GetWeakPtr()))); + return compositor; +} + +bool PaintPreviewCompositorServiceImpl::HasActiveClients() const { + return !active_clients_.empty(); +} + +void PaintPreviewCompositorServiceImpl::MarkCompositorAsDeleted( + const base::UnguessableToken& token) { + active_clients_.erase(token); +} + +bool PaintPreviewCompositorServiceImpl::IsServiceBoundAndConnected() const { + return compositor_service_.is_bound() && compositor_service_.is_connected(); +} + +const base::flat_set<base::UnguessableToken>& +PaintPreviewCompositorServiceImpl::ActiveClientsForTesting() const { + return active_clients_; +} + +void PaintPreviewCompositorServiceImpl::OnCompositorCreated( + const base::UnguessableToken& token) { + active_clients_.insert(token); +} + +void PaintPreviewCompositorServiceImpl::DisconnectHandler() { + std::move(user_disconnect_closure_).Run(); + compositor_service_.reset(); +} + +} // namespace paint_preview diff --git a/chromium/components/paint_preview/browser/paint_preview_compositor_service_impl.h b/chromium/components/paint_preview/browser/paint_preview_compositor_service_impl.h new file mode 100644 index 00000000000..9361780709d --- /dev/null +++ b/chromium/components/paint_preview/browser/paint_preview_compositor_service_impl.h @@ -0,0 +1,60 @@ +// 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_COMPOSITOR_SERVICE_IMPL_H_ +#define COMPONENTS_PAINT_PREVIEW_BROWSER_PAINT_PREVIEW_COMPOSITOR_SERVICE_IMPL_H_ + +#include <memory> + +#include "base/callback_forward.h" +#include "base/containers/flat_set.h" +#include "base/unguessable_token.h" +#include "components/paint_preview/public/paint_preview_compositor_service.h" +#include "components/services/paint_preview_compositor/public/mojom/paint_preview_compositor.mojom.h" +#include "mojo/public/cpp/bindings/remote.h" + +namespace paint_preview { + +class PaintPreviewCompositorServiceImpl : public PaintPreviewCompositorService { + public: + explicit PaintPreviewCompositorServiceImpl( + mojo::Remote<mojom::PaintPreviewCompositorCollection> remote, + base::OnceClosure disconnect_closure); + ~PaintPreviewCompositorServiceImpl() override; + + // PaintPreviewCompositorService Implementation. + std::unique_ptr<PaintPreviewCompositorClient> CreateCompositor( + base::OnceClosure connected_closure) override; + bool HasActiveClients() const override; + + // Marks the compositor associated with |token| as deleted in the + // |active_clients_| set. + void MarkCompositorAsDeleted(const base::UnguessableToken& token); + + bool IsServiceBoundAndConnected() const; + + // Test method to validate internal state. + const base::flat_set<base::UnguessableToken>& ActiveClientsForTesting() const; + + PaintPreviewCompositorServiceImpl(const PaintPreviewCompositorServiceImpl&) = + delete; + PaintPreviewCompositorServiceImpl& operator=( + const PaintPreviewCompositorServiceImpl&) = delete; + + private: + void OnCompositorCreated(const base::UnguessableToken& token); + + void DisconnectHandler(); + + mojo::Remote<mojom::PaintPreviewCompositorCollection> compositor_service_; + base::flat_set<base::UnguessableToken> active_clients_; + base::OnceClosure user_disconnect_closure_; + + base::WeakPtrFactory<PaintPreviewCompositorServiceImpl> weak_ptr_factory_{ + this}; +}; + +} // namespace paint_preview + +#endif // COMPONENTS_PAINT_PREVIEW_BROWSER_PAINT_PREVIEW_COMPOSITOR_SERVICE_IMPL_H_ diff --git a/chromium/components/paint_preview/browser/paint_preview_policy.h b/chromium/components/paint_preview/browser/paint_preview_policy.h new file mode 100644 index 00000000000..16972f3063c --- /dev/null +++ b/chromium/components/paint_preview/browser/paint_preview_policy.h @@ -0,0 +1,25 @@ +// 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_POLICY_H_ +#define COMPONENTS_PAINT_PREVIEW_BROWSER_PAINT_PREVIEW_POLICY_H_ + +namespace content { +class WebContents; +} + +namespace paint_preview { + +// Subclasses of PaintPreviewPolicy are responsible for determining whether a +// given WebContents are amenable for PaintPreview. For example, sites that make +// heavy use of script. +class PaintPreviewPolicy { + public: + virtual ~PaintPreviewPolicy() = default; + virtual bool SupportedForContents(content::WebContents* web_contents) = 0; +}; + +} // namespace paint_preview + +#endif // COMPONENTS_PAINT_PREVIEW_BROWSER_PAINT_PREVIEW_POLICY_H_ diff --git a/chromium/components/paint_preview/browser/test_paint_preview_policy.cc b/chromium/components/paint_preview/browser/test_paint_preview_policy.cc new file mode 100644 index 00000000000..986582ff6a7 --- /dev/null +++ b/chromium/components/paint_preview/browser/test_paint_preview_policy.cc @@ -0,0 +1,23 @@ +// 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/test_paint_preview_policy.h" + +#include "base/callback.h" +#include "base/optional.h" +#include "components/paint_preview/browser/paint_preview_policy.h" +#include "content/public/browser/web_contents.h" + +namespace paint_preview { + +TestPaintPreviewPolicy::TestPaintPreviewPolicy() = default; + +TestPaintPreviewPolicy::~TestPaintPreviewPolicy() = default; + +bool TestPaintPreviewPolicy::SupportedForContents( + content::WebContents* web_contents) { + return supported_for_contents_; +} + +} // namespace paint_preview diff --git a/chromium/components/paint_preview/browser/test_paint_preview_policy.h b/chromium/components/paint_preview/browser/test_paint_preview_policy.h new file mode 100644 index 00000000000..ab2be3518ec --- /dev/null +++ b/chromium/components/paint_preview/browser/test_paint_preview_policy.h @@ -0,0 +1,37 @@ +// 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_TEST_PAINT_PREVIEW_POLICY_H_ +#define COMPONENTS_PAINT_PREVIEW_BROWSER_TEST_PAINT_PREVIEW_POLICY_H_ + +#include "components/paint_preview/browser/paint_preview_policy.h" + +namespace content { +class WebContents; +} + +namespace paint_preview { + +// Simple implementation of PaintPreviewPolicy used in tests. +// SupportedForContents defaults to true, but tests may change +// this by calling SetSupportedForContents. +class TestPaintPreviewPolicy : public PaintPreviewPolicy { + public: + TestPaintPreviewPolicy(); + + ~TestPaintPreviewPolicy() override; + + bool SupportedForContents(content::WebContents* web_contents) override; + + void SetSupportedForContents(bool supported_for_contents) { + supported_for_contents_ = supported_for_contents; + } + + private: + bool supported_for_contents_ = true; +}; + +} // namespace paint_preview + +#endif // COMPONENTS_PAINT_PREVIEW_BROWSER_TEST_PAINT_PREVIEW_POLICY_H_ diff --git a/chromium/components/paint_preview/buildflags/BUILD.gn b/chromium/components/paint_preview/buildflags/BUILD.gn index c15c160561c..853f9317743 100644 --- a/chromium/components/paint_preview/buildflags/BUILD.gn +++ b/chromium/components/paint_preview/buildflags/BUILD.gn @@ -3,7 +3,7 @@ # found in the LICENSE file. import("//build/buildflag_header.gni") -import("//components/paint_preview/buildflags/buildflags.gni") +import("//build/config/buildflags_paint_preview.gni") # This target is standalone so any targets in the build can refer to the # buildflag without bringing in any paint preview targets. diff --git a/chromium/components/paint_preview/buildflags/buildflags.gni b/chromium/components/paint_preview/buildflags/buildflags.gni deleted file mode 100644 index 1ce611b2a59..00000000000 --- a/chromium/components/paint_preview/buildflags/buildflags.gni +++ /dev/null @@ -1,14 +0,0 @@ -# Copyright 2019 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. - -import("//build/config/chromecast_build.gni") -import("//build/config/features.gni") - -declare_args() { - # Enable basic paint preview support. Does not work on iOS or Fuchsia. Should - # not be included with Chromecast. Not ready for shipping builds yet so - # include in unofficial builds. - enable_paint_preview = - !is_chromecast && !is_ios && !is_fuchsia && !is_official_build -} diff --git a/chromium/components/paint_preview/common/BUILD.gn b/chromium/components/paint_preview/common/BUILD.gn index f7609a3128a..3d6c535bb45 100644 --- a/chromium/components/paint_preview/common/BUILD.gn +++ b/chromium/components/paint_preview/common/BUILD.gn @@ -9,6 +9,8 @@ if (!is_ios) { sources = [ "file_stream.cc", "file_stream.h", + "file_utils.cc", + "file_utils.h", "glyph_usage.cc", "glyph_usage.h", "paint_preview_tracker.cc", @@ -21,23 +23,21 @@ if (!is_ios) { deps = [ "//base", + "//components/paint_preview/common/proto", "//skia", "//third_party:freetype_harfbuzz", + "//third_party/harfbuzz-ng:hb_scoped_util", "//ui/gfx/geometry", "//url", ] - public_deps = [ - "//components/paint_preview/common/mojom", - ] + public_deps = [ "//components/paint_preview/common/mojom" ] } source_set("test_utils") { testonly = true - sources = [ - "test_utils.h", - ] + sources = [ "test_utils.h" ] deps = [ "//testing/gmock", diff --git a/chromium/components/paint_preview/common/file_utils.cc b/chromium/components/paint_preview/common/file_utils.cc new file mode 100644 index 00000000000..eb169185599 --- /dev/null +++ b/chromium/components/paint_preview/common/file_utils.cc @@ -0,0 +1,37 @@ +// 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/file_utils.h" + +#include <memory> + +#include "base/files/file_path.h" +#include "base/files/file_util.h" +#include "components/paint_preview/common/proto/paint_preview.pb.h" + +namespace paint_preview { + +bool WriteProtoToFile(const base::FilePath& file_path, + const PaintPreviewProto& proto) { + // TODO(crbug.com/1046925): stream to file instead. + std::string proto_str; + if (!proto.SerializeToString(&proto_str)) + return false; + return base::WriteFile(file_path, proto_str.data(), proto_str.size()) > 0; +} + +std::unique_ptr<PaintPreviewProto> ReadProtoFromFile( + const base::FilePath& file_path) { + // TODO(crbug.com/1046925): Use ZeroCopyInputStream instead. + std::string out; + std::unique_ptr<PaintPreviewProto> proto = + std::make_unique<PaintPreviewProto>(); + if (!base::ReadFileToString(file_path, &out) || + !(proto->ParseFromString(out))) + return nullptr; + + return proto; +} + +} // namespace paint_preview diff --git a/chromium/components/paint_preview/common/file_utils.h b/chromium/components/paint_preview/common/file_utils.h new file mode 100644 index 00000000000..ab583f66659 --- /dev/null +++ b/chromium/components/paint_preview/common/file_utils.h @@ -0,0 +1,27 @@ +// 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_FILE_UTILS_H_ +#define COMPONENTS_PAINT_PREVIEW_COMMON_FILE_UTILS_H_ + +#include <memory> + +#include "base/files/file_path.h" + +namespace paint_preview { + +class PaintPreviewProto; + +// Writes |proto| to |file_path|. Returns false on failure. +bool WriteProtoToFile(const base::FilePath& file_path, + const PaintPreviewProto& proto); + +// Reads a PaintPreviewProto from |file_path|. Returns nullptr in case of +// failure. +std::unique_ptr<PaintPreviewProto> ReadProtoFromFile( + const base::FilePath& file_path); + +} // namespace paint_preview + +#endif // COMPONENTS_PAINT_PREVIEW_COMMON_FILE_UTILS_H_ diff --git a/chromium/components/paint_preview/common/mojom/BUILD.gn b/chromium/components/paint_preview/common/mojom/BUILD.gn index 59ec8b4dcda..bd514d0584e 100644 --- a/chromium/components/paint_preview/common/mojom/BUILD.gn +++ b/chromium/components/paint_preview/common/mojom/BUILD.gn @@ -5,9 +5,7 @@ import("//mojo/public/tools/bindings/mojom.gni") mojom("mojom") { - sources = [ - "paint_preview_recorder.mojom", - ] + sources = [ "paint_preview_recorder.mojom" ] public_deps = [ "//mojo/public/mojom/base", diff --git a/chromium/components/paint_preview/common/mojom/paint_preview_recorder.mojom b/chromium/components/paint_preview/common/mojom/paint_preview_recorder.mojom index bb6ba91d5f6..f0970b511d1 100644 --- a/chromium/components/paint_preview/common/mojom/paint_preview_recorder.mojom +++ b/chromium/components/paint_preview/common/mojom/paint_preview_recorder.mojom @@ -5,6 +5,7 @@ module paint_preview.mojom; import "mojo/public/mojom/base/file.mojom"; +import "mojo/public/mojom/base/time.mojom"; import "mojo/public/mojom/base/unguessable_token.mojom"; import "ui/gfx/geometry/mojom/geometry.mojom"; import "url/mojom/url.mojom"; @@ -60,14 +61,17 @@ struct LinkData { }; struct PaintPreviewCaptureResponse { - // Routing ID of the frame. - uint64 id; + // Embedding token of the frame. Will be nullopt for the main frame. + mojo_base.mojom.UnguessableToken? embedding_token; // Map between subframe content IDs and the RenderFrameProxy Routing ID. - map<uint32, uint64> content_id_proxy_id_map; + map<uint32, mojo_base.mojom.UnguessableToken> content_id_to_embedding_token; // A list of links within the frame. array<LinkData> links; + + // The time spent capturing in Blink. + mojo_base.mojom.TimeDelta blink_recording_time; }; // Service for capturing a paint preview of a RenderFrame's contents. This diff --git a/chromium/components/paint_preview/common/paint_preview_tracker.cc b/chromium/components/paint_preview/common/paint_preview_tracker.cc index 4d73c7b708c..b584d36d404 100644 --- a/chromium/components/paint_preview/common/paint_preview_tracker.cc +++ b/chromium/components/paint_preview/common/paint_preview_tracker.cc @@ -38,19 +38,23 @@ bool ShouldUseDenseGlyphUsage(SkTypeface* typeface) { } // namespace -PaintPreviewTracker::PaintPreviewTracker(const base::UnguessableToken& guid, - int routing_id, - bool is_main_frame) - : guid_(guid), routing_id_(routing_id), is_main_frame_(is_main_frame) {} +PaintPreviewTracker::PaintPreviewTracker( + const base::UnguessableToken& guid, + const base::Optional<base::UnguessableToken>& embedding_token, + bool is_main_frame) + : guid_(guid), + embedding_token_(embedding_token), + is_main_frame_(is_main_frame) {} PaintPreviewTracker::~PaintPreviewTracker() = default; -uint32_t PaintPreviewTracker::CreateContentForRemoteFrame(const gfx::Rect& rect, - int routing_id) { +uint32_t PaintPreviewTracker::CreateContentForRemoteFrame( + const gfx::Rect& rect, + const base::UnguessableToken& embedding_token) { sk_sp<SkPicture> pic = SkPicture::MakePlaceholder( SkRect::MakeXYWH(rect.x(), rect.y(), rect.width(), rect.height())); const uint32_t content_id = pic->uniqueID(); - DCHECK(!base::Contains(content_id_to_proxy_id_, content_id)); - content_id_to_proxy_id_[content_id] = routing_id; + DCHECK(!base::Contains(content_id_to_embedding_token_, content_id)); + content_id_to_embedding_token_[content_id] = embedding_token; subframe_pics_[content_id] = pic; return content_id; } @@ -91,8 +95,8 @@ void PaintPreviewTracker::AnnotateLink(const GURL& url, const gfx::Rect& rect) { void PaintPreviewTracker::CustomDataToSkPictureCallback(SkCanvas* canvas, uint32_t content_id) { - auto map_it = content_id_to_proxy_id_.find(content_id); - if (map_it == content_id_to_proxy_id_.end()) + auto map_it = content_id_to_embedding_token_.find(content_id); + if (map_it == content_id_to_embedding_token_.end()) return; auto it = subframe_pics_.find(content_id); diff --git a/chromium/components/paint_preview/common/paint_preview_tracker.h b/chromium/components/paint_preview/common/paint_preview_tracker.h index 46da30b18b3..aed422aaaa3 100644 --- a/chromium/components/paint_preview/common/paint_preview_tracker.h +++ b/chromium/components/paint_preview/common/paint_preview_tracker.h @@ -27,23 +27,28 @@ namespace paint_preview { // produce a PaintPreviewFrameProto. class PaintPreviewTracker { public: - PaintPreviewTracker(const base::UnguessableToken& guid, - int routing_id, - bool is_main_frame); + PaintPreviewTracker( + const base::UnguessableToken& guid, + const base::Optional<base::UnguessableToken>& embedding_token, + bool is_main_frame); ~PaintPreviewTracker(); // Getters ------------------------------------------------------------------ - base::UnguessableToken Guid() const { return guid_; } - int RoutingId() const { return routing_id_; } + const base::UnguessableToken& Guid() const { return guid_; } + const base::Optional<base::UnguessableToken>& EmbeddingToken() const { + return embedding_token_; + } bool IsMainFrame() const { return is_main_frame_; } // Data Collection ---------------------------------------------------------- // Creates a placeholder SkPicture for an OOP subframe located at |rect| - // mapped to the |routing_id| of OOP RenderFrame. Returns the content id of - // the placeholder SkPicture. - uint32_t CreateContentForRemoteFrame(const gfx::Rect& rect, int routing_id); + // mapped to the |embedding_token| of OOP RenderFrame. Returns the content id + // of the placeholder SkPicture. + uint32_t CreateContentForRemoteFrame( + const gfx::Rect& rect, + const base::UnguessableToken& embedding_token); // Adds the glyphs in |blob| to the glyph usage tracker for the |blob|'s // associated typface. @@ -63,7 +68,7 @@ class PaintPreviewTracker { // Expose internal maps for use in MakeSerialProcs(). // NOTE: Cannot be const due to how SkPicture procs work. PictureSerializationContext* GetPictureSerializationContext() { - return &content_id_to_proxy_id_; + return &content_id_to_embedding_token_; } TypefaceUsageMap* GetTypefaceUsageMap() { return &typeface_glyph_usage_; } @@ -72,11 +77,11 @@ class PaintPreviewTracker { private: const base::UnguessableToken guid_; - const int routing_id_; + const base::Optional<base::UnguessableToken> embedding_token_; const bool is_main_frame_; std::vector<mojom::LinkData> links_; - PictureSerializationContext content_id_to_proxy_id_; + PictureSerializationContext content_id_to_embedding_token_; TypefaceUsageMap typeface_glyph_usage_; base::flat_map<uint32_t, sk_sp<SkPicture>> subframe_pics_; diff --git a/chromium/components/paint_preview/common/paint_preview_tracker_unittest.cc b/chromium/components/paint_preview/common/paint_preview_tracker_unittest.cc index adbbf22dcd8..02888e23a11 100644 --- a/chromium/components/paint_preview/common/paint_preview_tracker_unittest.cc +++ b/chromium/components/paint_preview/common/paint_preview_tracker_unittest.cc @@ -18,8 +18,6 @@ namespace paint_preview { namespace { -constexpr int32_t kRoutingId = 1; - struct TestContext { const gfx::Rect* rect; bool was_called; @@ -28,23 +26,30 @@ struct TestContext { } // namespace TEST(PaintPreviewTrackerTest, TestGetters) { - auto token = base::UnguessableToken::Create(); - PaintPreviewTracker tracker(token, kRoutingId, true); - EXPECT_EQ(tracker.Guid(), token); - EXPECT_EQ(tracker.RoutingId(), kRoutingId); + const base::UnguessableToken kDocToken = base::UnguessableToken::Create(); + const base::UnguessableToken kEmbeddingToken = + base::UnguessableToken::Create(); + PaintPreviewTracker tracker(kDocToken, kEmbeddingToken, true); + EXPECT_EQ(tracker.Guid(), kDocToken); + EXPECT_EQ(tracker.EmbeddingToken(), kEmbeddingToken); EXPECT_TRUE(tracker.IsMainFrame()); } TEST(PaintPreviewTrackerTest, TestRemoteFramePlaceholderPicture) { - PaintPreviewTracker tracker(base::UnguessableToken::Create(), kRoutingId, - true); - const int kRoutingId = 50; + const base::UnguessableToken kDocToken = base::UnguessableToken::Create(); + const base::UnguessableToken kEmbeddingToken = + base::UnguessableToken::Create(); + PaintPreviewTracker tracker(kDocToken, kEmbeddingToken, true); + + const base::UnguessableToken kEmbeddingTokenChild = + base::UnguessableToken::Create(); gfx::Rect rect(50, 40, 30, 20); - uint32_t content_id = tracker.CreateContentForRemoteFrame(rect, kRoutingId); + uint32_t content_id = + tracker.CreateContentForRemoteFrame(rect, kEmbeddingTokenChild); PictureSerializationContext* context = tracker.GetPictureSerializationContext(); EXPECT_TRUE(context->count(content_id)); - EXPECT_EQ((*context)[content_id], static_cast<uint32_t>(kRoutingId)); + EXPECT_EQ((*context)[content_id], kEmbeddingTokenChild); SkPictureRecorder recorder; SkCanvas* canvas = recorder.beginRecording(100, 100); @@ -57,7 +62,9 @@ TEST(PaintPreviewTrackerTest, TestRemoteFramePlaceholderPicture) { } TEST(PaintPreviewTrackerTest, TestGlyphRunList) { - PaintPreviewTracker tracker(base::UnguessableToken::Create(), kRoutingId, + const base::UnguessableToken kEmbeddingToken = + base::UnguessableToken::Create(); + PaintPreviewTracker tracker(base::UnguessableToken::Create(), kEmbeddingToken, true); std::string unichars = "abc"; auto typeface = SkTypeface::MakeDefault(); @@ -75,7 +82,9 @@ TEST(PaintPreviewTrackerTest, TestGlyphRunList) { } TEST(PaintPreviewTrackerTest, TestAnnotateLinks) { - PaintPreviewTracker tracker(base::UnguessableToken::Create(), kRoutingId, + const base::UnguessableToken kEmbeddingToken = + base::UnguessableToken::Create(); + PaintPreviewTracker tracker(base::UnguessableToken::Create(), kEmbeddingToken, true); const GURL url_1("https://www.chromium.org"); const gfx::Rect rect_1(10, 20, 30, 40); diff --git a/chromium/components/paint_preview/common/proto/BUILD.gn b/chromium/components/paint_preview/common/proto/BUILD.gn index 39e32b4b5d3..2694d4489e4 100644 --- a/chromium/components/paint_preview/common/proto/BUILD.gn +++ b/chromium/components/paint_preview/common/proto/BUILD.gn @@ -5,7 +5,5 @@ import("//third_party/protobuf/proto_library.gni") proto_library("proto") { - sources = [ - "paint_preview.proto", - ] + 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 0506481fb02..155be9dd555 100644 --- a/chromium/components/paint_preview/common/proto/paint_preview.proto +++ b/chromium/components/paint_preview/common/proto/paint_preview.proto @@ -24,24 +24,33 @@ message LinkDataProto { required 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; +} + // A paint preview of a single frame. -// NEXT_TAG = 6 +// NEXT_TAG = 7 message PaintPreviewFrameProto { - // Originates in renderer as Routing ID. - // Converted to (Process ID || Routing ID) once processed in browser. - required uint64 id = 1; + // 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; // Boolean indicating if the frame is the main frame. - required bool is_main_frame = 2; + required bool is_main_frame = 3; // The file path to the serialized Skia Picture. - optional string file_path = 3; + optional string file_path = 4; // A list of links within the frame. - repeated LinkDataProto links = 4; + repeated LinkDataProto links = 5; - // A map between the content IDs of subframes and the |id| field. - map<uint32, int64> content_id_proxy_id_map = 5; + // A mapping between the content IDs of subframes and the |id| field. + repeated ContentIdEmbeddingTokenPairProto content_id_to_embedding_tokens = 6; } // Metadata for the capture. diff --git a/chromium/components/paint_preview/common/serial_utils.h b/chromium/components/paint_preview/common/serial_utils.h index 28e43808252..6f78a12893e 100644 --- a/chromium/components/paint_preview/common/serial_utils.h +++ b/chromium/components/paint_preview/common/serial_utils.h @@ -10,6 +10,7 @@ #include "base/containers/flat_map.h" #include "base/containers/flat_set.h" #include "base/files/file.h" +#include "base/unguessable_token.h" #include "components/paint_preview/common/glyph_usage.h" #include "third_party/skia/include/core/SkPicture.h" #include "third_party/skia/include/core/SkRefCnt.h" @@ -19,8 +20,9 @@ namespace paint_preview { -// Maps a content ID to a frame ID (Process ID || Routing ID). -using PictureSerializationContext = base::flat_map<uint32_t, uint64_t>; +// Maps a content ID to an embedding token. +using PictureSerializationContext = + base::flat_map<uint32_t, base::UnguessableToken>; // Maps a typeface ID to a glyph usage tracker. using TypefaceUsageMap = base::flat_map<SkFontID, std::unique_ptr<GlyphUsage>>; diff --git a/chromium/components/paint_preview/common/serial_utils_unittest.cc b/chromium/components/paint_preview/common/serial_utils_unittest.cc index 82e3fd3bcc8..eeb59422304 100644 --- a/chromium/components/paint_preview/common/serial_utils_unittest.cc +++ b/chromium/components/paint_preview/common/serial_utils_unittest.cc @@ -23,7 +23,7 @@ TEST(PaintPreviewSerialUtils, TestMakeEmptyPicture) { TEST(PaintPreviewSerialUtils, TestPictureProcs) { auto pic = MakeEmptyPicture(); uint32_t content_id = pic->uniqueID(); - const uint64_t kFrameGuid = 2; + const base::UnguessableToken kFrameGuid = base::UnguessableToken::Create(); PictureSerializationContext picture_ctx; EXPECT_TRUE( picture_ctx.insert(std::make_pair(content_id, kFrameGuid)).second); diff --git a/chromium/components/paint_preview/common/subset_font.cc b/chromium/components/paint_preview/common/subset_font.cc index 203709f5b17..ba539170823 100644 --- a/chromium/components/paint_preview/common/subset_font.cc +++ b/chromium/components/paint_preview/common/subset_font.cc @@ -4,14 +4,17 @@ #include "components/paint_preview/common/subset_font.h" +// clang-format off +#include <hb.h> +#include <hb-subset.h> +// clang-format on + #include <memory> #include <utility> -#include <hb-subset.h> -#include <hb.h> - #include "base/bind.h" #include "base/callback.h" +#include "third_party/harfbuzz-ng/utils/hb_scoped.h" #include "third_party/skia/include/core/SkStream.h" #include "third_party/skia/include/core/SkTypeface.h" @@ -19,26 +22,6 @@ namespace paint_preview { namespace { -// Handles auto-deletion of harfbuzz objects. -template <typename T, T* P> -struct HbDeleter { - template <typename... Args> - auto operator()(Args&&... args) const - -> decltype(P(std::forward<Args>(args)...)) { - return P(std::forward<Args>(args)...); - } -}; - -using HbBlob = - std::unique_ptr<hb_blob_t, - HbDeleter<decltype(hb_blob_destroy), &hb_blob_destroy>>; -using HbFace = - std::unique_ptr<hb_face_t, - HbDeleter<decltype(hb_face_destroy), &hb_face_destroy>>; -using HbSubsetInput = std::unique_ptr< - hb_subset_input_t, - HbDeleter<decltype(hb_subset_input_destroy), &hb_subset_input_destroy>>; - // Converts and SkStream to an SkData object without copy if possible or // falls back to a copy. sk_sp<SkData> StreamToData(std::unique_ptr<SkStreamAsset> stream) { @@ -59,13 +42,14 @@ sk_sp<SkData> StreamToData(std::unique_ptr<SkStreamAsset> stream) { } // Converts SkData to a hb_blob_t. -HbBlob MakeBlob(sk_sp<SkData> data) { +HbScoped<hb_blob_t> MakeBlob(sk_sp<SkData> data) { if (!data || !base::IsValueInRangeForNumericType<unsigned int, size_t>(data->size())) return nullptr; - return HbBlob(hb_blob_create(static_cast<const char*>(data->data()), - static_cast<unsigned int>(data->size()), - HB_MEMORY_MODE_READONLY, nullptr, nullptr)); + return HbScoped<hb_blob_t>( + hb_blob_create(static_cast<const char*>(data->data()), + static_cast<unsigned int>(data->size()), + HB_MEMORY_MODE_READONLY, nullptr, nullptr)); } // Adds |glyph_id| to the set of glyphs to be retained. @@ -79,8 +63,8 @@ void AddGlyphs(hb_set_t* glyph_id_set, uint16_t glyph_id) { sk_sp<SkData> SubsetFont(SkTypeface* typeface, const GlyphUsage& usage) { int ttc_index = 0; sk_sp<SkData> data = StreamToData(typeface->openStream(&ttc_index)); - HbFace face(hb_face_create(MakeBlob(data).get(), ttc_index)); - HbSubsetInput input(hb_subset_input_create_or_fail()); + HbScoped<hb_face_t> face(hb_face_create(MakeBlob(data).get(), ttc_index)); + HbScoped<hb_subset_input_t> input(hb_subset_input_create_or_fail()); if (!face || !input) return nullptr; @@ -89,8 +73,8 @@ sk_sp<SkData> SubsetFont(SkTypeface* typeface, const GlyphUsage& usage) { usage.ForEach(base::BindRepeating(&AddGlyphs, base::Unretained(glyphs))); hb_subset_input_set_retain_gids(input.get(), true); - HbFace subset_face(hb_subset(face.get(), input.get())); - HbBlob subset_blob(hb_face_reference_blob(subset_face.get())); + HbScoped<hb_face_t> subset_face(hb_subset(face.get(), input.get())); + HbScoped<hb_blob_t> subset_blob(hb_face_reference_blob(subset_face.get())); if (!subset_blob) return nullptr; diff --git a/chromium/components/paint_preview/features/BUILD.gn b/chromium/components/paint_preview/features/BUILD.gn new file mode 100644 index 00000000000..f7b1c1332f8 --- /dev/null +++ b/chromium/components/paint_preview/features/BUILD.gn @@ -0,0 +1,11 @@ +# 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. + +source_set("features") { + sources = [ + "features.cc", + "features.h", + ] + deps = [ "//base" ] +} diff --git a/chromium/components/paint_preview/features/features.cc b/chromium/components/paint_preview/features/features.cc new file mode 100644 index 00000000000..494778d96d9 --- /dev/null +++ b/chromium/components/paint_preview/features/features.cc @@ -0,0 +1,16 @@ +// 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/features/features.h" + +#include "base/feature_list.h" + +namespace paint_preview { + +const base::Feature kPaintPreviewCaptureExperiment{ + "PaintPreviewCaptureExperiment", base::FEATURE_DISABLED_BY_DEFAULT}; +const base::Feature kPaintPreviewDemo{"PaintPreviewDemo", + base::FEATURE_DISABLED_BY_DEFAULT}; + +} // namespace paint_preview diff --git a/chromium/components/paint_preview/features/features.h b/chromium/components/paint_preview/features/features.h new file mode 100644 index 00000000000..f9d3f4748f9 --- /dev/null +++ b/chromium/components/paint_preview/features/features.h @@ -0,0 +1,27 @@ +// 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_FEATURES_FEATURES_H_ +#define COMPONENTS_PAINT_PREVIEW_FEATURES_FEATURES_H_ + +#include "base/feature_list.h" + +namespace paint_preview { + +// Used to enable the paint preview capture experiment on Android. If enabled, +// paint preview capture will be triggered for a fraction of page loads, with +// accordance to a probability threshold that is set by a field trial param. +// Metrics for the capture are logged and the resulting paint preview is then +// deleted. +extern const base::Feature kPaintPreviewCaptureExperiment; + +// Used to enable a main menu item on Android that captures and displays a paint +// preview for the current page. The paint preview UI will be dismissed on back +// press and all associated stored files deleted. This intended to test whether +// capturing and playing paint preview works on a specific site. +extern const base::Feature kPaintPreviewDemo; + +} // namespace paint_preview + +#endif // COMPONENTS_PAINT_PREVIEW_FEATURES_FEATURES_H_ diff --git a/chromium/components/paint_preview/player/BUILD.gn b/chromium/components/paint_preview/player/BUILD.gn index b29390dbff8..052be10efe7 100644 --- a/chromium/components/paint_preview/player/BUILD.gn +++ b/chromium/components/paint_preview/player/BUILD.gn @@ -2,8 +2,6 @@ # Use of this source code is governed by a BSD - style license that can be # found in the LICENSE file. -import("//build/config/android/rules.gni") - source_set("player") { sources = [ "player_compositor_delegate.cc", @@ -13,12 +11,31 @@ source_set("player") { deps = [ "//base", "//components/paint_preview/browser", - "//components/services/paint_preview_compositor", + "//components/paint_preview/common", + "//components/paint_preview/common/proto", + "//components/paint_preview/public", + "//mojo/public/cpp/bindings", "//ui/gfx/geometry", "//url", ] - public_deps = [ - "//components/services/paint_preview_compositor/public/mojom", + public_deps = + [ "//components/services/paint_preview_compositor/public/mojom" ] +} + +source_set("unit_tests") { + testonly = true + + sources = [ "player_compositor_delegate_unittest.cc" ] + + deps = [ + ":player", + "//base", + "//base/test:test_support", + "//components/paint_preview/browser", + "//components/paint_preview/common", + "//components/paint_preview/common/proto", + "//ui/gfx/geometry", + "//url", ] } diff --git a/chromium/components/paint_preview/player/android/BUILD.gn b/chromium/components/paint_preview/player/android/BUILD.gn index e9c958433c1..447498f0c4b 100644 --- a/chromium/components/paint_preview/player/android/BUILD.gn +++ b/chromium/components/paint_preview/player/android/BUILD.gn @@ -4,7 +4,7 @@ import("//build/config/android/rules.gni") -source_set("player_android") { +source_set("android") { sources = [ "player_compositor_delegate_android.cc", "player_compositor_delegate_android.h", @@ -15,25 +15,24 @@ source_set("player_android") { "//base", "//components/paint_preview/browser", "//components/paint_preview/player", - "//components/services/paint_preview_compositor", "//ui/gfx", "//url", ] - public_deps = [ - "//components/services/paint_preview_compositor/public/mojom", - ] + public_deps = + [ "//components/services/paint_preview_compositor/public/mojom" ] } source_set("unit_tests") { testonly = true - sources = [ - "player_compositor_delegate_android_unittest.cc", - ] + sources = [ "player_compositor_delegate_android_unittest.cc" ] deps = [ - ":player_android", + ":android", "//base", "//base/test:test_support", + "//components/paint_preview/browser", + "//components/paint_preview/player", + "//components/services/paint_preview_compositor/public/mojom", "//skia", "//testing/gmock", "//testing/gtest", @@ -41,15 +40,14 @@ source_set("unit_tests") { } generate_jni("jni_headers") { - sources = [ - "java/src/org/chromium/components/paintpreview/player/PlayerCompositorDelegateImpl.java", - ] + sources = [ "java/src/org/chromium/components/paintpreview/player/PlayerCompositorDelegateImpl.java" ] } android_library("java") { annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ] - java_files = [ + sources = [ + "java/src/org/chromium/components/paintpreview/player/LinkClickHandler.java", "java/src/org/chromium/components/paintpreview/player/PaintPreviewFrame.java", "java/src/org/chromium/components/paintpreview/player/PlayerCompositorDelegate.java", "java/src/org/chromium/components/paintpreview/player/PlayerCompositorDelegateImpl.java", @@ -67,12 +65,83 @@ android_library("java") { deps = [ "//base:base_java", "//base:jni_java", + "//components/paint_preview/browser/android:java", "//ui/android:ui_java", + "//url:gurl_java", + ] +} + +android_library("player_java_test_support") { + testonly = true + annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ] + + sources = [ + "javatests/src/org/chromium/components/paintpreview/player/PaintPreviewTestRule.java", + "javatests/src/org/chromium/components/paintpreview/player/PaintPreviewTestService.java", + ] + + deps = [ + ":java", + "//base:base_java", + "//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", + "//content/public/android:content_java", + "//content/public/test/android:content_java_test_support", + "//third_party/android_support_test_runner:rules_java", + "//third_party/android_support_test_runner:runner_java", + "//third_party/junit", + "//url:gurl_java", + ] +} + +android_library("javatests") { + testonly = true + + sources = [ "javatests/src/org/chromium/components/paintpreview/player/PaintPreviewPlayerTest.java" ] + deps = [ + ":java", + ":player_java_test_support", + "//base:base_java", + "//base:base_java_test_support", + "//content/public/android:content_java", + "//content/public/test/android:content_java_test_support", + "//third_party/android_support_test_runner:rules_java", + "//third_party/android_support_test_runner:runner_java", + "//third_party/junit", + "//third_party/ub-uiautomator:ub_uiautomator_java", + "//ui/android:ui_java_test_support", + "//url:gurl_java", + ] +} + +generate_jni("javatests_jni_headers") { + testonly = true + + sources = [ "javatests/src/org/chromium/components/paintpreview/player/PaintPreviewTestService.java" ] +} + +source_set("test_support") { + testonly = true + + sources = [ + "javatests/paint_preview_test_service.cc", + "javatests/paint_preview_test_service.h", + ] + + deps = [ + ":android", + ":javatests_jni_headers", + "//base", + "//components/paint_preview/browser", + "//components/paint_preview/browser:test_support", ] } junit_binary("paint_preview_junit_tests") { - java_files = [ + sources = [ "junit/src/org/chromium/components/paintpreview/player/PlayerManagerTest.java", "junit/src/org/chromium/components/paintpreview/player/frame/PlayerFrameBitmapPainterTest.java", "junit/src/org/chromium/components/paintpreview/player/frame/PlayerFrameMediatorTest.java", diff --git a/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/LinkClickHandler.java b/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/LinkClickHandler.java new file mode 100644 index 00000000000..4000d9fe449 --- /dev/null +++ b/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/LinkClickHandler.java @@ -0,0 +1,12 @@ +// 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. + +package org.chromium.components.paintpreview.player; + +import org.chromium.url.GURL; + +/** + * Interface for processing link click events from the player's hit tests. + */ +public interface LinkClickHandler { void onLinkClicked(GURL url); } diff --git a/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/PaintPreviewFrame.java b/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/PaintPreviewFrame.java index f995f7baa09..383e5d8270b 100644 --- a/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/PaintPreviewFrame.java +++ b/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/PaintPreviewFrame.java @@ -8,6 +8,8 @@ import android.graphics.Rect; import androidx.annotation.VisibleForTesting; +import org.chromium.base.UnguessableToken; + import java.util.Arrays; /** @@ -17,7 +19,7 @@ import java.util.Arrays; * Optionally, a frame can have other frames (iframes) as its children. or sub-frames. */ class PaintPreviewFrame { - private long mGuid; + private UnguessableToken mGuid; // The content size of this frame. In native, this is represented as 'scroll extent'. private int mContentWidth; private int mContentHeight; @@ -26,13 +28,13 @@ class PaintPreviewFrame { // The coordinates of the sub-frames relative to this frame. private Rect[] mSubFrameClips; - PaintPreviewFrame(long guid, int contentWidth, int contentHeight) { + PaintPreviewFrame(UnguessableToken guid, int contentWidth, int contentHeight) { mGuid = guid; mContentWidth = contentWidth; mContentHeight = contentHeight; } - private PaintPreviewFrame(long guid, int contentWidth, int contentHeight, + private PaintPreviewFrame(UnguessableToken guid, int contentWidth, int contentHeight, PaintPreviewFrame[] subFrames, Rect[] subFrameClips) { mGuid = guid; mContentWidth = contentWidth; @@ -49,7 +51,7 @@ class PaintPreviewFrame { mSubFrameClips = subFrameClips; } - long getGuid() { + UnguessableToken getGuid() { return mGuid; } @@ -74,7 +76,7 @@ class PaintPreviewFrame { if (obj == null || getClass() != obj.getClass()) return false; PaintPreviewFrame other = (PaintPreviewFrame) obj; - if (this.mGuid != other.mGuid) return false; + if (!this.mGuid.equals(other.mGuid)) return false; if (this.mContentHeight != other.mContentHeight) return false; @@ -104,8 +106,8 @@ class PaintPreviewFrame { } @VisibleForTesting - static PaintPreviewFrame createInstanceForTest(long guid, int contentWidth, int contentHeight, - PaintPreviewFrame[] subFrames, Rect[] subFrameClips) { + static PaintPreviewFrame createInstanceForTest(UnguessableToken guid, int contentWidth, + int contentHeight, PaintPreviewFrame[] subFrames, Rect[] subFrameClips) { return new PaintPreviewFrame(guid, contentWidth, contentHeight, subFrames, subFrameClips); } } diff --git a/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/PlayerCompositorDelegate.java b/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/PlayerCompositorDelegate.java index 3ed7374b87c..d12d2943e54 100644 --- a/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/PlayerCompositorDelegate.java +++ b/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/PlayerCompositorDelegate.java @@ -5,10 +5,10 @@ package org.chromium.components.paintpreview.player; import android.graphics.Bitmap; -import android.graphics.Point; import android.graphics.Rect; import org.chromium.base.Callback; +import org.chromium.base.UnguessableToken; /** * Used for communicating with the Paint Preview delegate for requesting new bitmaps and forwarding @@ -24,13 +24,14 @@ public interface PlayerCompositorDelegate { * if there are any errors. * @param errorCallback Gets notified if there are any errors. Won't get called otherwise. */ - void requestBitmap(long frameGuid, Rect clipRect, float scaleFactor, + void requestBitmap(UnguessableToken frameGuid, Rect clipRect, float scaleFactor, Callback<Bitmap> bitmapCallback, Runnable errorCallback); /** * Sends a click event for a frame to native for link hit testing. * @param frameGuid The GUID of the frame. - * @param point The coordinates of the click event, relative to the frame. + * @param x The x coordinate of the click event, relative to the frame. + * @param y The y coordinate of the click event, relative to the frame. */ - void onClick(long frameGuid, Point point); + void onClick(UnguessableToken frameGuid, int x, int y); } diff --git a/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/PlayerCompositorDelegateImpl.java b/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/PlayerCompositorDelegateImpl.java index 9ff36fee8da..9e3cbe51b7a 100644 --- a/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/PlayerCompositorDelegateImpl.java +++ b/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/PlayerCompositorDelegateImpl.java @@ -5,13 +5,15 @@ package org.chromium.components.paintpreview.player; import android.graphics.Bitmap; -import android.graphics.Point; import android.graphics.Rect; import org.chromium.base.Callback; +import org.chromium.base.UnguessableToken; import org.chromium.base.annotations.CalledByNative; import org.chromium.base.annotations.JNINamespace; import org.chromium.base.annotations.NativeMethods; +import org.chromium.components.paintpreview.browser.NativePaintPreviewServiceProvider; +import org.chromium.url.GURL; import javax.annotation.Nonnull; @@ -22,30 +24,42 @@ import javax.annotation.Nonnull; @JNINamespace("paint_preview") class PlayerCompositorDelegateImpl implements PlayerCompositorDelegate { interface CompositorListener { - void onCompositorReady(long rootFrameGuid, long[] frameGuids, int[] frameContentSize, - int[] subFramesCount, long[] subFrameGuids, int[] subFrameClipRects); + void onCompositorReady(boolean safeToShow, UnguessableToken rootFrameGuid, + UnguessableToken[] frameGuids, int[] frameContentSize, int[] subFramesCount, + UnguessableToken[] subFrameGuids, int[] subFrameClipRects); } private CompositorListener mCompositorListener; - private long mNativePaintPreviewPlayerMediator; + private LinkClickHandler mLinkClickHandler; + private long mNativePlayerCompositorDelegate; - PlayerCompositorDelegateImpl(String url, @Nonnull CompositorListener compositorListener) { + PlayerCompositorDelegateImpl(NativePaintPreviewServiceProvider service, GURL url, + String directoryKey, @Nonnull CompositorListener compositorListener, + @Nonnull LinkClickHandler linkClickHandler) { mCompositorListener = compositorListener; - mNativePaintPreviewPlayerMediator = - PlayerCompositorDelegateImplJni.get().initialize(this, url); + mLinkClickHandler = linkClickHandler; + if (service != null && service.getNativeService() != 0) { + mNativePlayerCompositorDelegate = PlayerCompositorDelegateImplJni.get().initialize( + this, service.getNativeService(), url.getSpec(), directoryKey); + } + // TODO(crbug.com/1021590): Handle initialization errors when + // mNativePlayerCompositorDelegate == 0. } /** * Called by native when the Paint Preview compositor is ready. + * + * @param safeToShow true if the native initialization of the Paint Preview player succeeded and + * the captured result matched the expected URL. * @param rootFrameGuid The GUID for the root frame. * @param frameGuids Contains all frame GUIDs that are in this hierarchy. * @param frameContentSize Contains the content size for each frame. In native, this is called * scroll extent. The order corresponds to {@code frameGuids}. The content width and height for * the ith frame in {@code frameGuids} are respectively in the {@code 2*i} and {@code 2*i+1} * indices of {@code frameContentSize}. - * @param subFramesCount Contains the number of sub-frames for each frame. The order - * corresponds to {@code frameGuids}. The number of sub-frames for the {@code i}th frame in - * {@code frameGuids} is {@code subFramesCount[i]}. + * @param subFramesCount Contains the number of sub-frames for each frame. The order corresponds + * to {@code frameGuids}. The number of sub-frames for the {@code i}th frame in {@code + * frameGuids} is {@code subFramesCount[i]}. * @param subFrameGuids Contains the GUIDs of all sub-frames. The GUID for the {@code j}th * sub-frame of {@code frameGuids[i]} will be at {@code subFrameGuids[k]}, where {@code k} is: * <pre> @@ -53,50 +67,64 @@ class PlayerCompositorDelegateImpl implements PlayerCompositorDelegate { * for (int s = 0; s < i; s++) k += subFramesCount[s]; * </pre> * @param subFrameClipRects Contains clip rect values for each sub-frame. Each clip rect value - * comes in a series of four consecutive integers that represent x, y, width, and height. - * The clip rect values for the {@code j}th sub-frame of {@code frameGuids[i]} will be at - * {@code subFrameGuids[4*k]}, {@code subFrameGuids[4*k+1]} , {@code subFrameGuids[4*k+2]}, - * and {@code subFrameGuids[4*k+3]}, where {@code k} has the same value as above. + * comes in a series of four consecutive integers that represent x, y, width, and height. The + * clip rect values for the {@code j}th sub-frame of {@code frameGuids[i]} will be at {@code + * subFrameGuids[4*k]}, {@code subFrameGuids[4*k+1]} , {@code subFrameGuids[4*k+2]}, and {@code + * subFrameGuids[4*k+3]}, where {@code k} has the same value as above. */ @CalledByNative - void onCompositorReady(long rootFrameGuid, long[] frameGuids, int[] frameContentSize, - int[] subFramesCount, long[] subFrameGuids, int[] subFrameClipRects) { - mCompositorListener.onCompositorReady(rootFrameGuid, frameGuids, frameContentSize, - subFramesCount, subFrameGuids, subFrameClipRects); + void onCompositorReady(boolean safeToShow, UnguessableToken rootFrameGuid, + UnguessableToken[] frameGuids, int[] frameContentSize, int[] subFramesCount, + UnguessableToken[] subFrameGuids, int[] subFrameClipRects) { + mCompositorListener.onCompositorReady(safeToShow, rootFrameGuid, frameGuids, + frameContentSize, subFramesCount, subFrameGuids, subFrameClipRects); } @Override - public void requestBitmap(long frameGuid, Rect clipRect, float scaleFactor, + public void requestBitmap(UnguessableToken frameGuid, Rect clipRect, float scaleFactor, Callback<Bitmap> bitmapCallback, Runnable errorCallback) { - if (mNativePaintPreviewPlayerMediator == 0) return; + if (mNativePlayerCompositorDelegate == 0) { + return; + } - PlayerCompositorDelegateImplJni.get().requestBitmap(mNativePaintPreviewPlayerMediator, + PlayerCompositorDelegateImplJni.get().requestBitmap(mNativePlayerCompositorDelegate, frameGuid, bitmapCallback, errorCallback, scaleFactor, clipRect.left, clipRect.top, clipRect.width(), clipRect.height()); } @Override - public void onClick(long frameGuid, Point point) { - if (mNativePaintPreviewPlayerMediator == 0) return; + public void onClick(UnguessableToken frameGuid, int x, int y) { + if (mNativePlayerCompositorDelegate == 0) { + return; + } PlayerCompositorDelegateImplJni.get().onClick( - mNativePaintPreviewPlayerMediator, frameGuid, point.x, point.y); + mNativePlayerCompositorDelegate, frameGuid, x, y); + } + + @CalledByNative + public void onLinkClicked(String url) { + mLinkClickHandler.onLinkClicked(new GURL(url)); } void destroy() { - if (mNativePaintPreviewPlayerMediator == 0) return; + if (mNativePlayerCompositorDelegate == 0) { + return; + } - PlayerCompositorDelegateImplJni.get().destroy(mNativePaintPreviewPlayerMediator); - mNativePaintPreviewPlayerMediator = 0; + PlayerCompositorDelegateImplJni.get().destroy(mNativePlayerCompositorDelegate); + mNativePlayerCompositorDelegate = 0; } @NativeMethods interface Natives { - long initialize(PlayerCompositorDelegateImpl caller, String url); + long initialize(PlayerCompositorDelegateImpl caller, long nativePaintPreviewBaseService, + String urlSpec, String directoryKey); void destroy(long nativePlayerCompositorDelegateAndroid); - void requestBitmap(long nativePlayerCompositorDelegateAndroid, long frameGuid, + void requestBitmap(long nativePlayerCompositorDelegateAndroid, UnguessableToken frameGuid, Callback<Bitmap> bitmapCallback, Runnable errorCallback, float scaleFactor, int clipX, int clipY, int clipWidth, int clipHeight); - void onClick(long nativePlayerCompositorDelegateAndroid, long frameGuid, int x, int y); + void onClick(long nativePlayerCompositorDelegateAndroid, UnguessableToken frameGuid, int x, + int y); } -}
\ No newline at end of file +} diff --git a/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/PlayerManager.java b/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/PlayerManager.java index 5f1474f45f4..e8b99036344 100644 --- a/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/PlayerManager.java +++ b/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/PlayerManager.java @@ -12,11 +12,17 @@ import android.widget.FrameLayout; import androidx.annotation.VisibleForTesting; +import org.chromium.base.Callback; +import org.chromium.base.UnguessableToken; +import org.chromium.components.paintpreview.browser.NativePaintPreviewServiceProvider; import org.chromium.components.paintpreview.player.frame.PlayerFrameCoordinator; +import org.chromium.url.GURL; import java.util.HashMap; import java.util.Map; +import javax.annotation.Nonnull; + /** * This is the only public class in this package and is hence the access point of this component for * the outer world. Users should call {@link #destroy()} to ensure the native part is destroyed. @@ -26,11 +32,17 @@ public class PlayerManager { private PlayerCompositorDelegateImpl mDelegate; private PlayerFrameCoordinator mRootFrameCoordinator; private FrameLayout mHostView; + private Callback<Boolean> mViewReadyCallback; - public PlayerManager(Context context, String url) { + public PlayerManager(GURL url, Context context, + NativePaintPreviewServiceProvider nativePaintPreviewServiceProvider, + String directoryKey, @Nonnull LinkClickHandler linkClickHandler, + Callback<Boolean> viewReadyCallback) { mContext = context; - mDelegate = new PlayerCompositorDelegateImpl(url, this::onCompositorReady); + mDelegate = new PlayerCompositorDelegateImpl(nativePaintPreviewServiceProvider, url, + directoryKey, this::onCompositorReady, linkClickHandler); mHostView = new FrameLayout(mContext); + mViewReadyCallback = viewReadyCallback; } /** @@ -38,8 +50,14 @@ public class PlayerManager { * method initializes a sub-component for each frame and adds the view for the root frame to * {@link #mHostView}. */ - private void onCompositorReady(long rootFrameGuid, long[] frameGuids, int[] frameContentSize, - int[] subFramesCount, long[] subFrameGuids, int[] subFrameClipRects) { + private void onCompositorReady(boolean safeToShow, UnguessableToken rootFrameGuid, + UnguessableToken[] frameGuids, int[] frameContentSize, int[] subFramesCount, + UnguessableToken[] subFrameGuids, int[] subFrameClipRects) { + if (!safeToShow) { + mViewReadyCallback.onResult(safeToShow); + return; + } + PaintPreviewFrame rootFrame = buildFrameTreeHierarchy(rootFrameGuid, frameGuids, frameContentSize, subFramesCount, subFrameGuids, subFrameClipRects); @@ -49,19 +67,21 @@ public class PlayerManager { mHostView.addView(mRootFrameCoordinator.getView(), new FrameLayout.LayoutParams( ViewGroup.LayoutParams.MATCH_PARENT, ViewGroup.LayoutParams.MATCH_PARENT)); + mViewReadyCallback.onResult(safeToShow); } /** - * This method builds a hierarchy of {@link PaintPreviewFrame}s from primitive variables - * that originate from native. Detailed explanation of the parameters can be found in - * {@link PlayerCompositorDelegateImpl#onCompositorReady}. + * This method builds a hierarchy of {@link PaintPreviewFrame}s from primitive variables that + * originate from native. Detailed explanation of the parameters can be found in {@link + * PlayerCompositorDelegateImpl#onCompositorReady}. + * * @return The root {@link PaintPreviewFrame} */ @VisibleForTesting - static PaintPreviewFrame buildFrameTreeHierarchy(long rootFrameGuid, long[] frameGuids, - int[] frameContentSize, int[] subFramesCount, long[] subFrameGuids, - int[] subFrameClipRects) { - Map<Long, PaintPreviewFrame> framesMap = new HashMap<>(); + static PaintPreviewFrame buildFrameTreeHierarchy(UnguessableToken rootFrameGuid, + UnguessableToken[] frameGuids, int[] frameContentSize, int[] subFramesCount, + UnguessableToken[] subFrameGuids, int[] subFrameClipRects) { + Map<UnguessableToken, PaintPreviewFrame> framesMap = new HashMap<>(); for (int i = 0; i < frameGuids.length; i++) { framesMap.put(frameGuids[i], new PaintPreviewFrame( @@ -95,7 +115,9 @@ public class PlayerManager { */ private void buildSubFrameCoordinators( PlayerFrameCoordinator frameCoordinator, PaintPreviewFrame frame) { - if (frame.getSubFrames() == null || frame.getSubFrames().length == 0) return; + if (frame.getSubFrames() == null || frame.getSubFrames().length == 0) { + return; + } for (int i = 0; i < frame.getSubFrames().length; i++) { PaintPreviewFrame childFrame = frame.getSubFrames()[i]; @@ -117,4 +139,4 @@ public class PlayerManager { public View getView() { return mHostView; } -}
\ No newline at end of file +} diff --git a/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameBitmapPainter.java b/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameBitmapPainter.java index 6f8ee4a080a..3eb04e41f96 100644 --- a/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameBitmapPainter.java +++ b/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameBitmapPainter.java @@ -60,6 +60,9 @@ class PlayerFrameBitmapPainter { for (int row = rowStart; row < rowEnd; row++) { for (int col = colStart; col < colEnd; col++) { Bitmap tileBitmap = mBitmapMatrix[row][col]; + if (tileBitmap == null) { + continue; + } // Calculate the portion of this tileBitmap that is visible in mViewPort. int bitmapLeft = Math.max(mViewPort.left - (col * tileWidth), 0); diff --git a/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameCoordinator.java b/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameCoordinator.java index 9346ba7b74f..81b6ab2714e 100644 --- a/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameCoordinator.java +++ b/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameCoordinator.java @@ -8,6 +8,7 @@ import android.content.Context; import android.graphics.Rect; import android.view.View; +import org.chromium.base.UnguessableToken; import org.chromium.components.paintpreview.player.PlayerCompositorDelegate; import org.chromium.ui.modelutil.PropertyModel; import org.chromium.ui.modelutil.PropertyModelChangeProcessor; @@ -24,7 +25,8 @@ public class PlayerFrameCoordinator { * binds them together. */ public PlayerFrameCoordinator(Context context, PlayerCompositorDelegate compositorDelegate, - long frameGuid, int contentWidth, int contentHeight, boolean canDetectZoom) { + UnguessableToken frameGuid, int contentWidth, int contentHeight, + boolean canDetectZoom) { PropertyModel model = new PropertyModel.Builder(PlayerFrameProperties.ALL_KEYS).build(); mMediator = new PlayerFrameMediator( model, compositorDelegate, frameGuid, contentWidth, contentHeight); diff --git a/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameGestureDetector.java b/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameGestureDetector.java index 88f52190691..e520579db45 100644 --- a/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameGestureDetector.java +++ b/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameGestureDetector.java @@ -49,7 +49,7 @@ class PlayerFrameGestureDetector @Override public boolean onDown(MotionEvent e) { - return false; + return true; } @Override diff --git a/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameMediator.java b/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameMediator.java index 75bfb81fea5..06f565f0138 100644 --- a/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameMediator.java +++ b/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameMediator.java @@ -5,12 +5,12 @@ package org.chromium.components.paintpreview.player.frame; import android.graphics.Bitmap; -import android.graphics.Point; import android.graphics.Rect; import android.util.Pair; import android.view.View; import org.chromium.base.Callback; +import org.chromium.base.UnguessableToken; import org.chromium.components.paintpreview.player.PlayerCompositorDelegate; import org.chromium.ui.modelutil.PropertyModel; @@ -35,7 +35,7 @@ import java.util.Map; */ class PlayerFrameMediator implements PlayerFrameViewDelegate { /** The GUID associated with the frame that this class is representing. */ - private final long mGuid; + private final UnguessableToken mGuid; /** The content width inside this frame, at a scale factor of 1. */ private final int mContentWidth; /** The content height inside this frame, at a scale factor of 1. */ @@ -62,7 +62,7 @@ class PlayerFrameMediator implements PlayerFrameViewDelegate { private float mScaleFactor; PlayerFrameMediator(PropertyModel model, PlayerCompositorDelegate compositorDelegate, - long frameGuid, int contentWidth, int contentHeight) { + UnguessableToken frameGuid, int contentWidth, int contentHeight) { mModel = model; mCompositorDelegate = compositorDelegate; mGuid = frameGuid; @@ -199,7 +199,7 @@ class PlayerFrameMediator implements PlayerFrameViewDelegate { @Override public void onClick(int x, int y) { - mCompositorDelegate.onClick(mGuid, new Point(x, y)); + mCompositorDelegate.onClick(mGuid, mViewportRect.left + x, mViewportRect.top + y); } /** diff --git a/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameProperties.java b/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameProperties.java index 094f1821e8c..94d58f50b27 100644 --- a/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameProperties.java +++ b/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameProperties.java @@ -20,19 +20,19 @@ import java.util.List; class PlayerFrameProperties { /** A matrix of bitmap tiles that collectively make the entire content. */ static final PropertyModel.WritableObjectPropertyKey<Bitmap[][]> BITMAP_MATRIX = - new PropertyModel.WritableObjectPropertyKey<>(); + new PropertyModel.WritableObjectPropertyKey<>(true); /** * Contains the current user-visible content window. The view should use this to draw the * appropriate bitmap tiles from {@link #BITMAP_MATRIX}. */ static final PropertyModel.WritableObjectPropertyKey<Rect> VIEWPORT = - new PropertyModel.WritableObjectPropertyKey<>(); + new PropertyModel.WritableObjectPropertyKey<>(true); /** * A list of sub-frames that are currently visible. Each element in the list is a {@link Pair} * consists of a {@link View}, that displays the sub-frame's content, and a {@link Rect}, that * contains its location. */ static final PropertyModel.WritableObjectPropertyKey<List<Pair<View, Rect>>> SUBFRAME_VIEWS = - new PropertyModel.WritableObjectPropertyKey<>(); + new PropertyModel.WritableObjectPropertyKey<>(true); static final PropertyKey[] ALL_KEYS = {BITMAP_MATRIX, VIEWPORT, SUBFRAME_VIEWS}; } diff --git a/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameView.java b/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameView.java index c3b08768252..03cf279e8a7 100644 --- a/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameView.java +++ b/chromium/components/paint_preview/player/android/java/src/org/chromium/components/paintpreview/player/frame/PlayerFrameView.java @@ -8,12 +8,13 @@ import android.content.Context; import android.graphics.Bitmap; import android.graphics.Canvas; import android.graphics.Rect; -import android.support.annotation.NonNull; import android.util.Pair; import android.view.MotionEvent; import android.view.View; import android.widget.FrameLayout; +import androidx.annotation.NonNull; + import java.util.List; /** @@ -36,6 +37,7 @@ class PlayerFrameView extends FrameLayout { PlayerFrameView(@NonNull Context context, boolean canDetectZoom, PlayerFrameViewDelegate playerFrameViewDelegate) { super(context); + setWillNotDraw(false); mDelegate = playerFrameViewDelegate; mBitmapPainter = new PlayerFrameBitmapPainter(this::invalidate); mGestureDetector = diff --git a/chromium/components/paint_preview/player/android/javatests/DEPS b/chromium/components/paint_preview/player/android/javatests/DEPS new file mode 100644 index 00000000000..7158b3c0703 --- /dev/null +++ b/chromium/components/paint_preview/player/android/javatests/DEPS @@ -0,0 +1,5 @@ +include_rules = [ + "+components/signin/core/browser", + "+content/public/android", + "+content/public/test/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 new file mode 100644 index 00000000000..965ee46e8e2 --- /dev/null +++ b/chromium/components/paint_preview/player/android/javatests/paint_preview_test_service.cc @@ -0,0 +1,81 @@ +// 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/player/android/javatests/paint_preview_test_service.h" + +#include <memory> + +#include "base/android/jni_android.h" +#include "base/android/jni_string.h" +#include "base/android/scoped_java_ref.h" +#include "base/files/file_path.h" +#include "components/paint_preview/browser/paint_preview_base_service.h" +#include "components/paint_preview/browser/test_paint_preview_policy.h" +#include "components/paint_preview/common/proto/paint_preview.pb.h" +#include "components/paint_preview/player/android/javatests_jni_headers/PaintPreviewTestService_jni.h" + +using base::android::JavaParamRef; + +namespace paint_preview { + +namespace { +const char kPaintPreviewDir[] = "paint_preview"; +const char kTestDirName[] = "PaintPreviewTestService"; + +void UpdateSkpPaths(const base::FilePath& test_data_dir, + const DirectoryKey& key, + PaintPreviewBaseService::OnReadProtoCallback callback, + std::unique_ptr<PaintPreviewProto> proto) { + // Update the file path for the root SKP to match the isolated test + // environment. + std::string root_skp_file_name = + base::FilePath(proto->root_frame().file_path()).BaseName().AsUTF8Unsafe(); + base::FilePath root_skp_file_path = + test_data_dir.AppendASCII(key.AsciiDirname()) + .AppendASCII(root_skp_file_name); + proto->mutable_root_frame()->set_file_path(root_skp_file_path.AsUTF8Unsafe()); + + // Update the file path for the subframe SKPs to match the isolated test + // environment. + for (auto& subframe : *(proto->mutable_subframes())) { + std::string subframe_skp_file_name = + base::FilePath(subframe.file_path()).BaseName().AsUTF8Unsafe(); + base::FilePath subframe_skp_file_path = + test_data_dir.AppendASCII(key.AsciiDirname()) + .AppendASCII(subframe_skp_file_name); + subframe.set_file_path(subframe_skp_file_path.AsUTF8Unsafe()); + } + std::move(callback).Run(std::move(proto)); +} +} // namespace + +jlong JNI_PaintPreviewTestService_GetInstance( + JNIEnv* env, + const JavaParamRef<jstring>& j_test_data_dir) { + base::FilePath file_path( + base::android::ConvertJavaStringToUTF8(env, j_test_data_dir)); + PaintPreviewTestService* service = new PaintPreviewTestService(file_path); + return reinterpret_cast<intptr_t>(service); +} + +PaintPreviewTestService::PaintPreviewTestService( + const base::FilePath& test_data_dir) + : PaintPreviewBaseService(test_data_dir, + kTestDirName, + std::make_unique<TestPaintPreviewPolicy>(), + false), + test_data_dir_(test_data_dir.AppendASCII(kPaintPreviewDir) + .AppendASCII(kTestDirName)) {} + +PaintPreviewTestService::~PaintPreviewTestService() = default; + +void PaintPreviewTestService::GetCapturedPaintPreviewProto( + const DirectoryKey& key, + OnReadProtoCallback on_read_proto_callback) { + PaintPreviewBaseService::GetCapturedPaintPreviewProto( + key, base::BindOnce(&UpdateSkpPaths, test_data_dir_, key, + std::move(on_read_proto_callback))); +} + +} // namespace paint_preview 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 new file mode 100644 index 00000000000..c16d30ab74d --- /dev/null +++ b/chromium/components/paint_preview/player/android/javatests/paint_preview_test_service.h @@ -0,0 +1,33 @@ +// 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_PLAYER_ANDROID_JAVATESTS_PAINT_PREVIEW_TEST_SERVICE_H_ +#define COMPONENTS_PAINT_PREVIEW_PLAYER_ANDROID_JAVATESTS_PAINT_PREVIEW_TEST_SERVICE_H_ + +#include "base/files/file_path.h" +#include "components/paint_preview/browser/paint_preview_base_service.h" +#include "components/paint_preview/common/proto/paint_preview.pb.h" + +namespace paint_preview { + +// A simple implementation of PaintPreviewBaseService used in tests. +class PaintPreviewTestService : public PaintPreviewBaseService { + public: + PaintPreviewTestService(const base::FilePath& test_data_dir); + ~PaintPreviewTestService() override; + + PaintPreviewTestService(const PaintPreviewTestService&) = delete; + PaintPreviewTestService& operator=(const PaintPreviewTestService&) = delete; + + void GetCapturedPaintPreviewProto( + const DirectoryKey& key, + OnReadProtoCallback on_read_proto_callback) override; + + private: + base::FilePath test_data_dir_; +}; + +} // namespace paint_preview + +#endif // COMPONENTS_PAINT_PREVIEW_PLAYER_ANDROID_JAVATESTS_PAINT_PREVIEW_TEST_SERVICE_H_ 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 new file mode 100644 index 00000000000..32b031435be --- /dev/null +++ b/chromium/components/paint_preview/player/android/javatests/src/org/chromium/components/paintpreview/player/PaintPreviewPlayerTest.java @@ -0,0 +1,204 @@ +// 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. + +package org.chromium.components.paintpreview.player; + +import android.graphics.Rect; +import android.os.SystemClock; +import android.support.test.InstrumentationRegistry; +import android.support.test.filters.MediumTest; +import android.support.test.uiautomator.UiDevice; +import android.view.MotionEvent; +import android.view.View; +import android.view.ViewGroup; + +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.chromium.base.task.PostTask; +import org.chromium.base.test.BaseJUnit4ClassRunner; +import org.chromium.base.test.util.DisabledTest; +import org.chromium.base.test.util.ScalableTimeout; +import org.chromium.base.test.util.UrlUtils; +import org.chromium.content_public.browser.UiThreadTaskTraits; +import org.chromium.content_public.browser.test.util.CriteriaHelper; +import org.chromium.ui.test.util.DummyUiActivityTestCase; +import org.chromium.url.GURL; + +/** + * Instrumentation tests for the Paint Preview player. + */ +@RunWith(BaseJUnit4ClassRunner.class) +public class PaintPreviewPlayerTest extends DummyUiActivityTestCase { + private static final long TIMEOUT_MS = ScalableTimeout.scaleTimeout(5000); + + private static final String TEST_DATA_DIR = "components/test/data/"; + private static final String TEST_DIRECTORY_KEY = "wikipedia"; + private static final String TEST_URL = "https://en.m.wikipedia.org/wiki/Main_Page"; + private static final String TEST_MAIN_PICTURE_LINK_URL = + "https://en.m.wikipedia.org/wiki/File:Volc%C3%A1n_Ubinas,_Arequipa,_Per%C3%BA,_2015-08-02,_DD_50.JPG"; + private static final String TEST_IN_VIEWPORT_LINK_URL = + "https://en.m.wikipedia.org/wiki/Arequipa"; + private static final String TEST_OUT_OF_VIEWPORT_LINK_URL = + "https://foundation.wikimedia.org/wiki/Privacy_policy"; + + private static final int TEST_PAGE_HEIGHT = 5019; + + @Rule + public PaintPreviewTestRule mPaintPreviewTestRule = new PaintPreviewTestRule(); + + private PlayerManager mPlayerManager; + private TestLinkClickHandler mLinkClickHandler; + + /** + * LinkClickHandler implementation for caching the last URL that was clicked. + */ + public class TestLinkClickHandler implements LinkClickHandler { + GURL mUrl; + + @Override + public void onLinkClicked(GURL url) { + mUrl = url; + } + } + + /** + * Tests the the player correctly initializes and displays a sample paint preview with 1 frame. + */ + @Test + @MediumTest + public void singleFrameDisplayTest() { + initPlayerManager(); + final View playerHostView = mPlayerManager.getView(); + final View activityContentView = getActivity().findViewById(android.R.id.content); + + // Assert that the player view has the same dimensions as the content view. + CriteriaHelper.pollUiThread( + () -> { + boolean contentSizeNonZero = activityContentView.getWidth() > 0 + && activityContentView.getHeight() > 0; + boolean viewSizeMatchContent = + activityContentView.getWidth() == playerHostView.getWidth() + && activityContentView.getHeight() == playerHostView.getHeight(); + return contentSizeNonZero && viewSizeMatchContent; + }, + "Player size doesn't match R.id.content", TIMEOUT_MS, + CriteriaHelper.DEFAULT_POLLING_INTERVAL); + } + + /** + * Tests that link clicks in the player work correctly. + */ + @Test + @MediumTest + @DisabledTest(message = "crbug.com/1065441") + public void linkClickTest() { + initPlayerManager(); + final View playerHostView = mPlayerManager.getView(); + + // Click on the top left picture and assert it directs to the correct link. + assertLinkUrl(playerHostView, 92, 424, TEST_MAIN_PICTURE_LINK_URL); + assertLinkUrl(playerHostView, 67, 527, TEST_MAIN_PICTURE_LINK_URL); + assertLinkUrl(playerHostView, 466, 668, TEST_MAIN_PICTURE_LINK_URL); + assertLinkUrl(playerHostView, 412, 432, TEST_MAIN_PICTURE_LINK_URL); + + // Click on a link that is visible in the default viewport. + assertLinkUrl(playerHostView, 732, 698, TEST_IN_VIEWPORT_LINK_URL); + assertLinkUrl(playerHostView, 876, 716, TEST_IN_VIEWPORT_LINK_URL); + assertLinkUrl(playerHostView, 798, 711, TEST_IN_VIEWPORT_LINK_URL); + + // Scroll to the bottom, and click on a link. + scrollToBottom(); + int playerHeight = playerHostView.getHeight(); + assertLinkUrl(playerHostView, 322, playerHeight - (TEST_PAGE_HEIGHT - 4946), + TEST_OUT_OF_VIEWPORT_LINK_URL); + assertLinkUrl(playerHostView, 376, playerHeight - (TEST_PAGE_HEIGHT - 4954), + TEST_OUT_OF_VIEWPORT_LINK_URL); + assertLinkUrl(playerHostView, 422, playerHeight - (TEST_PAGE_HEIGHT - 4965), + TEST_OUT_OF_VIEWPORT_LINK_URL); + } + + /** + * Scrolls to the bottom fo the paint preview. + */ + private void scrollToBottom() { + UiDevice uiDevice = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation()); + int deviceHeight = uiDevice.getDisplayHeight(); + + Rect visibleContentRect = new Rect(); + getActivity().getWindow().getDecorView().getWindowVisibleDisplayFrame(visibleContentRect); + int statusBarHeight = visibleContentRect.top; + + int navigationBarHeight = 100; + int resourceId = getActivity().getResources().getIdentifier( + "navigation_bar_height", "dimen", "android"); + if (resourceId > 0) { + navigationBarHeight = getActivity().getResources().getDimensionPixelSize(resourceId); + } + + int padding = 20; + int swipeSteps = 5; + int viewPortBottom = deviceHeight - statusBarHeight - navigationBarHeight; + while (viewPortBottom < TEST_PAGE_HEIGHT) { + int fromY = deviceHeight - navigationBarHeight - padding; + int toY = statusBarHeight + padding; + uiDevice.swipe(50, fromY, 50, toY, swipeSteps); + viewPortBottom += fromY - toY; + } + } + + private void initPlayerManager() { + mLinkClickHandler = new TestLinkClickHandler(); + PostTask.postTask(UiThreadTaskTraits.DEFAULT, () -> { + PaintPreviewTestService service = + new PaintPreviewTestService(UrlUtils.getIsolatedTestFilePath(TEST_DATA_DIR)); + mPlayerManager = new PlayerManager(new GURL(TEST_URL), getActivity(), service, + TEST_DIRECTORY_KEY, mLinkClickHandler, Assert::assertTrue); + getActivity().setContentView(mPlayerManager.getView()); + }); + + // Wait until PlayerManager is initialized. + CriteriaHelper.pollUiThread(() -> mPlayerManager != null, + "PlayerManager was not initialized.", TIMEOUT_MS, + CriteriaHelper.DEFAULT_POLLING_INTERVAL); + + // Assert that the player view is added to the player host view. + CriteriaHelper.pollUiThread( + () -> ((ViewGroup) mPlayerManager.getView()).getChildCount() > 0, + "Player view is not added to the host view.", TIMEOUT_MS, + CriteriaHelper.DEFAULT_POLLING_INTERVAL); + } + + private void assertLinkUrl(View view, int x, int y, String expectedUrl) { + mLinkClickHandler.mUrl = null; + dispatchTapEvent(view, x, y); + CriteriaHelper.pollUiThread( + ()-> { + GURL url = mLinkClickHandler.mUrl; + if (url == null) return false; + + return url.getSpec().equals(expectedUrl); + }, + "Link press on (" + x + ", " + y + ") failed. Expected: " + expectedUrl + + ", found: " + + (mLinkClickHandler.mUrl == null ? null + : mLinkClickHandler.mUrl.getSpec()), + TIMEOUT_MS, CriteriaHelper.DEFAULT_POLLING_INTERVAL); + } + + private void dispatchTapEvent(View view, int x, int y) { + long downTime = SystemClock.uptimeMillis(); + MotionEvent downEvent = MotionEvent.obtain( + downTime, downTime + 100, MotionEvent.ACTION_DOWN, (float) x, (float) y, 0); + MotionEvent upEvent = MotionEvent.obtain( + downTime + 150, downTime + 200, MotionEvent.ACTION_UP, (float) x, (float) y, 0); + + PostTask.postTask(UiThreadTaskTraits.DEFAULT, () -> { + view.dispatchTouchEvent(downEvent); + view.dispatchTouchEvent(upEvent); + }); + } +} diff --git a/chromium/components/paint_preview/player/android/javatests/src/org/chromium/components/paintpreview/player/PaintPreviewTestRule.java b/chromium/components/paint_preview/player/android/javatests/src/org/chromium/components/paintpreview/player/PaintPreviewTestRule.java new file mode 100644 index 00000000000..a52cbfc2e24 --- /dev/null +++ b/chromium/components/paint_preview/player/android/javatests/src/org/chromium/components/paintpreview/player/PaintPreviewTestRule.java @@ -0,0 +1,42 @@ +// 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. + +package org.chromium.components.paintpreview.player; + +import org.junit.runner.Description; +import org.junit.runners.model.Statement; + +import org.chromium.components.signin.AccountManagerFacade; +import org.chromium.components.signin.AccountManagerFacadeProvider; +import org.chromium.components.signin.test.util.FakeAccountManagerDelegate; +import org.chromium.content_public.browser.test.NativeLibraryTestRule; + +/** + * Loads native and initializes the browser process for Paint Preview instrumentation tests. + */ +public class PaintPreviewTestRule extends NativeLibraryTestRule { + private FakeAccountManagerDelegate mAccountManager; + + /** + * {@link AccountManagerFacadeProvider#getInstance()} is called in the browser initialization + * path. If we don't mock {@link AccountManagerFacade}, we'll run into a failed assertion. + */ + private void setUp() { + mAccountManager = new FakeAccountManagerDelegate( + FakeAccountManagerDelegate.DISABLE_PROFILE_DATA_SOURCE); + AccountManagerFacadeProvider.overrideAccountManagerFacadeForTests(mAccountManager); + loadNativeLibraryAndInitBrowserProcess(); + } + + @Override + public Statement apply(final Statement base, Description description) { + return super.apply(new Statement() { + @Override + public void evaluate() throws Throwable { + setUp(); + base.evaluate(); + } + }, description); + } +} 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 new file mode 100644 index 00000000000..ce6ac7fa122 --- /dev/null +++ b/chromium/components/paint_preview/player/android/javatests/src/org/chromium/components/paintpreview/player/PaintPreviewTestService.java @@ -0,0 +1,31 @@ +// 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. + +package org.chromium.components.paintpreview.player; + +import org.chromium.base.annotations.JNINamespace; +import org.chromium.base.annotations.NativeMethods; +import org.chromium.components.paintpreview.browser.NativePaintPreviewServiceProvider; + +/** + * A simple implementation of {@link NativePaintPreviewServiceProvider} used in tests. + */ +@JNINamespace("paint_preview") +public class PaintPreviewTestService implements NativePaintPreviewServiceProvider { + private long mNativePaintPreviewTestService; + + public PaintPreviewTestService(String testDataDir) { + mNativePaintPreviewTestService = PaintPreviewTestServiceJni.get().getInstance(testDataDir); + } + + @Override + public long getNativeService() { + return mNativePaintPreviewTestService; + } + + @NativeMethods + interface Natives { + long getInstance(String testDataDir); + } +} diff --git a/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/PlayerManagerTest.java b/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/PlayerManagerTest.java index bb17983ad2b..ba583b83118 100644 --- a/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/PlayerManagerTest.java +++ b/chromium/components/paint_preview/player/android/junit/src/org/chromium/components/paintpreview/player/PlayerManagerTest.java @@ -5,11 +5,13 @@ package org.chromium.components.paintpreview.player; import android.graphics.Rect; +import android.os.Parcel; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; +import org.chromium.base.UnguessableToken; import org.chromium.base.test.BaseRobolectricTestRunner; /** @@ -17,16 +19,26 @@ import org.chromium.base.test.BaseRobolectricTestRunner; */ @RunWith(BaseRobolectricTestRunner.class) public class PlayerManagerTest { + private UnguessableToken makeToken(long high, long low) { + // Use a parcel for testing to avoid calling the normal native constructor. + Parcel parcel = Parcel.obtain(); + parcel.writeLong(high); + parcel.writeLong(low); + parcel.setDataPosition(0); + return UnguessableToken.CREATOR.createFromParcel(parcel); + } + /** * Tests the {@link PlayerManager#buildFrameTreeHierarchy} method with a simple frame * that has no sub-frames. */ @Test public void testFrameHierarchyBuilderNoSubFrames() { - PaintPreviewFrame generatedFrame1 = PlayerManager.buildFrameTreeHierarchy(34213523421L, - new long[] {34213523421L}, new int[] {500, 600}, new int[] {0}, null, null); + UnguessableToken token = makeToken(3728193L, 3283974L); + PaintPreviewFrame generatedFrame1 = PlayerManager.buildFrameTreeHierarchy(token, + new UnguessableToken[] {token}, new int[] {500, 600}, new int[] {0}, null, null); PaintPreviewFrame expectedFrame1 = PaintPreviewFrame.createInstanceForTest( - 34213523421L, 500, 600, new PaintPreviewFrame[] {}, new Rect[] {}); + token, 500, 600, new PaintPreviewFrame[] {}, new Rect[] {}); Assert.assertEquals(expectedFrame1, generatedFrame1); } @@ -36,14 +48,16 @@ public class PlayerManagerTest { */ @Test public void testFrameHierarchyBuilderOneSubFrame() { - PaintPreviewFrame generatedFrame2 = PlayerManager.buildFrameTreeHierarchy( - 23874287482142734L, new long[] {23874287482142734L, 34747571234L}, - new int[] {500, 600, 100, 200}, new int[] {1, 0}, new long[] {34747571234L}, + UnguessableToken mainToken = makeToken(1293L, 89798L); + UnguessableToken subframeToken = makeToken(123982L, 637846L); + PaintPreviewFrame generatedFrame2 = PlayerManager.buildFrameTreeHierarchy(mainToken, + new UnguessableToken[] {mainToken, subframeToken}, new int[] {500, 600, 100, 200}, + new int[] {1, 0}, new UnguessableToken[] {subframeToken}, new int[] {10, 10, 50, 50}); PaintPreviewFrame expectedFrame2Subframe = PaintPreviewFrame.createInstanceForTest( - 34747571234L, 100, 200, new PaintPreviewFrame[] {}, new Rect[] {}); - PaintPreviewFrame expectedFrame2 = PaintPreviewFrame.createInstanceForTest( - 23874287482142734L, 500, 600, new PaintPreviewFrame[] {expectedFrame2Subframe}, + subframeToken, 100, 200, new PaintPreviewFrame[] {}, new Rect[] {}); + PaintPreviewFrame expectedFrame2 = PaintPreviewFrame.createInstanceForTest(mainToken, 500, + 600, new PaintPreviewFrame[] {expectedFrame2Subframe}, new Rect[] {new Rect(10, 10, 60, 60)}); Assert.assertEquals(expectedFrame2, generatedFrame2); } @@ -56,21 +70,23 @@ public class PlayerManagerTest { */ @Test public void testFrameHierarchyBuilderTwoSubFrames() { - PaintPreviewFrame generatedFrame3 = - PlayerManager.buildFrameTreeHierarchy(8475737372237427342L, - new long[] {8475737372237427342L, 218932173213L, 39728389472348L}, - new int[] {500, 600, 200, 100, 10, 20}, new int[] {1, 2, 0}, - new long[] {218932173213L, 39728389472348L, 39728389472348L}, - new int[] {50, 60, 100, 150, 10, 15, 20, 25, 30, 35, 5, 15}); + UnguessableToken mainToken = makeToken(9876L, 1234L); + UnguessableToken subframe1Token = makeToken(32879342L, 2931920L); + UnguessableToken subframe2Token = makeToken(989272L, 3789489L); + PaintPreviewFrame generatedFrame3 = PlayerManager.buildFrameTreeHierarchy(mainToken, + new UnguessableToken[] {mainToken, subframe1Token, subframe2Token}, + new int[] {500, 600, 200, 100, 10, 20}, new int[] {1, 2, 0}, + new UnguessableToken[] {subframe1Token, subframe2Token, subframe2Token}, + new int[] {50, 60, 100, 150, 10, 15, 20, 25, 30, 35, 5, 15}); PaintPreviewFrame expectedFrame3Subframe2 = PaintPreviewFrame.createInstanceForTest( - 39728389472348L, 10, 20, new PaintPreviewFrame[] {}, new Rect[] {}); + subframe2Token, 10, 20, new PaintPreviewFrame[] {}, new Rect[] {}); PaintPreviewFrame expectedFrame3Subframe1 = - PaintPreviewFrame.createInstanceForTest(218932173213L, 200, 100, + PaintPreviewFrame.createInstanceForTest(subframe1Token, 200, 100, new PaintPreviewFrame[] {expectedFrame3Subframe2, expectedFrame3Subframe2}, new Rect[] {new Rect(10, 15, 30, 40), new Rect(30, 35, 35, 50)}); - PaintPreviewFrame expectedFrame3 = PaintPreviewFrame.createInstanceForTest( - 8475737372237427342L, 500, 600, new PaintPreviewFrame[] {expectedFrame3Subframe1}, + PaintPreviewFrame expectedFrame3 = PaintPreviewFrame.createInstanceForTest(mainToken, 500, + 600, new PaintPreviewFrame[] {expectedFrame3Subframe1}, new Rect[] {new Rect(50, 60, 150, 210)}); Assert.assertEquals(expectedFrame3, generatedFrame3); } -}
\ No newline at end of file +} 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 a6df81085c4..072c18668cb 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 @@ -8,8 +8,9 @@ import android.graphics.Bitmap; import android.graphics.Canvas; import android.graphics.Paint; import android.graphics.Rect; -import android.support.annotation.NonNull; -import android.support.annotation.Nullable; + +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import org.junit.Assert; import org.junit.Test; @@ -80,6 +81,16 @@ public class PlayerFrameBitmapPainterTest { } } + private Bitmap[][] generateMockBitmapMatrix(int rows, int cols) { + Bitmap[][] matrix = new Bitmap[rows][cols]; + for (int row = 0; row < matrix.length; ++row) { + for (int col = 0; col < matrix[row].length; ++col) { + matrix[row][col] = Mockito.mock(Bitmap.class); + } + } + return matrix; + } + /** * Verifies no draw operations are performed on the canvas if the view port is invalid. */ @@ -87,7 +98,7 @@ public class PlayerFrameBitmapPainterTest { public void testDrawFaultyViewPort() { PlayerFrameBitmapPainter painter = new PlayerFrameBitmapPainter(Mockito.mock(Runnable.class)); - painter.updateBitmapMatrix(new Bitmap[2][3]); + painter.updateBitmapMatrix(generateMockBitmapMatrix(2, 3)); painter.updateViewPort(0, 5, 10, -10); MockCanvas canvas = new MockCanvas(); @@ -116,7 +127,7 @@ public class PlayerFrameBitmapPainterTest { painter.onDraw(canvas); canvas.assertNumberOfBitmapDraws(0); - painter.updateBitmapMatrix(new Bitmap[2][1]); + painter.updateBitmapMatrix(generateMockBitmapMatrix(2, 1)); painter.onDraw(canvas); canvas.assertNumberOfBitmapDraws(2); } 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 a3fe4422262..020038e4227 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 @@ -5,11 +5,13 @@ package org.chromium.components.paintpreview.player.frame; import android.graphics.Bitmap; -import android.graphics.Point; import android.graphics.Rect; +import android.os.Parcel; import android.util.Pair; import android.view.View; +import androidx.annotation.NonNull; + import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -17,6 +19,7 @@ import org.junit.runner.RunWith; import org.mockito.Mockito; import org.chromium.base.Callback; +import org.chromium.base.UnguessableToken; import org.chromium.base.test.BaseRobolectricTestRunner; import org.chromium.components.paintpreview.player.PlayerCompositorDelegate; import org.chromium.ui.modelutil.PropertyModel; @@ -30,25 +33,37 @@ import java.util.List; */ @RunWith(BaseRobolectricTestRunner.class) public class PlayerFrameMediatorTest { - private static final long FRAME_GUID = 123321L; private static final int CONTENT_WIDTH = 560; private static final int CONTENT_HEIGHT = 1150; + private UnguessableToken mFrameGuid; private PropertyModel mModel; private TestPlayerCompositorDelegate mCompositorDelegate; private PlayerFrameMediator mMediator; /** + * Generate an UnguessableToken with a static value. + */ + private UnguessableToken frameGuid() { + // Use a parcel for testing to avoid calling the normal native constructor. + Parcel parcel = Parcel.obtain(); + parcel.writeLong(123321L); + parcel.writeLong(987654L); + parcel.setDataPosition(0); + return UnguessableToken.CREATOR.createFromParcel(parcel); + } + + /** * Used for keeping track of all bitmap requests that {@link PlayerFrameMediator} makes. */ private class RequestedBitmap { - long mFrameGuid; + UnguessableToken mFrameGuid; Rect mClipRect; float mScaleFactor; Callback<Bitmap> mBitmapCallback; Runnable mErrorCallback; - public RequestedBitmap(long frameGuid, Rect clipRect, float scaleFactor, + public RequestedBitmap(UnguessableToken frameGuid, Rect clipRect, float scaleFactor, Callback<Bitmap> bitmapCallback, Runnable errorCallback) { this.mFrameGuid = frameGuid; this.mClipRect = clipRect; @@ -57,7 +72,7 @@ public class PlayerFrameMediatorTest { this.mErrorCallback = errorCallback; } - public RequestedBitmap(long frameGuid, Rect clipRect, float scaleFactor) { + public RequestedBitmap(UnguessableToken frameGuid, Rect clipRect, float scaleFactor) { this.mFrameGuid = frameGuid; this.mClipRect = clipRect; this.mScaleFactor = scaleFactor; @@ -72,10 +87,11 @@ public class PlayerFrameMediatorTest { if (o.getClass() != this.getClass()) return false; RequestedBitmap rb = (RequestedBitmap) o; - return rb.mClipRect.equals(mClipRect) && rb.mFrameGuid == mFrameGuid + return rb.mClipRect.equals(mClipRect) && rb.mFrameGuid.equals(mFrameGuid) && rb.mScaleFactor == mScaleFactor; } + @NonNull @Override public String toString() { return mFrameGuid + ", " + mClipRect + ", " + mScaleFactor; @@ -83,29 +99,68 @@ public class PlayerFrameMediatorTest { } /** + * Used for keeping track of all click events that {@link PlayerFrameMediator} sends to + * {@link PlayerCompositorDelegate}. + */ + private class ClickedPoint { + UnguessableToken mFrameGuid; + int mX; + int mY; + + public ClickedPoint(UnguessableToken frameGuid, int x, int y) { + mFrameGuid = frameGuid; + this.mX = x; + this.mY = y; + } + + @Override + public boolean equals(Object o) { + if (o == null) return false; + + if (o == this) return true; + + if (o.getClass() != this.getClass()) return false; + + ClickedPoint cp = (ClickedPoint) o; + return cp.mFrameGuid.equals(mFrameGuid) && cp.mX == mX && cp.mY == mY; + } + + @NonNull + @Override + public String toString() { + return "Click event for frame " + mFrameGuid.toString() + " on (" + mX + ", " + mY + + ")"; + } + } + + /** * Mocks {@link PlayerCompositorDelegate}. Stores all bitmap requests as * {@link RequestedBitmap}s. */ private class TestPlayerCompositorDelegate implements PlayerCompositorDelegate { List<RequestedBitmap> mRequestedBitmap = new ArrayList<>(); + List<ClickedPoint> mClickedPoints = new ArrayList<>(); @Override - public void requestBitmap(long frameGuid, Rect clipRect, float scaleFactor, + public void requestBitmap(UnguessableToken frameGuid, Rect clipRect, float scaleFactor, Callback<Bitmap> bitmapCallback, Runnable errorCallback) { mRequestedBitmap.add(new RequestedBitmap( frameGuid, new Rect(clipRect), scaleFactor, bitmapCallback, errorCallback)); } @Override - public void onClick(long frameGuid, Point point) {} + public void onClick(UnguessableToken frameGuid, int x, int y) { + mClickedPoints.add(new ClickedPoint(frameGuid, x, y)); + } } @Before public void setUp() { + mFrameGuid = frameGuid(); mModel = new PropertyModel.Builder(PlayerFrameProperties.ALL_KEYS).build(); mCompositorDelegate = new TestPlayerCompositorDelegate(); mMediator = new PlayerFrameMediator( - mModel, mCompositorDelegate, FRAME_GUID, CONTENT_WIDTH, CONTENT_HEIGHT); + mModel, mCompositorDelegate, mFrameGuid, CONTENT_WIDTH, CONTENT_HEIGHT); } /** @@ -143,7 +198,7 @@ public class PlayerFrameMediatorTest { // Since the current view port fully matches the top left bitmap tile, that should be the // only requested bitmap. List<RequestedBitmap> expectedRequestedBitmaps = new ArrayList<>(); - expectedRequestedBitmaps.add(new RequestedBitmap(FRAME_GUID, new Rect(0, 0, 100, 200), 1f)); + expectedRequestedBitmaps.add(new RequestedBitmap(mFrameGuid, new Rect(0, 0, 100, 200), 1f)); Assert.assertEquals(expectedRequestedBitmaps, mCompositorDelegate.mRequestedBitmap); mMediator.scrollBy(10, 20); @@ -155,11 +210,11 @@ public class PlayerFrameMediatorTest { // The current viewport covers portions of 4 adjacent bitmap tiles. Make sure requests for // compositing those bitmap tiles are made. expectedRequestedBitmaps.add( - new RequestedBitmap(FRAME_GUID, new Rect(0, 200, 100, 400), 1f)); + new RequestedBitmap(mFrameGuid, new Rect(0, 200, 100, 400), 1f)); expectedRequestedBitmaps.add( - new RequestedBitmap(FRAME_GUID, new Rect(100, 0, 200, 200), 1f)); + new RequestedBitmap(mFrameGuid, new Rect(100, 0, 200, 200), 1f)); expectedRequestedBitmaps.add( - new RequestedBitmap(FRAME_GUID, new Rect(100, 200, 200, 400), 1f)); + new RequestedBitmap(mFrameGuid, new Rect(100, 200, 200, 400), 1f)); Assert.assertEquals(expectedRequestedBitmaps, mCompositorDelegate.mRequestedBitmap); // Move the view port slightly. It is still covered by the same 4 tiles. Since there were @@ -174,13 +229,13 @@ public class PlayerFrameMediatorTest { Assert.assertEquals(expectedViewPort, mModel.get(PlayerFrameProperties.VIEWPORT)); expectedRequestedBitmaps.add( - new RequestedBitmap(FRAME_GUID, new Rect(400, 800, 500, 1000), 1f)); + new RequestedBitmap(mFrameGuid, new Rect(400, 800, 500, 1000), 1f)); expectedRequestedBitmaps.add( - new RequestedBitmap(FRAME_GUID, new Rect(400, 1000, 500, 1200), 1f)); + new RequestedBitmap(mFrameGuid, new Rect(400, 1000, 500, 1200), 1f)); expectedRequestedBitmaps.add( - new RequestedBitmap(FRAME_GUID, new Rect(500, 800, 600, 1000), 1f)); + new RequestedBitmap(mFrameGuid, new Rect(500, 800, 600, 1000), 1f)); expectedRequestedBitmaps.add( - new RequestedBitmap(FRAME_GUID, new Rect(500, 1000, 600, 1200), 1f)); + new RequestedBitmap(mFrameGuid, new Rect(500, 1000, 600, 1200), 1f)); Assert.assertEquals(expectedRequestedBitmaps, mCompositorDelegate.mRequestedBitmap); } @@ -211,13 +266,13 @@ public class PlayerFrameMediatorTest { // Assert that there are the only 4 total bitmap requests, i.e. we didn't request for the // tile at [0][0] again. List<RequestedBitmap> expectedRequestedBitmaps = new ArrayList<>(); - expectedRequestedBitmaps.add(new RequestedBitmap(FRAME_GUID, new Rect(0, 0, 150, 200), 1f)); + expectedRequestedBitmaps.add(new RequestedBitmap(mFrameGuid, new Rect(0, 0, 150, 200), 1f)); expectedRequestedBitmaps.add( - new RequestedBitmap(FRAME_GUID, new Rect(0, 200, 150, 400), 1f)); + new RequestedBitmap(mFrameGuid, new Rect(0, 200, 150, 400), 1f)); expectedRequestedBitmaps.add( - new RequestedBitmap(FRAME_GUID, new Rect(150, 0, 300, 200), 1f)); + new RequestedBitmap(mFrameGuid, new Rect(150, 0, 300, 200), 1f)); expectedRequestedBitmaps.add( - new RequestedBitmap(FRAME_GUID, new Rect(150, 200, 300, 400), 1f)); + new RequestedBitmap(mFrameGuid, new Rect(150, 200, 300, 400), 1f)); Assert.assertEquals(expectedRequestedBitmaps, mCompositorDelegate.mRequestedBitmap); // Mock a compositing failure for the second request. @@ -227,7 +282,7 @@ public class PlayerFrameMediatorTest { // compositing failure. mMediator.scrollBy(10, 10); expectedRequestedBitmaps.add( - new RequestedBitmap(FRAME_GUID, new Rect(0, 200, 150, 400), 1f)); + new RequestedBitmap(mFrameGuid, new Rect(0, 200, 150, 400), 1f)); Assert.assertEquals(expectedRequestedBitmaps, mCompositorDelegate.mRequestedBitmap); } @@ -344,6 +399,34 @@ public class PlayerFrameMediatorTest { } /** + * Tests that {@link PlayerFrameMediator} correctly relays the click events to + * {@link PlayerCompositorDelegate} and accounts for scroll offsets. + */ + @Test + public void testOnClick() { + // Initial view port setup. + mMediator.setLayoutDimensions(100, 200); + List<ClickedPoint> expectedClickedPoints = new ArrayList<>(); + + // No scrolling has happened yet. + mMediator.onClick(15, 26); + 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. + mMediator.scrollBy(90, 100); + mMediator.onClick(70, 50); + expectedClickedPoints.add(new ClickedPoint(mFrameGuid, 160, 150)); + Assert.assertEquals(expectedClickedPoints, mCompositorDelegate.mClickedPoints); + + mMediator.scrollBy(-40, -60); + mMediator.onClick(30, 80); + expectedClickedPoints.add(new ClickedPoint(mFrameGuid, 80, 120)); + Assert.assertEquals(expectedClickedPoints, mCompositorDelegate.mClickedPoints); + } + + /** * TODO(crbug.com/1020702): Implement after zooming support is added. */ @Test 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 4d6d75a47e8..25886aed2ce 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 @@ -9,7 +9,10 @@ #include "base/android/callback_android.h" #include "base/android/jni_array.h" #include "base/android/jni_string.h" +#include "base/android/unguessable_token_android.h" #include "base/bind.h" +#include "base/unguessable_token.h" +#include "components/paint_preview/browser/paint_preview_base_service.h" #include "components/paint_preview/player/android/jni_headers/PlayerCompositorDelegateImpl_jni.h" #include "components/services/paint_preview_compositor/public/mojom/paint_preview_compositor.mojom.h" #include "third_party/skia/include/core/SkBitmap.h" @@ -23,64 +26,108 @@ using base::android::ScopedJavaLocalRef; namespace paint_preview { +namespace { + +ScopedJavaLocalRef<jobjectArray> ToJavaUnguessableTokenArray( + JNIEnv* env, + const std::vector<base::UnguessableToken>& tokens) { + ScopedJavaLocalRef<jclass> j_unguessable_token_class = + base::android::GetClass(env, "org/chromium/base/UnguessableToken"); + jobjectArray joa = env->NewObjectArray( + tokens.size(), j_unguessable_token_class.obj(), nullptr); + base::android::CheckException(env); + + for (size_t i = 0; i < tokens.size(); ++i) { + ScopedJavaLocalRef<jobject> j_unguessable_token = + base::android::UnguessableTokenAndroid::Create(env, tokens[i]); + env->SetObjectArrayElement(joa, i, j_unguessable_token.obj()); + } + + return ScopedJavaLocalRef<jobjectArray>(env, joa); +} + +} // namespace + jlong JNI_PlayerCompositorDelegateImpl_Initialize( JNIEnv* env, const JavaParamRef<jobject>& j_object, - const JavaParamRef<jstring>& j_string_url) { - PlayerCompositorDelegateAndroid* mediator = - new PlayerCompositorDelegateAndroid(env, j_object, j_string_url); - return reinterpret_cast<intptr_t>(mediator); + jlong paint_preview_service, + const JavaParamRef<jstring>& j_url_spec, + const JavaParamRef<jstring>& j_directory_key) { + PlayerCompositorDelegateAndroid* delegate = + new PlayerCompositorDelegateAndroid( + env, j_object, + reinterpret_cast<PaintPreviewBaseService*>(paint_preview_service), + j_url_spec, j_directory_key); + return reinterpret_cast<intptr_t>(delegate); } PlayerCompositorDelegateAndroid::PlayerCompositorDelegateAndroid( JNIEnv* env, const JavaParamRef<jobject>& j_object, - const JavaParamRef<jstring>& j_string_url) + PaintPreviewBaseService* paint_preview_service, + const JavaParamRef<jstring>& j_url_spec, + const JavaParamRef<jstring>& j_directory_key) : PlayerCompositorDelegate( - GURL(base::android::ConvertJavaStringToUTF16(env, j_string_url))) { + paint_preview_service, + GURL(base::android::ConvertJavaStringToUTF8(env, j_url_spec)), + DirectoryKey{ + base::android::ConvertJavaStringToUTF8(env, j_directory_key)}) { java_ref_.Reset(env, j_object); } void PlayerCompositorDelegateAndroid::OnCompositorReady( - const mojom::PaintPreviewBeginCompositeResponse& composite_response) { + mojom::PaintPreviewCompositor::Status status, + mojom::PaintPreviewBeginCompositeResponsePtr composite_response) { JNIEnv* env = base::android::AttachCurrentThread(); - // We use int64_t instead of uint64_t because (i) there is no equivalent - // type to uint64_t in Java and (ii) base::android::ToJavaLongArray accepts - // a std::vector<int64_t> as input. - std::vector<int64_t> all_guids; + std::vector<base::UnguessableToken> all_guids; std::vector<int> scroll_extents; std::vector<int> subframe_count; - std::vector<int64_t> subframe_ids; + std::vector<base::UnguessableToken> subframe_ids; std::vector<int> subframe_rects; + base::UnguessableToken root_frame_guid; - CompositeResponseFramesToVectors(composite_response.frames, &all_guids, - &scroll_extents, &subframe_count, - &subframe_ids, &subframe_rects); + if (composite_response) { + CompositeResponseFramesToVectors(composite_response->frames, &all_guids, + &scroll_extents, &subframe_count, + &subframe_ids, &subframe_rects); + root_frame_guid = composite_response->root_frame_guid; + } else { + // If there is no composite response due to a failure we don't have a root + // frame GUID to pass. However, the token cannot be null so create a + // placeholder. + root_frame_guid = base::UnguessableToken::Create(); + } - ScopedJavaLocalRef<jlongArray> j_all_guids = - base::android::ToJavaLongArray(env, all_guids); + ScopedJavaLocalRef<jobjectArray> j_all_guids = + ToJavaUnguessableTokenArray(env, all_guids); ScopedJavaLocalRef<jintArray> j_scroll_extents = base::android::ToJavaIntArray(env, scroll_extents); ScopedJavaLocalRef<jintArray> j_subframe_count = base::android::ToJavaIntArray(env, subframe_count); - ScopedJavaLocalRef<jlongArray> j_subframe_ids = - base::android::ToJavaLongArray(env, subframe_ids); + ScopedJavaLocalRef<jobjectArray> j_subframe_ids = + ToJavaUnguessableTokenArray(env, subframe_ids); ScopedJavaLocalRef<jintArray> j_subframe_rects = base::android::ToJavaIntArray(env, subframe_rects); + ScopedJavaLocalRef<jobject> j_root_frame_guid = + base::android::UnguessableTokenAndroid::Create(env, root_frame_guid); Java_PlayerCompositorDelegateImpl_onCompositorReady( - env, java_ref_, composite_response.root_frame_guid, j_all_guids, - j_scroll_extents, j_subframe_count, j_subframe_ids, j_subframe_rects); + env, java_ref_, + static_cast<jboolean>(status == + mojom::PaintPreviewCompositor::Status::kSuccess), + j_root_frame_guid, j_all_guids, j_scroll_extents, j_subframe_count, + j_subframe_ids, j_subframe_rects); } // static void PlayerCompositorDelegateAndroid::CompositeResponseFramesToVectors( - const base::flat_map<uint64_t, mojom::FrameDataPtr>& frames, - std::vector<int64_t>* all_guids, + const base::flat_map<base::UnguessableToken, mojom::FrameDataPtr>& frames, + std::vector<base::UnguessableToken>* all_guids, std::vector<int>* scroll_extents, std::vector<int>* subframe_count, - std::vector<int64_t>* subframe_ids, + std::vector<base::UnguessableToken>* subframe_ids, std::vector<int>* subframe_rects) { all_guids->reserve(frames.size()); scroll_extents->reserve(2 * frames.size()); @@ -109,7 +156,7 @@ void PlayerCompositorDelegateAndroid::CompositeResponseFramesToVectors( void PlayerCompositorDelegateAndroid::RequestBitmap( JNIEnv* env, - jlong j_frame_guid, + const JavaParamRef<jobject>& j_frame_guid, const JavaParamRef<jobject>& j_bitmap_callback, const JavaParamRef<jobject>& j_error_callback, jfloat j_scale_factor, @@ -120,7 +167,9 @@ void PlayerCompositorDelegateAndroid::RequestBitmap( gfx::Rect clip_rect = gfx::Rect(j_clip_x, j_clip_y, j_clip_width, j_clip_height); PlayerCompositorDelegate::RequestBitmap( - j_frame_guid, clip_rect, j_scale_factor, + base::android::UnguessableTokenAndroid::FromJavaUnguessableToken( + env, j_frame_guid), + clip_rect, j_scale_factor, base::BindOnce(&PlayerCompositorDelegateAndroid::OnBitmapCallback, weak_factory_.GetWeakPtr(), ScopedJavaGlobalRef<jobject>(j_bitmap_callback), @@ -140,11 +189,22 @@ void PlayerCompositorDelegateAndroid::OnBitmapCallback( } } -void PlayerCompositorDelegateAndroid::OnClick(JNIEnv* env, - jlong j_frame_guid, - jint j_x, - jint j_y) { - PlayerCompositorDelegate::OnClick(j_frame_guid, j_x, j_y); +void PlayerCompositorDelegateAndroid::OnClick( + JNIEnv* env, + const JavaParamRef<jobject>& j_frame_guid, + jint j_x, + jint j_y) { + auto res = PlayerCompositorDelegate::OnClick( + base::android::UnguessableTokenAndroid::FromJavaUnguessableToken( + env, j_frame_guid), + gfx::Rect(static_cast<int>(j_x), static_cast<int>(j_y), 1U, 1U)); + if (res.empty()) + return; + // TODO(crbug/1061435): Resolve cases where there are multiple links. + // For now just return the first in the list. + Java_PlayerCompositorDelegateImpl_onLinkClicked( + env, java_ref_, + base::android::ConvertUTF8ToJavaString(env, res[0]->spec())); } void PlayerCompositorDelegateAndroid::Destroy(JNIEnv* env) { 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 ecafc4d1750..c808a137489 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 @@ -13,23 +13,27 @@ class SkBitmap; namespace paint_preview { +class PaintPreviewBaseService; class PlayerCompositorDelegateAndroid : public PlayerCompositorDelegate { public: PlayerCompositorDelegateAndroid( JNIEnv* env, const base::android::JavaParamRef<jobject>& jobject, - const base::android::JavaParamRef<jstring>& j_string_url); + PaintPreviewBaseService* paint_preview_service, + const base::android::JavaParamRef<jstring>& j_url_spec, + const base::android::JavaParamRef<jstring>& j_directory_key); - void OnCompositorReady(const mojom::PaintPreviewBeginCompositeResponse& - composite_response) override; + void OnCompositorReady( + mojom::PaintPreviewCompositor::Status status, + mojom::PaintPreviewBeginCompositeResponsePtr composite_response) override; // Called from Java when there is a request for a new bitmap. When the bitmap // is ready, it will be passed to j_bitmap_callback. In case of any failure, // j_error_callback will be called. void RequestBitmap( JNIEnv* env, - jlong j_frame_guid, + const base::android::JavaParamRef<jobject>& j_frame_guid, const base::android::JavaParamRef<jobject>& j_bitmap_callback, const base::android::JavaParamRef<jobject>& j_error_callback, jfloat j_scale_factor, @@ -39,16 +43,19 @@ class PlayerCompositorDelegateAndroid : public PlayerCompositorDelegate { jint j_clip_height); // Called from Java on touch event on a frame. - void OnClick(JNIEnv* env, jlong j_frame_guid, jint j_x, jint j_y); + void OnClick(JNIEnv* env, + const base::android::JavaParamRef<jobject>& j_frame_guid, + jint j_x, + jint j_y); void Destroy(JNIEnv* env); static void CompositeResponseFramesToVectors( - const base::flat_map<uint64_t, mojom::FrameDataPtr>& frames, - std::vector<int64_t>* all_guids, + const base::flat_map<base::UnguessableToken, mojom::FrameDataPtr>& frames, + std::vector<base::UnguessableToken>* all_guids, std::vector<int>* scroll_extents, std::vector<int>* subframe_count, - std::vector<int64_t>* subframe_ids, + std::vector<base::UnguessableToken>* subframe_ids, std::vector<int>* subframe_rects); private: diff --git a/chromium/components/paint_preview/player/android/player_compositor_delegate_android_unittest.cc b/chromium/components/paint_preview/player/android/player_compositor_delegate_android_unittest.cc index 2eb2e8bfb9b..07328828993 100644 --- a/chromium/components/paint_preview/player/android/player_compositor_delegate_android_unittest.cc +++ b/chromium/components/paint_preview/player/android/player_compositor_delegate_android_unittest.cc @@ -19,41 +19,72 @@ namespace paint_preview { TEST(PlayerCompositorDelegateAndroidTest, TestCompositeResponseFramesToVectors) { - base::flat_map<uint64_t, mojom::FrameDataPtr> frames; + base::flat_map<base::UnguessableToken, mojom::FrameDataPtr> frames; + auto main_guid = base::UnguessableToken::Create(); auto frame_data_main = mojom::FrameData::New(); + auto subframe_1_guid = base::UnguessableToken::Create(); auto frame_data_subframe_1 = mojom::FrameData::New(); + auto subframe_2_guid = base::UnguessableToken::Create(); auto frame_data_subframe_2 = mojom::FrameData::New(); frame_data_main->scroll_extents = gfx::Size(100, 200); frame_data_subframe_1->scroll_extents = gfx::Size(50, 60); frame_data_subframe_2->scroll_extents = gfx::Size(10, 20); - mojom::SubframeClipRect clip_rect1(50, gfx::Rect(5, 10, 50, 60)); - mojom::SubframeClipRect clip_rect2(76, gfx::Rect(15, 25, 30, 40)); + mojom::SubframeClipRect clip_rect1(subframe_1_guid, gfx::Rect(5, 10, 50, 60)); + mojom::SubframeClipRect clip_rect2(subframe_2_guid, + gfx::Rect(15, 25, 30, 40)); frame_data_main->subframes.push_back(clip_rect1.Clone()); frame_data_subframe_1->subframes.push_back(clip_rect2.Clone()); - frames.insert({12, std::move(frame_data_main)}); - frames.insert({50, std::move(frame_data_subframe_1)}); - frames.insert({76, std::move(frame_data_subframe_2)}); + frames.insert({main_guid, std::move(frame_data_main)}); + frames.insert({subframe_1_guid, std::move(frame_data_subframe_1)}); + frames.insert({subframe_2_guid, std::move(frame_data_subframe_2)}); - std::vector<int64_t> all_guids; + std::vector<base::UnguessableToken> all_guids; std::vector<int> scroll_extents; std::vector<int> subframe_count; - std::vector<int64_t> subframe_ids; + std::vector<base::UnguessableToken> subframe_ids; std::vector<int> subframe_rects; PlayerCompositorDelegateAndroid::CompositeResponseFramesToVectors( frames, &all_guids, &scroll_extents, &subframe_count, &subframe_ids, &subframe_rects); - EXPECT_THAT(all_guids, ::testing::ElementsAre(12, 50, 76)); - EXPECT_THAT(scroll_extents, ::testing::ElementsAre(100, 200, 50, 60, 10, 20)); - EXPECT_THAT(subframe_count, ::testing::ElementsAre(1, 1, 0)); - EXPECT_THAT(subframe_ids, ::testing::ElementsAre(50, 76)); - EXPECT_THAT(subframe_rects, - ::testing::ElementsAre(5, 10, 50, 60, 15, 25, 30, 40)); + EXPECT_EQ(all_guids.size(), frames.size()); + EXPECT_EQ(scroll_extents.size(), 2 * frames.size()); + EXPECT_EQ(subframe_count.size(), frames.size()); + EXPECT_EQ(subframe_ids.size(), 2U); // 2 subframes. + EXPECT_EQ(subframe_rects.size(), 2U * 4U); // 2 * subframes * 4 per rect. + size_t subframe_idx = 0; + for (size_t i = 0; i < all_guids.size(); ++i) { + auto it = frames.find(all_guids[i]); + ASSERT_NE(it, frames.end()); + + const size_t scroll_offset = i * 2; + EXPECT_EQ(scroll_extents[scroll_offset], + it->second->scroll_extents.width()); + EXPECT_EQ(scroll_extents[scroll_offset + 1], + it->second->scroll_extents.height()); + EXPECT_EQ(subframe_count[i], + static_cast<int>(it->second->subframes.size())); + for (size_t j = 0; j < it->second->subframes.size(); ++j) { + const size_t offset = subframe_idx + j; + EXPECT_EQ(subframe_ids[offset], it->second->subframes[j]->frame_guid); + + const size_t rect_offset = offset * 4; + EXPECT_EQ(subframe_rects[rect_offset], + it->second->subframes[j]->clip_rect.x()); + EXPECT_EQ(subframe_rects[rect_offset + 1], + it->second->subframes[j]->clip_rect.y()); + EXPECT_EQ(subframe_rects[rect_offset + 2], + it->second->subframes[j]->clip_rect.width()); + EXPECT_EQ(subframe_rects[rect_offset + 3], + it->second->subframes[j]->clip_rect.height()); + } + subframe_idx += it->second->subframes.size(); + } } } // namespace paint_preview diff --git a/chromium/components/paint_preview/player/player_compositor_delegate.cc b/chromium/components/paint_preview/player/player_compositor_delegate.cc index 8cd1e8eae52..f464d67b6a4 100644 --- a/chromium/components/paint_preview/player/player_compositor_delegate.cc +++ b/chromium/components/paint_preview/player/player_compositor_delegate.cc @@ -4,42 +4,252 @@ #include "components/paint_preview/player/player_compositor_delegate.h" +#include <string> +#include <utility> #include <vector> #include "base/callback.h" +#include "base/containers/flat_map.h" +#include "base/files/file_path.h" +#include "base/memory/read_only_shared_memory_region.h" +#include "base/memory/weak_ptr.h" +#include "base/optional.h" +#include "base/strings/string_piece.h" +#include "base/strings/utf_string_conversions.h" +#include "base/task/post_task.h" +#include "base/task/thread_pool.h" +#include "base/unguessable_token.h" +#include "build/build_config.h" +#include "components/paint_preview/browser/compositor_utils.h" +#include "components/paint_preview/browser/paint_preview_base_service.h" +#include "components/paint_preview/common/proto/paint_preview.pb.h" +#include "components/paint_preview/public/paint_preview_compositor_client.h" +#include "components/paint_preview/public/paint_preview_compositor_service.h" #include "components/services/paint_preview_compositor/public/mojom/paint_preview_compositor.mojom.h" +#include "mojo/public/cpp/bindings/remote.h" #include "third_party/skia/include/core/SkBitmap.h" #include "ui/gfx/geometry/rect.h" -#include "url/gurl.h" namespace paint_preview { -PlayerCompositorDelegate::PlayerCompositorDelegate(const GURL& url) { - // TODO(crbug.com/1019885): Use url to get proto and file map. - // TODO(crbug.com/1019885): Initialize the PaintPreviewCompositor class. - // TODO(crbug.com/1019883): Initialize the HitTester class. +namespace { + +std::pair<base::UnguessableToken, std::unique_ptr<HitTester>> BuildHitTester( + const PaintPreviewFrameProto& proto) { + std::pair<base::UnguessableToken, std::unique_ptr<HitTester>> out( + base::UnguessableToken::Deserialize(proto.embedding_token_high(), + proto.embedding_token_low()), + std::make_unique<HitTester>()); + out.second->Build(proto); + return out; +} + +base::flat_map<base::UnguessableToken, std::unique_ptr<HitTester>> +BuildHitTesters(const PaintPreviewProto& proto) { + std::vector<std::pair<base::UnguessableToken, std::unique_ptr<HitTester>>> + hit_testers; + hit_testers.reserve(proto.subframes_size() + 1); + hit_testers.push_back(BuildHitTester(proto.root_frame())); + for (const auto& frame_proto : proto.subframes()) + hit_testers.push_back(BuildHitTester(frame_proto)); + + return base::flat_map<base::UnguessableToken, std::unique_ptr<HitTester>>( + std::move(hit_testers)); +} + +base::FilePath ToFilePath(base::StringPiece path_str) { +#if defined(OS_WIN) + return base::FilePath(base::UTF8ToUTF16(path_str)); +#else + return base::FilePath(path_str); +#endif +} + +base::flat_map<base::UnguessableToken, base::File> CreateFileMapFromProto( + const PaintPreviewProto& proto) { + std::vector<std::pair<base::UnguessableToken, base::File>> entries; + entries.reserve(1 + proto.subframes_size()); + base::UnguessableToken root_frame_id = base::UnguessableToken::Deserialize( + proto.root_frame().embedding_token_high(), + proto.root_frame().embedding_token_low()); + base::File root_frame_skp_file = + base::File(ToFilePath(proto.root_frame().file_path()), + base::File::FLAG_OPEN | base::File::FLAG_READ); + + // We can't composite anything with an invalid SKP file path for the root + // frame. + if (!root_frame_skp_file.IsValid()) + return base::flat_map<base::UnguessableToken, base::File>(); + + entries.emplace_back(std::move(root_frame_id), + std::move(root_frame_skp_file)); + for (const auto& subframe : proto.subframes()) { + base::File frame_skp_file(ToFilePath(subframe.file_path()), + base::File::FLAG_OPEN | base::File::FLAG_READ); + + // Skip this frame if it doesn't have a valid SKP file path. + if (!frame_skp_file.IsValid()) + continue; + + entries.emplace_back( + base::UnguessableToken::Deserialize(subframe.embedding_token_high(), + subframe.embedding_token_low()), + std::move(frame_skp_file)); + } + return base::flat_map<base::UnguessableToken, base::File>(std::move(entries)); +} + +base::Optional<base::ReadOnlySharedMemoryRegion> ToReadOnlySharedMemory( + const paint_preview::PaintPreviewProto& proto) { + auto region = base::WritableSharedMemoryRegion::Create(proto.ByteSizeLong()); + if (!region.IsValid()) + return base::nullopt; + + auto mapping = region.Map(); + if (!mapping.IsValid()) + return base::nullopt; + + proto.SerializeToArray(mapping.memory(), mapping.size()); + return base::WritableSharedMemoryRegion::ConvertToReadOnly(std::move(region)); +} + +paint_preview::mojom::PaintPreviewBeginCompositeRequestPtr +PrepareCompositeRequest(const paint_preview::PaintPreviewProto& proto) { + paint_preview::mojom::PaintPreviewBeginCompositeRequestPtr + begin_composite_request = + paint_preview::mojom::PaintPreviewBeginCompositeRequest::New(); + begin_composite_request->file_map = CreateFileMapFromProto(proto); + if (begin_composite_request->file_map.empty()) + return nullptr; + + auto read_only_proto = ToReadOnlySharedMemory(proto); + if (!read_only_proto) { + // TODO(crbug.com/1021590): Handle initialization errors. + return nullptr; + } + begin_composite_request->proto = std::move(read_only_proto.value()); + return begin_composite_request; +} + +} // namespace + +PlayerCompositorDelegate::PlayerCompositorDelegate( + PaintPreviewBaseService* paint_preview_service, + const GURL& expected_url, + const DirectoryKey& key, + bool skip_service_launch) + : paint_preview_service_(paint_preview_service) { + if (skip_service_launch) { + paint_preview_service_->GetCapturedPaintPreviewProto( + key, base::BindOnce(&PlayerCompositorDelegate::OnProtoAvailable, + weak_factory_.GetWeakPtr(), expected_url)); + return; + } + paint_preview_compositor_service_ = + paint_preview_service_->StartCompositorService(base::BindOnce( + &PlayerCompositorDelegate::OnCompositorServiceDisconnected, + weak_factory_.GetWeakPtr())); + + paint_preview_compositor_client_ = + paint_preview_compositor_service_->CreateCompositor( + base::BindOnce(&PlayerCompositorDelegate::OnCompositorClientCreated, + weak_factory_.GetWeakPtr(), expected_url, key)); + paint_preview_compositor_client_->SetDisconnectHandler( + base::BindOnce(&PlayerCompositorDelegate::OnCompositorClientDisconnected, + weak_factory_.GetWeakPtr())); +} + +PlayerCompositorDelegate::~PlayerCompositorDelegate() = default; + +void PlayerCompositorDelegate::OnCompositorServiceDisconnected() { + // TODO(crbug.com/1039699): Handle compositor service disconnect event. +} + +void PlayerCompositorDelegate::OnCompositorClientCreated( + const GURL& expected_url, + const DirectoryKey& key) { + paint_preview_service_->GetCapturedPaintPreviewProto( + key, base::BindOnce(&PlayerCompositorDelegate::OnProtoAvailable, + weak_factory_.GetWeakPtr(), expected_url)); +} + +void PlayerCompositorDelegate::OnProtoAvailable( + const GURL& expected_url, + std::unique_ptr<PaintPreviewProto> proto) { + if (!proto || !proto->IsInitialized()) { + // TODO(crbug.com/1021590): Handle initialization errors. + OnCompositorReady( + mojom::PaintPreviewCompositor::Status::kCompositingFailure, nullptr); + return; + } + + auto proto_url = GURL(proto->metadata().url()); + if (expected_url != proto_url) { + OnCompositorReady( + mojom::PaintPreviewCompositor::Status::kDeserializingFailure, nullptr); + return; + } + + hit_testers_ = BuildHitTesters(*proto); + + if (!paint_preview_compositor_client_) { + OnCompositorReady( + mojom::PaintPreviewCompositor::Status::kCompositingFailure, nullptr); + return; + } + + paint_preview_compositor_client_->SetRootFrameUrl(proto_url); + + base::ThreadPool::PostTaskAndReplyWithResult( + FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE}, + base::BindOnce(&PrepareCompositeRequest, *proto), + base::BindOnce(&PlayerCompositorDelegate::SendCompositeRequest, + weak_factory_.GetWeakPtr())); +} + +void PlayerCompositorDelegate::SendCompositeRequest( + mojom::PaintPreviewBeginCompositeRequestPtr begin_composite_request) { + // TODO(crbug.com/1021590): Handle initialization errors. + if (!begin_composite_request) { + OnCompositorReady( + mojom::PaintPreviewCompositor::Status::kCompositingFailure, nullptr); + return; + } + + paint_preview_compositor_client_->BeginComposite( + std::move(begin_composite_request), + base::BindOnce(&PlayerCompositorDelegate::OnCompositorReady, + weak_factory_.GetWeakPtr())); +} + +void PlayerCompositorDelegate::OnCompositorClientDisconnected() { + // TODO(crbug.com/1039699): Handle compositor client disconnect event. } void PlayerCompositorDelegate::RequestBitmap( - uint64_t frame_guid, + const base::UnguessableToken& frame_guid, const gfx::Rect& clip_rect, float scale_factor, base::OnceCallback<void(mojom::PaintPreviewCompositor::Status, const SkBitmap&)> callback) { - if (!paint_preview_compositor_ || !paint_preview_compositor_.is_bound()) { + if (!paint_preview_compositor_client_) { std::move(callback).Run( mojom::PaintPreviewCompositor::Status::kCompositingFailure, SkBitmap()); return; } - paint_preview_compositor_->BitmapForFrame(frame_guid, clip_rect, scale_factor, - std::move(callback)); + paint_preview_compositor_client_->BitmapForFrame( + frame_guid, clip_rect, scale_factor, std::move(callback)); } -void PlayerCompositorDelegate::OnClick(uint64_t frame_guid, int x, int y) { - // TODO(crbug.com/1019883): Handle url clicks with the HitTester class. +std::vector<const GURL*> PlayerCompositorDelegate::OnClick( + const base::UnguessableToken& frame_guid, + const gfx::Rect& rect) { + std::vector<const GURL*> urls; + auto it = hit_testers_.find(frame_guid); + if (it != hit_testers_.end()) + it->second->HitTest(rect, &urls); + return urls; } -PlayerCompositorDelegate::~PlayerCompositorDelegate() = default; - } // namespace paint_preview diff --git a/chromium/components/paint_preview/player/player_compositor_delegate.h b/chromium/components/paint_preview/player/player_compositor_delegate.h index dc4cbf16cd8..9759331fe44 100644 --- a/chromium/components/paint_preview/player/player_compositor_delegate.h +++ b/chromium/components/paint_preview/player/player_compositor_delegate.h @@ -6,45 +6,76 @@ #define COMPONENTS_PAINT_PREVIEW_PLAYER_PLAYER_COMPOSITOR_DELEGATE_H_ #include "base/callback_forward.h" +#include "base/containers/flat_map.h" +#include "base/memory/weak_ptr.h" +#include "base/optional.h" +#include "base/unguessable_token.h" +#include "components/paint_preview/browser/hit_tester.h" +#include "components/paint_preview/browser/paint_preview_base_service.h" +#include "components/paint_preview/public/paint_preview_compositor_client.h" +#include "components/paint_preview/public/paint_preview_compositor_service.h" #include "components/services/paint_preview_compositor/public/mojom/paint_preview_compositor.mojom.h" #include "mojo/public/cpp/bindings/remote.h" namespace gfx { class Rect; -} +} // namespace gfx class SkBitmap; namespace paint_preview { +class DirectoryKey; + class PlayerCompositorDelegate { public: - PlayerCompositorDelegate(const GURL& url); + PlayerCompositorDelegate(PaintPreviewBaseService* paint_preview_service, + const GURL& url, + const DirectoryKey& key, + bool skip_service_launch = false); + virtual ~PlayerCompositorDelegate(); + + PlayerCompositorDelegate(const PlayerCompositorDelegate&) = delete; + PlayerCompositorDelegate& operator=(const PlayerCompositorDelegate&) = delete; virtual void OnCompositorReady( - const mojom::PaintPreviewBeginCompositeResponse& composite_response) = 0; + mojom::PaintPreviewCompositor::Status status, + mojom::PaintPreviewBeginCompositeResponsePtr composite_response) {} // Called when there is a request for a new bitmap. When the bitmap // is ready, it will be passed to callback. void RequestBitmap( - uint64_t frame_guid, + const base::UnguessableToken& frame_guid, const gfx::Rect& clip_rect, float scale_factor, base::OnceCallback<void(mojom::PaintPreviewCompositor::Status, const SkBitmap&)> callback); // Called on touch event on a frame. - void OnClick(uint64_t frame_guid, int x, int y); - - protected: - virtual ~PlayerCompositorDelegate(); + std::vector<const GURL*> OnClick(const base::UnguessableToken& frame_guid, + const gfx::Rect& rect); private: - // The current instance of PaintPreviewCompositor. - mojo::Remote<mojom::PaintPreviewCompositor> paint_preview_compositor_; + void OnCompositorServiceDisconnected(); - PlayerCompositorDelegate(const PlayerCompositorDelegate&) = delete; - PlayerCompositorDelegate& operator=(const PlayerCompositorDelegate&) = delete; + void OnCompositorClientCreated(const GURL& expected_url, + const DirectoryKey& key); + void OnCompositorClientDisconnected(); + + void OnProtoAvailable(const GURL& expected_url, + std::unique_ptr<PaintPreviewProto> proto); + void SendCompositeRequest( + mojom::PaintPreviewBeginCompositeRequestPtr begin_composite_request); + + PaintPreviewBaseService* paint_preview_service_; + std::unique_ptr<PaintPreviewCompositorService> + paint_preview_compositor_service_; + std::unique_ptr<PaintPreviewCompositorClient> + paint_preview_compositor_client_; + base::flat_map<base::UnguessableToken, std::unique_ptr<HitTester>> + hit_testers_; + + base::WeakPtrFactory<PlayerCompositorDelegate> weak_factory_{this}; }; } // namespace paint_preview diff --git a/chromium/components/paint_preview/player/player_compositor_delegate_unittest.cc b/chromium/components/paint_preview/player/player_compositor_delegate_unittest.cc new file mode 100644 index 00000000000..d37bd8c5473 --- /dev/null +++ b/chromium/components/paint_preview/player/player_compositor_delegate_unittest.cc @@ -0,0 +1,99 @@ +// 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/player/player_compositor_delegate.h" + +#include <utility> + +#include "base/callback.h" +#include "base/files/file_path.h" +#include "base/files/scoped_temp_dir.h" +#include "base/memory/scoped_refptr.h" +#include "base/run_loop.h" +#include "base/test/task_environment.h" +#include "base/unguessable_token.h" +#include "components/paint_preview/browser/directory_key.h" +#include "components/paint_preview/browser/file_manager.h" +#include "components/paint_preview/browser/paint_preview_base_service.h" +#include "components/paint_preview/common/proto/paint_preview.pb.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "ui/gfx/geometry/rect.h" +#include "url/gurl.h" + +namespace paint_preview { + +TEST(PlayerCompositorDelegate, OnClick) { + base::test::TaskEnvironment env; + base::ScopedTempDir temp_dir; + ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); + + PaintPreviewBaseService service(temp_dir.GetPath(), "test", nullptr, false); + auto file_manager = service.GetFileManager(); + auto key = file_manager->CreateKey(1U); + + GURL url("www.example.com"); + PaintPreviewProto proto; + proto.mutable_metadata()->set_url(url.spec()); + + GURL root_frame_link("www.chromium.org"); + auto root_frame_id = base::UnguessableToken::Create(); + + auto* root_frame = proto.mutable_root_frame(); + root_frame->set_embedding_token_high(root_frame_id.GetHighForSerialization()); + root_frame->set_embedding_token_low(root_frame_id.GetLowForSerialization()); + root_frame->set_is_main_frame(true); + auto* root_frame_link_proto = root_frame->add_links(); + root_frame_link_proto->set_url(root_frame_link.spec()); + auto* root_frame_rect_proto = root_frame_link_proto->mutable_rect(); + root_frame_rect_proto->set_x(10); + root_frame_rect_proto->set_y(20); + root_frame_rect_proto->set_width(30); + root_frame_rect_proto->set_height(40); + + GURL subframe_link("www.foo.com"); + auto subframe_id = base::UnguessableToken::Create(); + + auto* subframe = proto.add_subframes(); + subframe->set_embedding_token_high(subframe_id.GetHighForSerialization()); + subframe->set_embedding_token_low(subframe_id.GetLowForSerialization()); + subframe->set_is_main_frame(true); + auto* subframe_link_proto = subframe->add_links(); + subframe_link_proto->set_url(subframe_link.spec()); + auto* subframe_rect_proto = subframe_link_proto->mutable_rect(); + subframe_rect_proto->set_x(1); + subframe_rect_proto->set_y(2); + subframe_rect_proto->set_width(3); + subframe_rect_proto->set_height(4); + + base::RunLoop loop; + file_manager->GetTaskRunner()->PostTask( + FROM_HERE, + base::BindOnce( + [](base::OnceClosure quit, scoped_refptr<FileManager> file_manager, + const PaintPreviewProto& proto, const DirectoryKey& key) { + file_manager->CreateOrGetDirectory(key, true); + file_manager->SerializePaintPreviewProto(key, proto, false); + std::move(quit).Run(); + }, + loop.QuitClosure(), file_manager, proto, key)); + loop.Run(); + + PlayerCompositorDelegate player_compositor_delegate(&service, url, key, true); + env.RunUntilIdle(); + + auto res = player_compositor_delegate.OnClick(root_frame_id, + gfx::Rect(10, 20, 1, 1)); + ASSERT_EQ(res.size(), 1U); + EXPECT_EQ(*(res[0]), root_frame_link); + + res = + player_compositor_delegate.OnClick(root_frame_id, gfx::Rect(0, 0, 1, 1)); + EXPECT_TRUE(res.empty()); + + res = player_compositor_delegate.OnClick(subframe_id, gfx::Rect(1, 2, 1, 1)); + ASSERT_EQ(res.size(), 1U); + EXPECT_EQ(*(res[0]), subframe_link); +} + +} // namespace paint_preview diff --git a/chromium/components/paint_preview/public/BUILD.gn b/chromium/components/paint_preview/public/BUILD.gn new file mode 100644 index 00000000000..702371c04c9 --- /dev/null +++ b/chromium/components/paint_preview/public/BUILD.gn @@ -0,0 +1,20 @@ +# 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. + +assert(!is_ios, "Paint Previews are not supported on iOS.") + +source_set("public") { + sources = [ + "paint_preview_compositor_client.h", + "paint_preview_compositor_service.h", + ] + + public_deps = + [ "//components/services/paint_preview_compositor/public/mojom" ] + + deps = [ + "//base", + "//url", + ] +} diff --git a/chromium/components/paint_preview/public/DEPS b/chromium/components/paint_preview/public/DEPS new file mode 100644 index 00000000000..2349e2eaf97 --- /dev/null +++ b/chromium/components/paint_preview/public/DEPS @@ -0,0 +1,3 @@ +include_rules = [ + "+components/services/paint_preview_compositor/public/mojom", +] diff --git a/chromium/components/paint_preview/public/paint_preview_compositor_client.h b/chromium/components/paint_preview/public/paint_preview_compositor_client.h new file mode 100644 index 00000000000..8b782bceb5b --- /dev/null +++ b/chromium/components/paint_preview/public/paint_preview_compositor_client.h @@ -0,0 +1,56 @@ +// 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_PUBLIC_PAINT_PREVIEW_COMPOSITOR_CLIENT_H_ +#define COMPONENTS_PAINT_PREVIEW_PUBLIC_PAINT_PREVIEW_COMPOSITOR_CLIENT_H_ + +#include "base/callback_forward.h" +#include "base/optional.h" +#include "base/unguessable_token.h" +#include "components/services/paint_preview_compositor/public/mojom/paint_preview_compositor.mojom.h" +#include "url/gurl.h" + +namespace gfx { +class Rect; +} // namespace gfx + +namespace paint_preview { + +// An instance of a paint preview compositor that is running in a utility +// process service. The class' lifetime is tied to that of the compositor +// running in the utility process (unless there is some kind of IPC disconnect +// that occurs). +class PaintPreviewCompositorClient { + public: + virtual ~PaintPreviewCompositorClient() = default; + + // Returns the token associated with the client. Will be null if the client + // isn't started. + virtual const base::Optional<base::UnguessableToken>& Token() const = 0; + + // Adds |closure| as a disconnect handler. + virtual void SetDisconnectHandler(base::OnceClosure closure) = 0; + + // mojom::PaintPreviewCompositor API + virtual void BeginComposite( + mojom::PaintPreviewBeginCompositeRequestPtr request, + mojom::PaintPreviewCompositor::BeginCompositeCallback callback) = 0; + virtual void BitmapForFrame( + const base::UnguessableToken& frame_guid, + const gfx::Rect& clip_rect, + float scale_factor, + mojom::PaintPreviewCompositor::BitmapForFrameCallback callback) = 0; + virtual void SetRootFrameUrl(const GURL& url) = 0; + + PaintPreviewCompositorClient(const PaintPreviewCompositorClient&) = delete; + PaintPreviewCompositorClient& operator=(const PaintPreviewCompositorClient&) = + delete; + + protected: + PaintPreviewCompositorClient() = default; +}; + +} // namespace paint_preview + +#endif // COMPONENTS_PAINT_PREVIEW_PUBLIC_PAINT_PREVIEW_COMPOSITOR_CLIENT_H_ diff --git a/chromium/components/paint_preview/public/paint_preview_compositor_service.h b/chromium/components/paint_preview/public/paint_preview_compositor_service.h new file mode 100644 index 00000000000..a1dffb53ef6 --- /dev/null +++ b/chromium/components/paint_preview/public/paint_preview_compositor_service.h @@ -0,0 +1,46 @@ +// 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_PUBLIC_PAINT_PREVIEW_COMPOSITOR_SERVICE_H_ +#define COMPONENTS_PAINT_PREVIEW_PUBLIC_PAINT_PREVIEW_COMPOSITOR_SERVICE_H_ + +#include <memory> + +#include "base/callback_forward.h" +#include "components/paint_preview/public/paint_preview_compositor_client.h" + +namespace paint_preview { + +// An instance of a paint preview compositor utility process service. This +// class' lifetime is tied to that of the utility process to which it is +// connected. As long as the instance of this class is alive the utility process +// should be as well*. When this class is destructed then the utility process +// will also exit shortly. +// +// * If the utility process is killed by the OS or disconnected via Mojo then +// this isn't necessarily the case; however, in the general it is true. +class PaintPreviewCompositorService { + public: + virtual ~PaintPreviewCompositorService() = default; + + // Creates a compositor instance tied to the service. |connected_closure| is + // run once the compositor is started. + virtual std::unique_ptr<PaintPreviewCompositorClient> CreateCompositor( + base::OnceClosure connected_closure) = 0; + + // Returns whether there are any active clients. This can be used to + // check if killing this service is safe (i.e. won't drop messages). + virtual bool HasActiveClients() const = 0; + + PaintPreviewCompositorService(const PaintPreviewCompositorService&) = delete; + PaintPreviewCompositorService& operator=( + const PaintPreviewCompositorService&) = delete; + + protected: + PaintPreviewCompositorService() = default; +}; + +} // namespace paint_preview + +#endif // COMPONENTS_PAINT_PREVIEW_PUBLIC_PAINT_PREVIEW_COMPOSITOR_SERVICE_H_ diff --git a/chromium/components/paint_preview/renderer/BUILD.gn b/chromium/components/paint_preview/renderer/BUILD.gn index 86e15924aef..e262fc5a59c 100644 --- a/chromium/components/paint_preview/renderer/BUILD.gn +++ b/chromium/components/paint_preview/renderer/BUILD.gn @@ -31,9 +31,7 @@ if (!is_ios) { source_set("unit_tests") { testonly = true - sources = [ - "paint_preview_recorder_utils_unittest.cc", - ] + sources = [ "paint_preview_recorder_utils_unittest.cc" ] deps = [ ":renderer", 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 327d138904c..d18bfcc4a9b 100644 --- a/chromium/components/paint_preview/renderer/paint_preview_recorder_browsertest.cc +++ b/chromium/components/paint_preview/renderer/paint_preview_recorder_browsertest.cc @@ -78,6 +78,7 @@ TEST_F(PaintPreviewRecorderRenderViewTest, TestCaptureMainFrameAndClipping) { " background: yellow;'></div>" " </div>" "</body>"); + base::FilePath skp_path = MakeTestFilePath("test.skp"); mojom::PaintPreviewCaptureParamsPtr params = @@ -92,16 +93,14 @@ TEST_F(PaintPreviewRecorderRenderViewTest, TestCaptureMainFrameAndClipping) { auto out_response = mojom::PaintPreviewCaptureResponse::New(); content::RenderFrame* frame = GetFrame(); - int routing_id = frame->GetRoutingID(); PaintPreviewRecorderImpl paint_preview_recorder(frame); paint_preview_recorder.CapturePaintPreview( std::move(params), base::BindOnce(&OnCaptureFinished, mojom::PaintPreviewStatus::kOk, base::Unretained(&out_response))); - // Here id() is just the routing ID. - EXPECT_EQ(static_cast<int>(out_response->id), routing_id); - EXPECT_EQ(out_response->content_id_proxy_id_map.size(), 0U); + EXPECT_FALSE(out_response->embedding_token.has_value()); + EXPECT_EQ(out_response->content_id_to_embedding_token.size(), 0U); EXPECT_EQ(out_response->links.size(), 1U); EXPECT_EQ(out_response->links[0]->url, GURL("http://www.google.com/")); @@ -143,6 +142,64 @@ TEST_F(PaintPreviewRecorderRenderViewTest, TestCaptureMainFrameAndClipping) { 0xFFFFFFFFU); } +TEST_F(PaintPreviewRecorderRenderViewTest, TestCaptureMainFrameWithScroll) { + LoadHTML( + "<body>" + " <div style='width: 600px; height: 80vh; " + " background-color: #ff0000'> </div>" + " <div style='width: 600px; height: 1200px; " + " background-color: #00ff00'> </div>" + "</body>"); + + // Scroll to bottom of page to ensure scroll position has no effect on + // capture. + ExecuteJavaScriptForTests("window.scrollTo(0,document.body.scrollHeight);"); + base::FilePath skp_path = MakeTestFilePath("test.skp"); + + mojom::PaintPreviewCaptureParamsPtr params = + mojom::PaintPreviewCaptureParams::New(); + auto token = base::UnguessableToken::Create(); + params->guid = token; + params->clip_rect = gfx::Rect(); + params->is_main_frame = true; + base::File skp_file(skp_path, + base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE); + params->file = std::move(skp_file); + + auto out_response = mojom::PaintPreviewCaptureResponse::New(); + content::RenderFrame* frame = GetFrame(); + PaintPreviewRecorderImpl paint_preview_recorder(frame); + paint_preview_recorder.CapturePaintPreview( + std::move(params), + base::BindOnce(&OnCaptureFinished, mojom::PaintPreviewStatus::kOk, + base::Unretained(&out_response))); + + EXPECT_FALSE(out_response->embedding_token.has_value()); + EXPECT_EQ(out_response->content_id_to_embedding_token.size(), 0U); + + // Relaxed checks on dimensions and no checks on positions. This is not + // intended to intensively test the rendering behavior of the page. + 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); + } + SkBitmap bitmap; + ASSERT_TRUE(bitmap.tryAllocN32Pixels(pic->cullRect().width(), + pic->cullRect().height())); + SkCanvas canvas(bitmap); + canvas.drawPicture(pic); + // This should be inside the top right corner of the top div. Success means + // there was no horizontal or vertical clipping as this region is red, + // matching the div. + EXPECT_EQ(bitmap.getColor(600, 50), 0xFFFF0000U); + // This should be inside the bottom of the bottom div. Success means there was + // no vertical clipping as this region is blue matching the div. + EXPECT_EQ(bitmap.getColor(50, pic->cullRect().height() - 100), 0xFF00FF00U); +} + TEST_F(PaintPreviewRecorderRenderViewTest, TestCaptureFragment) { // Use position absolute position to check that the captured link dimensions // match what is specified. @@ -166,15 +223,14 @@ TEST_F(PaintPreviewRecorderRenderViewTest, TestCaptureFragment) { auto out_response = mojom::PaintPreviewCaptureResponse::New(); content::RenderFrame* frame = GetFrame(); - int routing_id = frame->GetRoutingID(); PaintPreviewRecorderImpl paint_preview_recorder(frame); paint_preview_recorder.CapturePaintPreview( std::move(params), base::BindOnce(&OnCaptureFinished, mojom::PaintPreviewStatus::kOk, &out_response)); - // Here id() is just the routing ID. - EXPECT_EQ(static_cast<int>(out_response->id), routing_id); - EXPECT_EQ(out_response->content_id_proxy_id_map.size(), 0U); + + EXPECT_FALSE(out_response->embedding_token.has_value()); + EXPECT_EQ(out_response->content_id_to_embedding_token.size(), 0U); EXPECT_EQ(out_response->links.size(), 1U); EXPECT_EQ(out_response->links[0]->url, GURL("fragment")); @@ -225,15 +281,13 @@ TEST_F(PaintPreviewRecorderRenderViewTest, TestCaptureMainFrameAndLocalFrame) { auto out_response = mojom::PaintPreviewCaptureResponse::New(); content::RenderFrame* frame = GetFrame(); - int routing_id = frame->GetRoutingID(); PaintPreviewRecorderImpl paint_preview_recorder(frame); paint_preview_recorder.CapturePaintPreview( std::move(params), base::BindOnce(&OnCaptureFinished, mojom::PaintPreviewStatus::kOk, base::Unretained(&out_response))); - // Here id() is just the routing ID. - EXPECT_EQ(static_cast<int>(out_response->id), routing_id); - EXPECT_EQ(out_response->content_id_proxy_id_map.size(), 0U); + EXPECT_FALSE(out_response->embedding_token.has_value()); + EXPECT_EQ(out_response->content_id_to_embedding_token.size(), 0U); } TEST_F(PaintPreviewRecorderRenderViewTest, TestCaptureLocalFrame) { @@ -259,15 +313,13 @@ TEST_F(PaintPreviewRecorderRenderViewTest, TestCaptureLocalFrame) { auto* child_frame = content::RenderFrame::FromWebFrame( GetFrame()->GetWebFrame()->FirstChild()->ToWebLocalFrame()); ASSERT_TRUE(child_frame); - int routing_id = child_frame->GetRoutingID(); PaintPreviewRecorderImpl paint_preview_recorder(child_frame); paint_preview_recorder.CapturePaintPreview( std::move(params), base::BindOnce(&OnCaptureFinished, mojom::PaintPreviewStatus::kOk, base::Unretained(&out_response))); - // Here id() is just the routing ID. - EXPECT_EQ(static_cast<int>(out_response->id), routing_id); - EXPECT_EQ(out_response->content_id_proxy_id_map.size(), 0U); + EXPECT_FALSE(out_response->embedding_token.has_value()); + EXPECT_EQ(out_response->content_id_to_embedding_token.size(), 0U); } } // namespace paint_preview 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 d807c8ec180..ebdc3aec4f6 100644 --- a/chromium/components/paint_preview/renderer/paint_preview_recorder_impl.cc +++ b/chromium/components/paint_preview/renderer/paint_preview_recorder_impl.cc @@ -7,7 +7,9 @@ #include <utility> #include "base/auto_reset.h" +#include "base/metrics/histogram_functions.h" #include "base/task_runner.h" +#include "base/time/time.h" #include "cc/paint/paint_record.h" #include "cc/paint/paint_recorder.h" #include "components/paint_preview/renderer/paint_preview_recorder_utils.h" @@ -40,8 +42,7 @@ PaintPreviewRecorderImpl::PaintPreviewRecorderImpl( content::RenderFrame* render_frame) : content::RenderFrameObserver(render_frame), is_painting_preview_(false), - is_main_frame_(render_frame->IsMainFrame()), - routing_id_(render_frame->GetRoutingID()) { + is_main_frame_(render_frame->IsMainFrame()) { render_frame->GetAssociatedInterfaceRegistry()->AddInterface( base::BindRepeating(&PaintPreviewRecorderImpl::BindPaintPreviewRecorder, weak_ptr_factory_.GetWeakPtr())); @@ -88,6 +89,12 @@ void PaintPreviewRecorderImpl::CapturePaintPreviewInternal( mojom::PaintPreviewCaptureResponse* response, mojom::PaintPreviewStatus* status) { blink::WebLocalFrame* frame = render_frame()->GetWebFrame(); + // Ensure the a frame actually exists to avoid a possible crash. + if (!frame) { + DVLOG(1) << "Error: renderer has no frame yet!"; + return; + } + // Warm up paint for an out-of-lifecycle paint phase. frame->DispatchBeforePrintEvent(); @@ -95,17 +102,56 @@ void PaintPreviewRecorderImpl::CapturePaintPreviewInternal( gfx::Rect bounds; if (is_main_frame_ || params->clip_rect == gfx::Rect(0, 0, 0, 0)) { auto size = frame->DocumentSize(); + + // |size| may be 0 if a tab is captured prior to layout finishing. This + // shouldn't occur often, if at all, in normal usage. However, this may + // occur during tests. Capturing prior to layout is non-sensical as the + // canvas size cannot be deremined so just abort. + if (size.height == 0 || size.width == 0) { + *status = mojom::PaintPreviewStatus::kCaptureFailed; + return; + } bounds = gfx::Rect(0, 0, size.width, size.height); } else { bounds = gfx::Rect(params->clip_rect.size()); } cc::PaintRecorder recorder; - PaintPreviewTracker tracker(params->guid, routing_id_, is_main_frame_); + PaintPreviewTracker tracker(params->guid, frame->GetEmbeddingToken(), + is_main_frame_); cc::PaintCanvas* canvas = recorder.beginRecording(bounds.width(), bounds.height()); canvas->SetPaintPreviewTracker(&tracker); + + // Use time ticks manually rather than a histogram macro so as to; + // 1. Account for main frames and subframes separately. + // 2. Mitigate binary size as this won't be used that often. + // 3. Record only on successes as failures are likely to be outliers (fast or + // slow). + base::TimeTicks start_time = base::TimeTicks::Now(); bool success = frame->CapturePaintPreview(bounds, canvas); + base::TimeDelta capture_time = base::TimeTicks::Now() - start_time; + response->blink_recording_time = capture_time; + + if (is_main_frame_) { + base::UmaHistogramBoolean("Renderer.PaintPreview.Capture.MainFrameSuccess", + success); + if (success) { + // Main frame should generally be the largest cost and will always run so + // it is tracked separately. + base::UmaHistogramTimes( + "Renderer.PaintPreview.Capture.MainFrameBlinkCaptureDuration", + capture_time); + } + } else { + base::UmaHistogramBoolean("Renderer.PaintPreview.Capture.SubframeSuccess", + success); + if (success) { + base::UmaHistogramTimes( + "Renderer.PaintPreview.Capture.SubframeBlinkCaptureDuration", + capture_time); + } + } // Restore to before out-of-lifecycle paint phase. frame->DispatchAfterPrintEvent(); diff --git a/chromium/components/paint_preview/renderer/paint_preview_recorder_impl.h b/chromium/components/paint_preview/renderer/paint_preview_recorder_impl.h index 34f231db542..60a935f4003 100644 --- a/chromium/components/paint_preview/renderer/paint_preview_recorder_impl.h +++ b/chromium/components/paint_preview/renderer/paint_preview_recorder_impl.h @@ -49,7 +49,6 @@ class PaintPreviewRecorderImpl : public content::RenderFrameObserver, bool is_painting_preview_; const bool is_main_frame_; - const int32_t routing_id_; mojo::AssociatedReceiver<mojom::PaintPreviewRecorder> paint_preview_recorder_receiver_{this}; 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 7c93b6f15af..de44d6bf57f 100644 --- a/chromium/components/paint_preview/renderer/paint_preview_recorder_utils.cc +++ b/chromium/components/paint_preview/renderer/paint_preview_recorder_utils.cc @@ -23,8 +23,7 @@ void ParseGlyphs(const cc::PaintOpBuffer* buffer, // Recurse into nested records if they contain text blobs (equivalent to // nested SkPictures). auto* record_op = static_cast<cc::DrawRecordOp*>(*it); - if (record_op->HasText()) - ParseGlyphs(record_op->record.get(), tracker); + ParseGlyphs(record_op->record.get(), tracker); } } } @@ -59,9 +58,10 @@ bool SerializeAsSkPicture(sk_sp<const cc::PaintRecord> record, void BuildResponse(PaintPreviewTracker* tracker, mojom::PaintPreviewCaptureResponse* response) { - response->id = tracker->RoutingId(); + response->embedding_token = tracker->EmbeddingToken(); for (const auto& id_pair : *(tracker->GetPictureSerializationContext())) { - response->content_id_proxy_id_map.insert({id_pair.first, id_pair.second}); + response->content_id_to_embedding_token.insert( + {id_pair.first, id_pair.second}); } for (const auto& link : tracker->GetLinks()) response->links.push_back(link.Clone()); diff --git a/chromium/components/paint_preview/renderer/paint_preview_recorder_utils_unittest.cc b/chromium/components/paint_preview/renderer/paint_preview_recorder_utils_unittest.cc index 51b783d17c2..9c84e0a7136 100644 --- a/chromium/components/paint_preview/renderer/paint_preview_recorder_utils_unittest.cc +++ b/chromium/components/paint_preview/renderer/paint_preview_recorder_utils_unittest.cc @@ -27,12 +27,6 @@ namespace paint_preview { -namespace { - -constexpr int32_t kRoutingId = 1; - -} // namespace - TEST(PaintPreviewServiceUtilsTest, TestParseGlyphs) { auto typeface = SkTypeface::MakeDefault(); SkFont font(typeface); @@ -51,8 +45,8 @@ TEST(PaintPreviewServiceUtilsTest, TestParseGlyphs) { outer_canvas->drawPicture(inner_recorder.finishRecordingAsPicture()); auto record = outer_recorder.finishRecordingAsPicture(); - PaintPreviewTracker tracker(base::UnguessableToken::Create(), kRoutingId, - true); + PaintPreviewTracker tracker(base::UnguessableToken::Create(), + base::UnguessableToken::Create(), true); ParseGlyphs(record.get(), &tracker); auto* usage_map = tracker.GetTypefaceUsageMap(); EXPECT_TRUE(usage_map->count(typeface->uniqueID())); @@ -71,8 +65,8 @@ TEST(PaintPreviewServiceUtilsTest, TestParseGlyphs) { } TEST(PaintPreviewServiceUtilsTest, TestSerializeAsSkPicture) { - PaintPreviewTracker tracker(base::UnguessableToken::Create(), kRoutingId, - true); + PaintPreviewTracker tracker(base::UnguessableToken::Create(), + base::UnguessableToken::Create(), true); gfx::Rect dimensions(100, 100); cc::PaintRecorder recorder; @@ -83,12 +77,12 @@ TEST(PaintPreviewServiceUtilsTest, TestSerializeAsSkPicture) { flags); base::flat_set<uint32_t> ctx; - uint32_t content_id = - tracker.CreateContentForRemoteFrame(gfx::Rect(10, 10), kRoutingId + 1); + uint32_t content_id = tracker.CreateContentForRemoteFrame( + gfx::Rect(10, 10), base::UnguessableToken::Create()); canvas->recordCustomData(content_id); ctx.insert(content_id); - content_id = - tracker.CreateContentForRemoteFrame(gfx::Rect(20, 20), kRoutingId + 2); + content_id = tracker.CreateContentForRemoteFrame( + gfx::Rect(20, 20), base::UnguessableToken::Create()); canvas->recordCustomData(content_id); ctx.insert(content_id); @@ -123,16 +117,19 @@ TEST(PaintPreviewServiceUtilsTest, TestSerializeAsSkPicture) { TEST(PaintPreviewServiceUtilsTest, TestBuildAndSerializeProto) { auto token = base::UnguessableToken::Create(); - PaintPreviewTracker tracker(token, kRoutingId, true); + auto embedding_token = base::UnguessableToken::Create(); + PaintPreviewTracker tracker(token, embedding_token, true); tracker.AnnotateLink(GURL("www.google.com"), gfx::Rect(1, 2, 3, 4)); tracker.AnnotateLink(GURL("www.chromium.org"), gfx::Rect(10, 20, 10, 20)); - tracker.CreateContentForRemoteFrame(gfx::Rect(1, 1, 1, 1), kRoutingId + 1); - tracker.CreateContentForRemoteFrame(gfx::Rect(1, 2, 4, 8), kRoutingId + 2); + tracker.CreateContentForRemoteFrame(gfx::Rect(1, 1, 1, 1), + base::UnguessableToken::Create()); + tracker.CreateContentForRemoteFrame(gfx::Rect(1, 2, 4, 8), + base::UnguessableToken::Create()); auto response = mojom::PaintPreviewCaptureResponse::New(); BuildResponse(&tracker, response.get()); - EXPECT_EQ(static_cast<int>(response->id), kRoutingId); + EXPECT_EQ(response->embedding_token, embedding_token); EXPECT_EQ(response->links.size(), 2U); EXPECT_EQ(response->links.size(), tracker.GetLinks().size()); for (size_t i = 0; i < response->links.size(); ++i) { @@ -140,7 +137,7 @@ TEST(PaintPreviewServiceUtilsTest, TestBuildAndSerializeProto) { EXPECT_THAT(response->links[i]->rect, tracker.GetLinks()[i].rect); } auto* content_map = tracker.GetPictureSerializationContext(); - for (const auto& id_pair : response->content_id_proxy_id_map) { + for (const auto& id_pair : response->content_id_to_embedding_token) { auto it = content_map->find(id_pair.first); EXPECT_NE(it, content_map->end()); EXPECT_EQ(id_pair.first, it->first); |