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..a4b93843c21ea0d997bb09378f824e72688bf027 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,14 +37,26 @@ 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(
+      int frame_tree_node_id_of_sending_frame,
+      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.
   bool SendToNextRenderFrame();
 
   // Indicates if more calls to SendToNextRenderFrame are needed.
-  bool HasMoreFramesToProcess() const {
-    return !pending_frame_tree_node_ids_.empty();
+  bool IsDone() const {
+    bool waiting_for_response_from_renderer =
+        frame_tree_node_id_of_busy_frame_ !=
+        FrameTreeNode::kFrameTreeNodeInvalidId;
+    bool no_more_requests_to_send = pending_frame_tree_node_ids_.empty();
+    bool done = !waiting_for_response_from_renderer && no_more_requests_to_send;

[nasko, 2015/12/30 19:22:26]

Why not "return !b1 && b2;"?

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

Done.

RE: Why:

Desire to spell "done" one more time and hoping that this will improve
readability?

+    return done;
   }
 
   // Close the file on the file thread and respond back on the UI thread with
@@ -71,12 +84,17 @@ class MHTMLGenerationManager::Job : public RenderProcessHostObserver {
   // See also MHTMLGenerationManager::id_to_job_ map.
   int job_id_;
 
-  // The handle to the file the MHTML is saved to for the browser process.
-  base::File browser_file_;
-
   // The IDs of frames we still need to process.

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

I tackled one of "we"-s in the comments, but didn't see an easy way out for the
remaining 2 occurrences below.  OTOH, this probably isn't a big enough deal to
precent CQ-ing this CL :-)

   std::queue<int> pending_frame_tree_node_ids_;
 
+  // Identifies a frame to which we've sent FrameMsg_SerializeAsMHTML but for
+  // which we didn't yet process FrameHostMsg_SerializeAsMHTMLResponse via
+  // OnSerializeAsMHTMLResponse.
+  int frame_tree_node_id_of_busy_frame_;
+
+  // The handle to the file the MHTML is saved to for the browser process.
+  base::File browser_file_;
+
   // Map from frames into content ids (see WebPageSerializer::generateMHTMLParts
   // for more details about what "content ids" are and how they are used).
   std::map<int, std::string> frame_tree_node_to_content_id_;
@@ -84,6 +102,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_;
 
@@ -98,7 +120,9 @@ MHTMLGenerationManager::Job::Job(int job_id,
                                  WebContents* web_contents,
                                  GenerateMHTMLCallback callback)
     : job_id_(job_id),
+      frame_tree_node_id_of_busy_frame_(FrameTreeNode::kFrameTreeNodeInvalidId),
       mhtml_boundary_marker_(GenerateMHTMLBoundaryMarker()),
+      salt_(base::GenerateGUID()),
       callback_(callback),
       observed_renderer_process_host_(this) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -142,9 +166,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 +183,21 @@ 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());
+
+  // Send the IPC asking the renderer to serialize the frame.
+  DCHECK_EQ(FrameTreeNode::kFrameTreeNodeInvalidId,
+            frame_tree_node_id_of_busy_frame_);
+  frame_tree_node_id_of_busy_frame_ = frame_tree_node_id;
+  rfh->Send(new FrameMsg_SerializeAsMHTML(rfh->GetRoutingID(), ipc_params));
   return true;
 }
 
@@ -205,6 +242,28 @@ void MHTMLGenerationManager::Job::CloseFile(
       callback);
 }
 
+bool MHTMLGenerationManager::Job::OnSerializeAsMHTMLResponse(
+    int frame_tree_node_id_of_sending_frame,
+    const std::set<std::string>& digests_of_uris_of_serialized_resources) {
+  // Sanitize renderer input / reject unexpected messages.
+  if (frame_tree_node_id_of_sending_frame != frame_tree_node_id_of_busy_frame_)
+    return false;  // Report failure.
+  frame_tree_node_id_of_busy_frame_ = FrameTreeNode::kFrameTreeNodeInvalidId;
+
+  // Renderer should be deduping resources with the same uris.
+  DCHECK_EQ(0u, base::STLSetIntersection<std::set<std::string>>(
+                    digests_of_already_serialized_uris_,
+                    digests_of_uris_of_serialized_resources).size());
+  digests_of_already_serialized_uris_.insert(
+      digests_of_uris_of_serialized_resources.begin(),
+      digests_of_uris_of_serialized_resources.end());
+
+  if (pending_frame_tree_node_ids_.empty())
+    return true;  // Report success.
+
+  return SendToNextRenderFrame();
+}
+
 // static
 int64 MHTMLGenerationManager::Job::CloseFileOnFileThread(base::File file) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
@@ -258,9 +317,11 @@ void MHTMLGenerationManager::SaveMHTML(WebContents* web_contents,
                  job_id));
 }
 
-void MHTMLGenerationManager::OnSavedFrameAsMHTML(
+void MHTMLGenerationManager::OnSerializeAsMHTMLResponse(
+    int frame_tree_node_id_of_sending_frame,
     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 +333,15 @@ void MHTMLGenerationManager::OnSavedFrameAsMHTML(
   if (!job)
     return;
 
-  if (job->HasMoreFramesToProcess()) {
-    if (!job->SendToNextRenderFrame()) {
-      JobFinished(job_id, JobStatus::FAILURE);
-    }
+  if (!job->OnSerializeAsMHTMLResponse(
+          frame_tree_node_id_of_sending_frame,
+          digests_of_uris_of_serialized_resources)) {
+    JobFinished(job_id, JobStatus::FAILURE);
     return;
   }
 
-  JobFinished(job_id, JobStatus::SUCCESS);
+  if (job->IsDone())
+    JobFinished(job_id, JobStatus::SUCCESS);
 }
 
 // static