OOPIFs: Deduplicating MHTML parts across frames.

content/browser/download/mhtml_generation_manager.cc

Index: content/browser/download/mhtml_generation_manager.cc
diff --git a/content/browser/download/mhtml_generation_manager.cc b/content/browser/download/mhtml_generation_manager.cc
index c63405e93d55507e4e871dfa85c1ded7c295ae9f..f0c6606dbd1f54db5e6f85182a8248a6d1994e78 100644
--- a/content/browser/download/mhtml_generation_manager.cc
+++ b/content/browser/download/mhtml_generation_manager.cc
@@ -22,6 +22,7 @@
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_process_host_observer.h"
 #include "content/public/browser/web_contents.h"
+#include "url/gurl.h"
 
 namespace content {
 
@@ -36,6 +37,12 @@ class MHTMLGenerationManager::Job : public RenderProcessHostObserver {
 
   GenerateMHTMLCallback callback() const { return callback_; }
 
+  // Handler for FrameHostMsg_SerializeAsMHTMLResponse (a notification from the
+  // renderer that the MHTML generation for previous frame has finished).
+  // Returns |true| upon success; |false| otherwise.
+  bool OnSerializeAsMHTMLResponse(
+      const std::set<std::string>& digests_of_uris_of_serialized_resources);
+
   // Sends IPC to the renderer, asking for MHTML generation of the next frame.
   //
   // Returns true if the message was sent successfully; false otherwise.
@@ -84,6 +91,10 @@ class MHTMLGenerationManager::Job : public RenderProcessHostObserver {
   // MIME multipart boundary to use in the MHTML doc.
   std::string mhtml_boundary_marker_;
 
+  // Digests of URIs of already generated MHTML parts.
+  std::set<std::string> digests_of_already_serialized_uris_;
+  std::string salt_;
+
   // The callback to call once generation is complete.
   GenerateMHTMLCallback callback_;
 
@@ -99,6 +110,7 @@ MHTMLGenerationManager::Job::Job(int job_id,
                                  GenerateMHTMLCallback callback)
     : job_id_(job_id),
       mhtml_boundary_marker_(GenerateMHTMLBoundaryMarker()),
+      salt_(base::GenerateGUID()),
       callback_(callback),
       observed_renderer_process_host_(this) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -142,9 +154,13 @@ bool MHTMLGenerationManager::Job::SendToNextRenderFrame() {
   DCHECK(browser_file_.IsValid());
   DCHECK_LT(0u, pending_frame_tree_node_ids_.size());
 
+  FrameMsg_SerializeAsMHTML_Params ipc_params;
+  ipc_params.job_id = job_id_;
+  ipc_params.mhtml_boundary_marker = mhtml_boundary_marker_;
+
   int frame_tree_node_id = pending_frame_tree_node_ids_.front();
   pending_frame_tree_node_ids_.pop();
-  bool is_last_frame = pending_frame_tree_node_ids_.empty();
+  ipc_params.is_last_frame = pending_frame_tree_node_ids_.empty();
 
   FrameTreeNode* ftn = FrameTreeNode::GloballyFindByID(frame_tree_node_id);
   if (!ftn)  // The contents went away.
@@ -155,12 +171,16 @@ bool MHTMLGenerationManager::Job::SendToNextRenderFrame() {
   observed_renderer_process_host_.RemoveAll();
   observed_renderer_process_host_.Add(rfh->GetProcess());
 
-  IPC::PlatformFileForTransit renderer_file = IPC::GetFileHandleForProcess(
+  // Tell the renderer to skip (= deduplicate) already covered MHTML parts.
+  ipc_params.salt = salt_;
+  ipc_params.digests_of_uris_to_skip = digests_of_already_serialized_uris_;
+
+  ipc_params.destination_file = IPC::GetFileHandleForProcess(
       browser_file_.GetPlatformFile(), rfh->GetProcess()->GetHandle(),
       false);  // |close_source_handle|.
-  rfh->Send(new FrameMsg_SerializeAsMHTML(
-      rfh->GetRoutingID(), job_id_, renderer_file, mhtml_boundary_marker_,
-      CreateFrameRoutingIdToContentId(rfh->GetSiteInstance()), is_last_frame));
+  ipc_params.frame_routing_id_to_content_id =
+      CreateFrameRoutingIdToContentId(rfh->GetSiteInstance());
+  rfh->Send(new FrameMsg_SerializeAsMHTML(rfh->GetRoutingID(), ipc_params));
   return true;
 }
 
@@ -205,6 +225,18 @@ void MHTMLGenerationManager::Job::CloseFile(
       callback);
 }
 
+bool MHTMLGenerationManager::Job::OnSerializeAsMHTMLResponse(
+    const std::set<std::string>& digests_of_uris_of_serialized_resources) {
+  digests_of_already_serialized_uris_.insert(
+      digests_of_uris_of_serialized_resources.begin(),
+      digests_of_uris_of_serialized_resources.end());
+
+  if (!HasMoreFramesToProcess())
+    return true;  // Success.
+
+  return SendToNextRenderFrame();
+}
+
 // static
 int64 MHTMLGenerationManager::Job::CloseFileOnFileThread(base::File file) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
@@ -258,9 +290,10 @@ void MHTMLGenerationManager::SaveMHTML(WebContents* web_contents,
                  job_id));
 }
 
-void MHTMLGenerationManager::OnSavedFrameAsMHTML(
+void MHTMLGenerationManager::OnSerializeAsMHTMLResponse(
     int job_id,
-    bool mhtml_generation_in_renderer_succeeded) {
+    bool mhtml_generation_in_renderer_succeeded,
+    const std::set<std::string>& digests_of_uris_of_serialized_resources) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   if (!mhtml_generation_in_renderer_succeeded) {
@@ -272,14 +305,15 @@ void MHTMLGenerationManager::OnSavedFrameAsMHTML(
   if (!job)
     return;
 
-  if (job->HasMoreFramesToProcess()) {
-    if (!job->SendToNextRenderFrame()) {
-      JobFinished(job_id, JobStatus::FAILURE);
-    }
+  if (!job->HasMoreFramesToProcess()) {
+    JobFinished(job_id, JobStatus::SUCCESS);
     return;
   }
 
-  JobFinished(job_id, JobStatus::SUCCESS);
+  if (!job->OnSerializeAsMHTMLResponse(
+          digests_of_uris_of_serialized_resources)) {
+    JobFinished(job_id, JobStatus::FAILURE);
+  }

[Randy Smith (Not in Mondays), 2015/12/30 00:45:49]

nit, suggestion: The current implementation of OnSerializeAsMHTMLResponse
doesn't need to be called on the last response, but it strikes me as less future
proof; the name says that it'll be called on all responses, but it's actually
only called on all but the last one (IIUC).  Willing to restructure the code so
that it's always called and handles being the last frame correctly?

[Łukasz Anforowicz, 2015/12/30 19:09:09]

Done.

MHTMLGenerationManager::Job::OnSerializeAsMHTMLResponse already checks and
short-circuits HasMoreFramesToProcess.  OTOH, HasMoreFramesToProcess != IsDone. 
So, I've split these 2 methods.  This in turn forced me to think about tracking
pending responses - we can tighten IPC security by only accepting responses from
expected frames.

I've also added a new DCHECK to
MHTMLGenerationManager::Job::OnSerializeAsMHTMLResponse:

  // Renderer should be deduping resources with the same uris.
  DCHECK_EQ(0, STLSetIntersection<std::set<std::string>>(
      digests_of_already_serialized_uris_,
      digests_of_uris_of_serialized_resources).size());

 }
 
 // static