Permit rejection of paint previews due to policy

With this change, the paint preview base service now can (optionally)
be passed a policy object which can disallow paint previews for which
it is not well suited.

Bug: 1026349
Change-Id: I2dc9707fe17ca076c95f7ebdf0d064a710bb3b9b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1986615
Commit-Queue: Ian Vollick <vollick@chromium.org>
Reviewed-by: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#728304}

components/paint_preview/browser/BUILD.gn

@@ -18,6 +18,7 @@ static_library("browser") {
     "paint_preview_base_service.h",
     "paint_preview_client.cc",
     "paint_preview_client.h",
+    "paint_preview_policy.h",
   ]
 
   deps = [

components/paint_preview/browser/paint_preview_base_service.cc

@@ -28,8 +28,10 @@ const char kPaintPreviewDir[] = "paint_preview";
 PaintPreviewBaseService::PaintPreviewBaseService(
     const base::FilePath& path,
     const std::string& ascii_feature_name,
+    std::unique_ptr<PaintPreviewPolicy> policy,
     bool is_off_the_record)
-    : file_manager_(
+    : policy_(std::move(policy)),
+      file_manager_(
           path.AppendASCII(kPaintPreviewDir).AppendASCII(ascii_feature_name)),
       is_off_the_record_(is_off_the_record) {}
 PaintPreviewBaseService::~PaintPreviewBaseService() = default;
@@ -49,6 +51,11 @@ void PaintPreviewBaseService::CapturePaintPreview(
     const base::FilePath& root_dir,
     gfx::Rect clip_rect,
     OnCapturedCallback callback) {
+  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) {

components/paint_preview/browser/paint_preview_base_service.h

@@ -12,6 +12,7 @@
 #include "base/macros.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/mojom/paint_preview_recorder.mojom.h"
 #include "components/paint_preview/common/proto/paint_preview.pb.h"
 #include "content/public/browser/web_contents.h"
@@ -33,6 +34,7 @@ class PaintPreviewBaseService : public KeyedService {
  public:
   enum CaptureStatus {
     kOk = 0,
+    kContentUnsupported,
     kClientCreationFailed,
     kCaptureFailed,
   };
@@ -44,9 +46,12 @@ class PaintPreviewBaseService : public KeyedService {
   // 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,
+                          std::unique_ptr<PaintPreviewPolicy> policy,
                           bool is_off_the_record);
   ~PaintPreviewBaseService() override;
 
@@ -95,6 +100,7 @@ class PaintPreviewBaseService : public KeyedService {
                   mojom::PaintPreviewStatus status,
                   std::unique_ptr<PaintPreviewProto> proto);
 
+  std::unique_ptr<PaintPreviewPolicy> policy_ = nullptr;
   FileManager file_manager_;
   bool is_off_the_record_;
 

components/paint_preview/browser/paint_preview_base_service_unittest.cc

@@ -27,10 +27,33 @@ namespace {
 
 const char kTestFeatureDir[] = "test_feature";
 
+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,
+      std::make_unique<RejectionPaintPreviewPolicy>(), key->IsOffTheRecord());
+}
+
 // Builds a PaintPreviewBaseService associated with |key|.
 std::unique_ptr<KeyedService> BuildService(SimpleFactoryKey* key) {
   return std::make_unique<PaintPreviewBaseService>(
-      key->GetPath(), kTestFeatureDir, key->IsOffTheRecord());
+      key->GetPath(), kTestFeatureDir, nullptr, key->IsOffTheRecord());
 }
 
 // Returns the GUID corresponding to |rfh|.
@@ -133,6 +156,12 @@ class PaintPreviewBaseServiceTest : public content::RenderViewHostTestHarness {
         browser_context()->GetPath(), browser_context()->IsOffTheRecord());
     PaintPreviewBaseServiceFactory::GetInstance()->SetTestingFactory(
         key_.get(), base::BindRepeating(&BuildService));
+
+    rejection_policy_key_ = std::make_unique<SimpleFactoryKey>(
+        browser_context()->GetPath(), browser_context()->IsOffTheRecord());
+    PaintPreviewBaseServiceFactory::GetInstance()->SetTestingFactory(
+        rejection_policy_key_.get(),
+        base::BindRepeating(&BuildServiceWithRejectionPolicy));
   }
 
   void OverrideInterface(MockPaintPreviewRecorder* service) {
@@ -148,8 +177,14 @@ class PaintPreviewBaseServiceTest : public content::RenderViewHostTestHarness {
     return PaintPreviewBaseServiceFactory::GetForKey(key_.get());
   }
 
+  PaintPreviewBaseService* CreateServiceWithRejectionPolicy() {
+    return PaintPreviewBaseServiceFactory::GetForKey(
+        rejection_policy_key_.get());
+  }
+
  private:
   std::unique_ptr<SimpleFactoryKey> key_ = nullptr;
+  std::unique_ptr<SimpleFactoryKey> rejection_policy_key_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(PaintPreviewBaseServiceTest);
 };
@@ -234,4 +269,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->id = main_rfh()->GetRoutingID();
+  recorder.SetResponse(mojom::PaintPreviewStatus::kFailed, std::move(response));
+  OverrideInterface(&recorder);
+
+  auto* service = CreateServiceWithRejectionPolicy();
+  EXPECT_FALSE(service->IsOffTheRecord());
+  base::FilePath path;
+  ASSERT_TRUE(service->GetFileManager()->CreateOrGetDirectoryFor(
+      web_contents()->GetLastCommittedURL(), &path));
+
+  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

components/paint_preview/browser/paint_preview_policy.h

+// 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_