Print with dead subframes for pdf composition

Handle two cases with dead subframes for pdf composition:
-- When a web page has a dead subframe prior to printing, we need to
detect the liveness of the subframe, and avoid requesting printing for
such frame;
-- If after we request printing a subframe, the subframe dies, we add
monitoring for render frame's closed event to check whether it is one
of our pending ones. If so, notify pdf compositor service about that.

We add an interface in pdf compositor service to be notified about
the frame's unavailability for either of the above cases.

In this CL, we also add a map to record the subframes that are already
printed and use it to avoid printing the same ones repeatedly.

BUG=814086

Change-Id: Ibd69dd21a6498a5c2784dfe892bc5803e84fa6f0
Reviewed-on: https://chromium-review.googlesource.com/932018
Commit-Queue: Wei Li <weili@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539291}

chrome/browser/printing/print_browsertest.cc

@@ -100,6 +100,32 @@ class TestPrintFrameContentMsgFilter : public content::BrowserMessageFilter {
   base::RepeatingClosure msg_callback_;
 };
 
+class KillPrintFrameContentMsgFilter : public content::BrowserMessageFilter {
+ public:
+  explicit KillPrintFrameContentMsgFilter(content::RenderProcessHost* rph)
+      : content::BrowserMessageFilter(PrintMsgStart), rph_(rph) {}
+
+  bool OnMessageReceived(const IPC::Message& message) override {
+    // Only handle PrintHostMsg_DidPrintFrameContent message.
+    bool handled = true;
+    IPC_BEGIN_MESSAGE_MAP(KillPrintFrameContentMsgFilter, message)
+      IPC_MESSAGE_HANDLER(PrintHostMsg_DidPrintFrameContent, KillRenderProcess)
+      IPC_MESSAGE_UNHANDLED(handled = false)
+    IPC_END_MESSAGE_MAP()
+    return handled;
+  }
+
+ private:
+  ~KillPrintFrameContentMsgFilter() override {}
+
+  void KillRenderProcess(int document_cookie,
+                         const PrintHostMsg_DidPrintContent_Params& param) {
+    rph_->Shutdown(0, true);
+  }
+
+  content::RenderProcessHost* rph_;
+};
+
 }  // namespace
 
 class PrintBrowserTest : public InProcessBrowserTest {
@@ -169,6 +195,17 @@ class PrintBrowserTest : public InProcessBrowserTest {
   std::unique_ptr<base::RunLoop> run_loop_;
 };
 
+class SitePerProcessPrintBrowserTest : public PrintBrowserTest {
+ public:
+  SitePerProcessPrintBrowserTest() {}
+  ~SitePerProcessPrintBrowserTest() override {}
+
+  // content::BrowserTestBase
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    content::IsolateAllSitesForTesting(command_line);
+  }
+};
+
 // Printing only a selection containing iframes is partially supported.
 // Iframes aren't currently displayed. This test passes whenever the print
 // preview is rendered (i.e. no timeout in the test).
@@ -317,4 +354,58 @@ IN_PROC_BROWSER_TEST_F(PrintBrowserTest, PrintSubframeABA) {
   WaitUntilMessagesReceived();
 }
 
+// Printing a web page with a dead subframe for site per process should succeed.
+// This test passes whenever the print preview is rendered. This should not be
+// a timed out test which indicates the print preview hung.
+IN_PROC_BROWSER_TEST_F(SitePerProcessPrintBrowserTest,
+                       SubframeUnavailableBeforePrint) {
+  ASSERT_TRUE(embedded_test_server()->Started());
+  GURL url(
+      embedded_test_server()->GetURL("/printing/content_with_iframe.html"));
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  content::WebContents* original_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_EQ(2u, original_contents->GetAllFrames().size());
+  content::RenderFrameHost* test_frame = original_contents->GetAllFrames()[1];
+  ASSERT_TRUE(test_frame);
+  ASSERT_TRUE(test_frame->IsRenderFrameLive());
+  // Shutdown the subframe.
+  ASSERT_TRUE(test_frame->GetProcess()->Shutdown(0, true));
+  // Wait for the renderer to be down.
+  if (test_frame->IsRenderFrameLive()) {
+    content::RenderProcessHostWatcher render_process_watcher(
+        test_frame->GetProcess(),
+        content::RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
+    render_process_watcher.Wait();
+  }
+  ASSERT_FALSE(test_frame->IsRenderFrameLive());
+
+  PrintAndWaitUntilPreviewIsReady(/*print_only_selection=*/false);
+}
+
+// If a subframe dies during printing, the page printing should still succeed.
+// This test passes whenever the print preview is rendered. This should not be
+// a timed out test which indicates the print preview hung.
+IN_PROC_BROWSER_TEST_F(SitePerProcessPrintBrowserTest,
+                       SubframeUnavailableDuringPrint) {
+  ASSERT_TRUE(embedded_test_server()->Started());
+  GURL url(
+      embedded_test_server()->GetURL("/printing/content_with_iframe.html"));
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  content::WebContents* original_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_EQ(2u, original_contents->GetAllFrames().size());
+  content::RenderFrameHost* subframe = original_contents->GetAllFrames()[1];
+  ASSERT_TRUE(subframe);
+  auto* subframe_rph = subframe->GetProcess();
+
+  auto filter =
+      base::MakeRefCounted<KillPrintFrameContentMsgFilter>(subframe_rph);
+  subframe_rph->AddFilter(filter.get());
+
+  PrintAndWaitUntilPreviewIsReady(/*print_only_selection=*/false);
+}
+
 }  // namespace printing

components/printing/browser/print_composite_client.cc

@@ -8,6 +8,7 @@
 
 #include "base/bind.h"
 #include "base/memory/shared_memory_handle.h"
+#include "base/stl_util.h"
 #include "components/printing/common/print_messages.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_frame_host.h"
@@ -47,6 +48,22 @@ bool PrintCompositeClient::OnMessageReceived(
   return handled;
 }
 
+void PrintCompositeClient::RenderFrameDeleted(
+    content::RenderFrameHost* render_frame_host) {
+  auto frame_guid = GenerateFrameGuid(render_frame_host->GetProcess()->GetID(),
+                                      render_frame_host->GetRoutingID());
+  auto iter = pending_subframe_cookies_.find(frame_guid);
+  if (iter != pending_subframe_cookies_.end()) {
+    // When a subframe we are expecting is deleted, we should notify pdf
+    // compositor service.
+    for (auto doc_cookie : iter->second) {
+      auto& compositor = GetCompositeRequest(doc_cookie);
+      compositor->NotifyUnavailableSubframe(frame_guid);
+    }
+    pending_subframe_cookies_.erase(iter);
+  }
+}
+
 void PrintCompositeClient::OnDidPrintFrameContent(
     content::RenderFrameHost* render_frame_host,
     int document_cookie,
@@ -55,7 +72,6 @@ void PrintCompositeClient::OnDidPrintFrameContent(
   // is done here. Most of it will be directly forwarded to pdf compositor
   // service.
   auto& compositor = GetCompositeRequest(document_cookie);
-  DCHECK(compositor.is_bound());
 
   mojo::ScopedSharedBufferHandle buffer_handle = mojo::WrapSharedMemoryHandle(
       params.metafile_data_handle, params.data_size,
@@ -66,6 +82,12 @@ void PrintCompositeClient::OnDidPrintFrameContent(
       frame_guid, std::move(buffer_handle),
       ConvertContentInfoMap(web_contents(), render_frame_host,
                             params.subframe_content_info));
+
+  // Update our internal states about this frame.
+  pending_subframe_cookies_[frame_guid].erase(document_cookie);
+  if (pending_subframe_cookies_[frame_guid].empty())
+    pending_subframe_cookies_.erase(frame_guid);
+  printed_subframes_[document_cookie].insert(frame_guid);
 }
 
 void PrintCompositeClient::PrintCrossProcessSubframe(
@@ -75,9 +97,33 @@ void PrintCompositeClient::PrintCrossProcessSubframe(
   PrintMsg_PrintFrame_Params params;
   params.printable_area = rect;
   params.document_cookie = document_cookie;
-  // Send the request to the destination frame.
-  subframe_host->Send(
-      new PrintMsg_PrintFrameContent(subframe_host->GetRoutingID(), params));
+  uint64_t frame_guid = GenerateFrameGuid(subframe_host->GetProcess()->GetID(),
+                                          subframe_host->GetRoutingID());
+  if (subframe_host->IsRenderFrameLive()) {
+    auto subframe_iter = printed_subframes_.find(document_cookie);
+    if (subframe_iter != printed_subframes_.end() &&
+        base::ContainsKey(subframe_iter->second, frame_guid)) {
+      // If this frame is already printed, no need to print again.
+      return;
+    }
+
+    auto cookie_iter = pending_subframe_cookies_.find(frame_guid);
+    if (cookie_iter != pending_subframe_cookies_.end() &&
+        base::ContainsKey(cookie_iter->second, document_cookie)) {
+      // If this frame is being printed, no need to print again.
+      return;
+    }
+
+    // Send the request to the destination frame.
+    subframe_host->Send(
+        new PrintMsg_PrintFrameContent(subframe_host->GetRoutingID(), params));
+    pending_subframe_cookies_[frame_guid].insert(document_cookie);
+  } else {
+    // When the subframe is dead, no need to send message,
+    // just notify the service.
+    auto& compositor = GetCompositeRequest(document_cookie);
+    compositor->NotifyUnavailableSubframe(frame_guid);
+  }
 }
 
 void PrintCompositeClient::DoCompositePageToPdf(
@@ -149,6 +195,8 @@ void PrintCompositeClient::OnDidCompositeDocumentToPdf(
     printing::mojom::PdfCompositor::Status status,
     mojo::ScopedSharedBufferHandle handle) {
   RemoveCompositeRequest(document_cookie);
+  // Clear all stored printed subframes.
+  printed_subframes_.erase(document_cookie);
   std::move(callback).Run(status, std::move(handle));
 }
 
@@ -179,8 +227,10 @@ ContentToFrameMap PrintCompositeClient::ConvertContentInfoMap(
 
 mojom::PdfCompositorPtr& PrintCompositeClient::GetCompositeRequest(int cookie) {
   auto iter = compositor_map_.find(cookie);
-  if (iter != compositor_map_.end())
+  if (iter != compositor_map_.end()) {
+    DCHECK(iter->second.is_bound());
     return iter->second;
+  }
 
   auto iterator =
       compositor_map_.emplace(cookie, CreateCompositeRequest()).first;

components/printing/browser/print_composite_client.h

@@ -8,6 +8,7 @@
 #include <map>
 #include <memory>
 
+#include "base/containers/flat_set.h"
 #include "base/optional.h"
 #include "components/printing/service/public/cpp/pdf_service_mojo_types.h"
 #include "components/printing/service/public/interfaces/pdf_compositor.mojom.h"
@@ -35,6 +36,7 @@ class PrintCompositeClient
   // content::WebContentsObserver
   bool OnMessageReceived(const IPC::Message& message,
                          content::RenderFrameHost* render_frame_host) override;
+  void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override;
 
   // IPC message handler.
   void OnDidPrintFrameContent(
@@ -123,6 +125,14 @@ class PrintCompositeClient
   // requests.
   std::map<int, mojom::PdfCompositorPtr> compositor_map_;
 
+  // Stores the mapping between render frame's global unique id and document
+  // cookies that requested such frame.
+  std::map<uint64_t, base::flat_set<int>> pending_subframe_cookies_;
+
+  // Stores the mapping between document cookie and all the printed subframes
+  // for that document.
+  std::map<int, base::flat_set<uint64_t>> printed_subframes_;
+
   DISALLOW_COPY_AND_ASSIGN(PrintCompositeClient);
 };
 

components/printing/service/pdf_compositor_impl.cc

@@ -8,6 +8,7 @@
 
 #include "base/logging.h"
 #include "base/memory/shared_memory_handle.h"
+#include "base/stl_util.h"
 #include "components/printing/service/public/cpp/pdf_service_mojo_types.h"
 #include "components/printing/service/public/cpp/pdf_service_mojo_utils.h"
 #include "mojo/public/cpp/system/platform_handle.h"
@@ -26,6 +27,21 @@ PdfCompositorImpl::PdfCompositorImpl(
 
 PdfCompositorImpl::~PdfCompositorImpl() = default;
 
+void PdfCompositorImpl::NotifyUnavailableSubframe(uint64_t frame_guid) {
+  // Add this frame into the map.
+  DCHECK(!base::ContainsKey(frame_info_map_, frame_guid));
+  auto& frame_info =
+      frame_info_map_.emplace(frame_guid, std::make_unique<FrameInfo>())
+          .first->second;
+  frame_info->composited = true;
+  // Set content to be nullptr so it will be replaced by an empty picture during
+  // deserialization of its parent.
+  frame_info->content = nullptr;
+
+  // Update the requests in case any of them might be waiting for this frame.
+  UpdateRequestsWithSubframeInfo(frame_guid, std::vector<uint64_t>());
+}
+
 void PdfCompositorImpl::AddSubframeContent(
     uint64_t frame_guid,
     mojo::ScopedSharedBufferHandle serialized_content,
@@ -56,6 +72,33 @@ void PdfCompositorImpl::AddSubframeContent(
       pending_subframes.push_back(subframe_guid);
   }
 
+  // Update the requests in case any of them is waiting for this frame.
+  UpdateRequestsWithSubframeInfo(frame_guid, pending_subframes);
+}
+
+void PdfCompositorImpl::CompositePageToPdf(
+    uint64_t frame_guid,
+    uint32_t page_num,
+    mojo::ScopedSharedBufferHandle serialized_content,
+    const ContentToFrameMap& subframe_content_map,
+    mojom::PdfCompositor::CompositePageToPdfCallback callback) {
+  HandleCompositionRequest(frame_guid, page_num, std::move(serialized_content),
+                           subframe_content_map, std::move(callback));
+}
+
+void PdfCompositorImpl::CompositeDocumentToPdf(
+    uint64_t frame_guid,
+    mojo::ScopedSharedBufferHandle serialized_content,
+    const ContentToFrameMap& subframe_content_map,
+    mojom::PdfCompositor::CompositeDocumentToPdfCallback callback) {
+  HandleCompositionRequest(frame_guid, base::nullopt,
+                           std::move(serialized_content), subframe_content_map,
+                           std::move(callback));
+}
+
+void PdfCompositorImpl::UpdateRequestsWithSubframeInfo(
+    uint64_t frame_guid,
+    const std::vector<uint64_t>& pending_subframes) {
   // Check for each request's pending list.
   for (auto it = requests_.begin(); it != requests_.end();) {
     auto& request = *it;
@@ -81,26 +124,6 @@ void PdfCompositorImpl::AddSubframeContent(
   }
 }
 
-void PdfCompositorImpl::CompositePageToPdf(
-    uint64_t frame_guid,
-    uint32_t page_num,
-    mojo::ScopedSharedBufferHandle serialized_content,
-    const ContentToFrameMap& subframe_content_map,
-    mojom::PdfCompositor::CompositePageToPdfCallback callback) {
-  HandleCompositionRequest(frame_guid, page_num, std::move(serialized_content),
-                           subframe_content_map, std::move(callback));
-}
-
-void PdfCompositorImpl::CompositeDocumentToPdf(
-    uint64_t frame_guid,
-    mojo::ScopedSharedBufferHandle serialized_content,
-    const ContentToFrameMap& subframe_content_map,
-    mojom::PdfCompositor::CompositeDocumentToPdfCallback callback) {
-  HandleCompositionRequest(frame_guid, base::nullopt,
-                           std::move(serialized_content), subframe_content_map,
-                           std::move(callback));
-}
-
 bool PdfCompositorImpl::IsReadyToComposite(
     uint64_t frame_guid,
     const ContentToFrameMap& subframe_content_map,

components/printing/service/pdf_compositor_impl.h

@@ -33,6 +33,7 @@ class PdfCompositorImpl : public mojom::PdfCompositor {
   ~PdfCompositorImpl() override;
 
   // mojom::PdfCompositor
+  void NotifyUnavailableSubframe(uint64_t frame_guid) override;
   void AddSubframeContent(
       uint64_t frame_guid,
       mojo::ScopedSharedBufferHandle serialized_content,
@@ -127,6 +128,12 @@ class PdfCompositorImpl : public mojom::PdfCompositor {
     CompositeToPdfCallback callback;
   };
 
+  // Check whether any request is waiting for the specific subframe, if so,
+  // update its dependecy with the subframe's pending child frames.
+  void UpdateRequestsWithSubframeInfo(
+      uint64_t frame_guid,
+      const std::vector<uint64_t>& pending_subframes);
+
   // Check whether the frame with a list of subframe content is ready to
   // composite. If not, return all unavailable frames' ids in
   // |pending_subframes|.

components/printing/service/pdf_compositor_impl_unittest.cc

@@ -324,4 +324,25 @@ TEST_F(PdfCompositorImplTest, MultiRequestsDepOrder) {
                           std::move(subframe_content_map));
 }
 
+TEST_F(PdfCompositorImplTest, NotifyUnavailableSubframe) {
+  MockPdfCompositorImpl impl;
+  // Page 0 with frame 3 has content 1, which refers to frame 8.
+  // When the content is not available, the request is not fulfilled.
+  ContentToFrameMap subframe_content_map;
+  subframe_content_map[1u] = 8u;
+  EXPECT_CALL(impl, OnFulfillRequest(testing::_, testing::_)).Times(0);
+  impl.CompositePageToPdf(
+      3u, 0, mojo::SharedBufferHandle::Create(10),
+      std::move(subframe_content_map),
+      base::BindOnce(&PdfCompositorImplTest::OnCompositeToPdfCallback,
+                     base::Unretained(this)));
+  testing::Mock::VerifyAndClearExpectations(&impl);
+
+  // Notifies that frame 8's unavailable, the previous request should be
+  // fulfilled.
+  EXPECT_CALL(impl, OnFulfillRequest(3u, 0)).Times(1);
+  impl.NotifyUnavailableSubframe(8u);
+  testing::Mock::VerifyAndClearExpectations(&impl);
+}
+
 }  // namespace printing

components/printing/service/public/interfaces/pdf_compositor.mojom

@@ -15,6 +15,12 @@ interface PdfCompositor {
     COMPOSTING_FAILURE,
   };
 
+  // Notifies that a subframe is unavailable, such as the render frame process
+  // hosting it crashed or terminated. The subframe will be composited with no
+  // content in the composited result.
+  // |frame_guid| is this subframe's global unique id.
+  NotifyUnavailableSubframe(uint64 frame_guid);
+
   // Add the content of a subframe for composition.
   // |frame_guid| is this subframe's global unique id.
   // |serialized_content| points to a buffer of a serialized Skia picture which

printing/common/pdf_metafile_utils.cc

@@ -77,7 +77,7 @@ sk_sp<SkPicture> DeserializeOopPicture(const void* data,
 
   auto* context = reinterpret_cast<DeserializationContext*>(ctx);
   auto iter = context->find(pic_id);
-  if (iter == context->end()) {
+  if (iter == context->end() || !iter->second) {
     // When we don't have the out-of-process picture available, we return
     // an empty picture. Returning a nullptr will cause the deserialization
     // crash.