[Paint Preview] Capture Local Frames Separately

Capturing OOPIFs separately allows them to scroll for the purposes of paint previews. However, on pages where iframes may not be out-of-process it would be ideal if we recorded them as separate SkPictures if they are scrollable to allow them to scroll and not become clipped. This is especially important in cases like AMP on mobile where, without strict site-isolation, a same-process iframe is the majority of the content view. However this iframe is clipped and cannot scroll due to how printing works normally. This can be fixed by capturing these iframes using the same mechanism as OOPIFs when capturing a Paint Preview. Bug: 1098300 Change-Id: Ic57c59bd2d358d96334ad892387e7e785f3c9df7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2194381 Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Reviewed-by: Ken Buchanan <kenrb@chromium.org> Reviewed-by: David Trainor <dtrainor@chromium.org> Reviewed-by: Philip Rogers <pdr@chromium.org> Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org> Cr-Commit-Position: refs/heads/master@{#785845}
chromium · Jul 7, 2020 · c43ac98 · c43ac98
1 parent c92098d
commit c43ac98
Show file tree

Hide file tree

Showing 22 changed files with 280 additions and 99 deletions.
diff --git a/chrome/browser/android/tab_web_contents_delegate_android.cc b/chrome/browser/android/tab_web_contents_delegate_android.cc
@@ -537,7 +537,7 @@ void TabWebContentsDelegateAndroid::PrintCrossProcessSubframe(
 #endif
 
 #if BUILDFLAG(ENABLE_PAINT_PREVIEW)
-void TabWebContentsDelegateAndroid::CapturePaintPreviewOfCrossProcessSubframe(
+void TabWebContentsDelegateAndroid::CapturePaintPreviewOfSubframe(
     content::WebContents* web_contents,
     const gfx::Rect& rect,
     const base::UnguessableToken& guid,

diff --git a/chrome/browser/android/tab_web_contents_delegate_android.h b/chrome/browser/android/tab_web_contents_delegate_android.h
@@ -132,7 +132,7 @@ class TabWebContentsDelegateAndroid
 #endif
 
 #if BUILDFLAG(ENABLE_PAINT_PREVIEW)
-  void CapturePaintPreviewOfCrossProcessSubframe(
+  void CapturePaintPreviewOfSubframe(
       content::WebContents* web_contents,
       const gfx::Rect& rect,
       const base::UnguessableToken& guid,

diff --git a/chrome/browser/paint_preview/paint_preview_browsertest.cc b/chrome/browser/paint_preview/paint_preview_browsertest.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/base64.h"
 #include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
@@ -28,6 +29,37 @@
 
 namespace paint_preview {
 
+namespace {
+
+base::FilePath ProtoPathToFilePath(const base::StringPiece& proto_path) {
+#if defined(OS_WIN)
+  return base::FilePath(base::UTF8ToUTF16(proto_path));
+#else
+  return base::FilePath(proto_path);
+#endif
+}
+
+// Check that |path| points to a valid SkPicture. Don't bother checking the
+// contents as this is non-trivial and could change. Instead check that
+// the SkPicture can be read correctly and has a cull rect of at least |size|.
+void EnsureSkPictureIsValid(const base::FilePath& path,
+                            size_t expected_subframe_count,
+                            const gfx::Size& size = gfx::Size(1, 1)) {
+  base::ScopedAllowBlockingForTesting scoped_blocking;
+  EXPECT_TRUE(base::PathExists(path));
+  FileRStream rstream(
+      base::File(path, base::File::FLAG_OPEN | base::File::FLAG_READ));
+  DeserializationContext ctx;
+  auto deserial_procs = MakeDeserialProcs(&ctx);
+  auto skp = SkPicture::MakeFromStream(&rstream, &deserial_procs);
+  EXPECT_NE(skp, nullptr);
+  EXPECT_GE(skp->cullRect().width(), 0);
+  EXPECT_GE(skp->cullRect().height(), 0);
+  EXPECT_EQ(ctx.size(), expected_subframe_count);
+}
+
+}  // namespace
+
 // Test harness for a integration test of paint previews. In this test:
 // - Each RenderFrame has an instance of PaintPreviewRecorder attached.
 // - Each WebContents has an instance of PaintPreviewClient attached.
@@ -63,10 +95,26 @@ class PaintPreviewBrowserTest : public InProcessBrowserTest {
     return browser()->tab_strip_model()->GetActiveWebContents();
   }
 
-  void LoadPage(const GURL& url) {
+  void LoadPage(const GURL& url) const {
     ui_test_utils::NavigateToURL(browser(), url);
   }
 
+  void LoadHtml(const base::StringPiece& html) const {
+    std::string base64_html;
+    base::Base64Encode(html, &base64_html);
+    GURL url(std::string("data:text/html;base64,") + base64_html);
+    ui_test_utils::NavigateToURL(browser(), url);
+  }
+
+  PaintPreviewClient::PaintPreviewParams MakeParams() const {
+    PaintPreviewClient::PaintPreviewParams params;
+    params.document_guid = base::UnguessableToken::Create();
+    params.is_main_frame = true;
+    params.root_dir = temp_dir_.GetPath();
+    params.max_per_capture_size = 0;
+    return params;
+  }
+
   void WaitForLoadStopWithoutSuccessCheck() {
     // In many cases, the load may have finished before we get here.  Only wait
     // if the tab still has a pending navigation.
@@ -92,13 +140,8 @@ class PaintPreviewBrowserTest : public InProcessBrowserTest {
 IN_PROC_BROWSER_TEST_F(PaintPreviewBrowserTest, CaptureFrame) {
   LoadPage(http_server_.GetURL("a.com", "/cross_site_iframe_factory.html?a"));
   ukm::TestAutoSetUkmRecorder ukm_recorder;
+  auto params = MakeParams();
 
-  base::UnguessableToken guid = base::UnguessableToken::Create();
-  PaintPreviewClient::PaintPreviewParams params;
-  params.document_guid = guid;
-  params.is_main_frame = true;
-  params.root_dir = temp_dir_.GetPath();
-  params.max_per_capture_size = 0;
   base::RunLoop loop;
 
   CreateClient();
@@ -117,44 +160,24 @@ IN_PROC_BROWSER_TEST_F(PaintPreviewBrowserTest, CaptureFrame) {
             EXPECT_EQ(proto->root_frame().content_id_to_embedding_tokens_size(),
                       0);
             EXPECT_TRUE(proto->root_frame().is_main_frame());
-#if defined(OS_WIN)
-            base::FilePath path = base::FilePath(
-                base::UTF8ToUTF16(proto->root_frame().file_path()));
-#else
-            base::FilePath path =
-                base::FilePath(proto->root_frame().file_path());
-#endif
-            {
-              base::ScopedAllowBlockingForTesting scoped_blocking;
-              EXPECT_TRUE(base::PathExists(path));
-              FileRStream rstream(base::File(
-                  path, base::File::FLAG_OPEN | base::File::FLAG_READ));
-              // Check that the result is a valid SkPicture. Don't bother
-              // checking the contents as this could change depending on how
-              // page rendering changes and is possibly unstable. nullptr is
-              // safe for serial procs as there are no iframes to deserialize.
-              EXPECT_NE(SkPicture::MakeFromStream(&rstream, nullptr), nullptr);
-            }
+            EnsureSkPictureIsValid(
+                ProtoPathToFilePath(proto->root_frame().file_path()), 0);
             quit.Run();
           },
-          loop.QuitClosure(), guid));
+          loop.QuitClosure(), params.document_guid));
   loop.Run();
 
   auto entries = ukm_recorder.GetEntriesByName(
       ukm::builders::PaintPreviewCapture::kEntryName);
   EXPECT_EQ(1u, entries.size());
 }
 
-IN_PROC_BROWSER_TEST_F(PaintPreviewBrowserTest, CaptureMainFrameWithSubframe) {
+IN_PROC_BROWSER_TEST_F(PaintPreviewBrowserTest,
+                       CaptureMainFrameWithCrossProcessSubframe) {
   LoadPage(
       http_server_.GetURL("a.com", "/cross_site_iframe_factory.html?a(b)"));
+  auto params = MakeParams();
 
-  base::UnguessableToken guid = base::UnguessableToken::Create();
-  PaintPreviewClient::PaintPreviewParams params;
-  params.document_guid = guid;
-  params.is_main_frame = true;
-  params.root_dir = temp_dir_.GetPath();
-  params.max_per_capture_size = 0;
   base::RunLoop loop;
 
   CreateClient();
@@ -176,43 +199,105 @@ IN_PROC_BROWSER_TEST_F(PaintPreviewBrowserTest, CaptureMainFrameWithSubframe) {
             EXPECT_EQ(proto->subframes(0).content_id_to_embedding_tokens_size(),
                       0);
             EXPECT_FALSE(proto->subframes(0).is_main_frame());
-#if defined(OS_WIN)
-            base::FilePath main_path = base::FilePath(
-                base::UTF8ToUTF16(proto->root_frame().file_path()));
-            base::FilePath subframe_path = base::FilePath(
-                base::UTF8ToUTF16(proto->subframes(0).file_path()));
-#else
-            base::FilePath main_path =
-                base::FilePath(proto->root_frame().file_path());
-            base::FilePath subframe_path =
-                base::FilePath(proto->subframes(0).file_path());
-#endif
-            {
-              base::ScopedAllowBlockingForTesting scoped_blocking;
-              EXPECT_TRUE(base::PathExists(main_path));
-              EXPECT_TRUE(base::PathExists(subframe_path));
-              FileRStream rstream_main(base::File(
-                  main_path, base::File::FLAG_OPEN | base::File::FLAG_READ));
-              // Check that the result is a valid SkPicture. Don't bother
-              // checking the contents as this could change depending on how
-              // page rendering changes and is possibly unstable.
-              DeserializationContext ctx;
-              auto deserial_procs = MakeDeserialProcs(&ctx);
-              EXPECT_NE(
-                  SkPicture::MakeFromStream(&rstream_main, &deserial_procs),
-                  nullptr);
-              FileRStream rstream_subframe(
-                  base::File(subframe_path,
-                             base::File::FLAG_OPEN | base::File::FLAG_READ));
-              ctx.clear();
-              deserial_procs = MakeDeserialProcs(&ctx);
-              EXPECT_NE(
-                  SkPicture::MakeFromStream(&rstream_subframe, &deserial_procs),
-                  nullptr);
-            }
+            EnsureSkPictureIsValid(
+                ProtoPathToFilePath(proto->root_frame().file_path()), 1);
+            EnsureSkPictureIsValid(
+                ProtoPathToFilePath(proto->subframes(0).file_path()), 0);
+            quit.Run();
+          },
+          loop.QuitClosure(), params.document_guid));
+  loop.Run();
+}
+
+IN_PROC_BROWSER_TEST_F(PaintPreviewBrowserTest,
+                       CaptureMainFrameWithScrollableSameProcessSubframe) {
+  std::string html = R"(<html>
+                          <iframe
+                            srcdoc="<div
+                                      style='width: 300px;
+                                             height: 300px;
+                                             background-color: #ff0000'>
+                                      &nbsp;
+                                    </div>"
+                            title="subframe"
+                            width="100px"
+                            height="100px">
+                          </iframe>
+                        </html>)";
+  LoadHtml(html);
+  auto params = MakeParams();
+
+  base::RunLoop loop;
+  CreateClient();
+  auto* client = PaintPreviewClient::FromWebContents(GetWebContents());
+  WaitForLoadStopWithoutSuccessCheck();
+  client->CapturePaintPreview(
+      params, GetWebContents()->GetMainFrame(),
+      base::BindOnce(
+          [](base::RepeatingClosure quit, base::UnguessableToken expected_guid,
+             base::UnguessableToken guid, mojom::PaintPreviewStatus status,
+             std::unique_ptr<PaintPreviewProto> proto) {
+            EXPECT_EQ(guid, expected_guid);
+            EXPECT_EQ(status, mojom::PaintPreviewStatus::kOk);
+            EXPECT_TRUE(proto->has_root_frame());
+            EXPECT_EQ(proto->subframes_size(), 1);
+            EXPECT_EQ(proto->root_frame().content_id_to_embedding_tokens_size(),
+                      1);
+            EXPECT_TRUE(proto->root_frame().is_main_frame());
+            EXPECT_EQ(proto->subframes(0).content_id_to_embedding_tokens_size(),
+                      0);
+            EXPECT_FALSE(proto->subframes(0).is_main_frame());
+            EnsureSkPictureIsValid(
+                ProtoPathToFilePath(proto->root_frame().file_path()), 1);
+            EnsureSkPictureIsValid(
+                ProtoPathToFilePath(proto->subframes(0).file_path()), 0,
+                gfx::Size(300, 300));
+            quit.Run();
+          },
+          loop.QuitClosure(), params.document_guid));
+  loop.Run();
+}
+
+IN_PROC_BROWSER_TEST_F(PaintPreviewBrowserTest,
+                       CaptureMainFrameWithNonScrollableSameProcessSubframe) {
+  std::string html = R"(<html>
+                          <iframe
+                            srcdoc="<div
+                                      style='width: 50px;
+                                             height: 50px;
+                                             background-color: #ff0000'>
+                                      &nbsp;
+                                    </div>"
+                            title="subframe"
+                            width="100px"
+                            height="100px">
+                          </iframe>
+                        </html>)";
+  LoadHtml(html);
+  auto params = MakeParams();
+
+  base::RunLoop loop;
+  CreateClient();
+  auto* client = PaintPreviewClient::FromWebContents(GetWebContents());
+  WaitForLoadStopWithoutSuccessCheck();
+  client->CapturePaintPreview(
+      params, GetWebContents()->GetMainFrame(),
+      base::BindOnce(
+          [](base::RepeatingClosure quit, base::UnguessableToken expected_guid,
+             base::UnguessableToken guid, mojom::PaintPreviewStatus status,
+             std::unique_ptr<PaintPreviewProto> proto) {
+            EXPECT_EQ(guid, expected_guid);
+            EXPECT_EQ(status, mojom::PaintPreviewStatus::kOk);
+            EXPECT_TRUE(proto->has_root_frame());
+            EXPECT_EQ(proto->subframes_size(), 0);
+            EXPECT_EQ(proto->root_frame().content_id_to_embedding_tokens_size(),
+                      0);
+            EXPECT_TRUE(proto->root_frame().is_main_frame());
+            EnsureSkPictureIsValid(
+                ProtoPathToFilePath(proto->root_frame().file_path()), 0);
             quit.Run();
           },
-          loop.QuitClosure(), guid));
+          loop.QuitClosure(), params.document_guid));
   loop.Run();
 }
 

diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
@@ -2166,7 +2166,7 @@ void Browser::PrintCrossProcessSubframe(
 #endif
 
 #if BUILDFLAG(ENABLE_PAINT_PREVIEW)
-void Browser::CapturePaintPreviewOfCrossProcessSubframe(
+void Browser::CapturePaintPreviewOfSubframe(
     content::WebContents* web_contents,
     const gfx::Rect& rect,
     const base::UnguessableToken& guid,

diff --git a/chrome/browser/ui/browser.h b/chrome/browser/ui/browser.h
@@ -853,7 +853,7 @@ class Browser : public TabStripModelObserver,
 #endif
 
 #if BUILDFLAG(ENABLE_PAINT_PREVIEW)
-  void CapturePaintPreviewOfCrossProcessSubframe(
+  void CapturePaintPreviewOfSubframe(
       content::WebContents* web_contents,
       const gfx::Rect& rect,
       const base::UnguessableToken& guid,

diff --git a/components/paint_preview/browser/paint_preview_client.cc b/components/paint_preview/browser/paint_preview_client.cc
@@ -218,6 +218,9 @@ mojom::PaintPreviewCaptureParamsPtr PaintPreviewClient::CreateMojoParams(
       mojom::PaintPreviewCaptureParams::New();
   mojo_params->guid = params.document_guid;
   mojo_params->clip_rect = params.clip_rect;
+  // For now treat all clip rects as hints only. This API should be exposed
+  // when clip_rects are used intentionally to limit capture time.
+  mojo_params->clip_rect_is_hint = true;
   mojo_params->is_main_frame = params.is_main_frame;
   mojo_params->file = std::move(file);
   mojo_params->max_capture_size = params.max_per_capture_size;

diff --git a/components/paint_preview/common/mojom/paint_preview_recorder.mojom b/components/paint_preview/common/mojom/paint_preview_recorder.mojom
@@ -44,6 +44,9 @@ struct PaintPreviewCaptureParams {
   // unclipped and will default to the frame (document) size.
   gfx.mojom.Rect clip_rect;
 
+  // Set to true if the clip rect is only a hint as to size.
+  bool clip_rect_is_hint;
+
   // Used to identify if the capture request is for the main frame.
   bool is_main_frame;
 

diff --git a/components/paint_preview/renderer/paint_preview_recorder_impl.cc b/components/paint_preview/renderer/paint_preview_recorder_impl.cc
@@ -137,22 +137,26 @@ void PaintPreviewRecorderImpl::CapturePaintPreviewInternal(
   frame->DispatchBeforePrintEvent();
 
   DCHECK_EQ(is_main_frame_, params->is_main_frame);
-  gfx::Rect bounds;
-  if (is_main_frame_ || params->clip_rect == gfx::Rect(0, 0, 0, 0)) {
+  // Default to using the clip rect.
+  gfx::Rect bounds = gfx::Rect(params->clip_rect.size());
+  if (bounds.IsEmpty() || params->clip_rect_is_hint) {
+    // If the clip rect is empty or only a hint try to use the document size.
     auto size = frame->DocumentSize();
-
-    // |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) {
+    gfx::Rect document_rect = gfx::Rect(0, 0, size.width, size.height);
+    if (!document_rect.IsEmpty())
+      bounds = document_rect;
+
+    if (bounds.IsEmpty()) {
+      // |bounds| may be empty if a capture is triggered prior to geometry
+      // being finalized and no clip rect was provided. If this happens there
+      // are no valid dimensions for the canvas and an abort is needed.
+      //
+      // This should only happen in tests or if a capture is triggered
+      // immediately after a navigation finished.
       std::move(callback).Run(mojom::PaintPreviewStatus::kCaptureFailed,
                               std::move(response));
       return;
     }
-    bounds = gfx::Rect(0, 0, size.width, size.height);
-  } else {
-    bounds = gfx::Rect(params->clip_rect.size());
   }
 
   auto tracker = std::make_unique<PaintPreviewTracker>(

diff --git a/content/browser/bad_message.h b/content/browser/bad_message.h
@@ -255,6 +255,7 @@ enum BadMessageReason {
   RFH_INVALID_CALL_FROM_NOT_MAIN_FRAME = 227,
   INPUT_ROUTER_INVALID_EVENT_SOURCE = 228,
   RFH_INACTIVE_CHECK_FROM_SPECULATIVE_RFH = 229,
+  RFH_SUBFRAME_CAPTURE_ON_MAIN_FRAME = 230,
 
   // Please add new elements here. The naming convention is abbreviated class
   // name (e.g. RenderFrameHost becomes RFH) plus a unique description of the