OOPIFs: Deduplicating MHTML parts across frames.

content/renderer/render_frame_impl.cc

Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 9e01406593394ecf970e84632b087772fde715ad..04944817cd39c4d36dc98a3950cfd1fa5ab00072 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -126,6 +126,7 @@
 #include "content/renderer/web_frame_utils.h"
 #include "content/renderer/web_ui_extension.h"
 #include "content/renderer/websharedworker_proxy.h"
+#include "crypto/sha2.h"
 #include "gin/modules/module_registry.h"
 #include "media/audio/audio_output_device.h"
 #include "media/base/audio_renderer_mixer_input.h"
@@ -581,6 +582,45 @@ WebString ConvertRelativePathToHtmlAttribute(const base::FilePath& path) {
       path.NormalizePathSeparatorsTo(FILE_PATH_LITERAL('/')).AsUTF8Unsafe());
 }
 
+// Implementation of WebPageSerializer::MHTMLPartsGenerationDelegate that
+// 1. Bases shouldSkipResource and getContentID responses on contents of
+//    FrameMsg_SerializeAsMHTML_Params.
+// 2. Stores urls of serialized resources (reported via shouldSkipResource)
+//    into |uris_of_serialized_resources| passed to the constructor.
+class MHTMLPartsGenerationDelegate
+    : public WebPageSerializer::MHTMLPartsGenerationDelegate {
+ public:
+  MHTMLPartsGenerationDelegate(const FrameMsg_SerializeAsMHTML_Params& params,
+                               std::set<GURL>* uris_of_serialized_resources_)
+      : params_(params),
+        uris_of_serialized_resources_(uris_of_serialized_resources_) {
+    DCHECK(uris_of_serialized_resources_);
+  }
+
+  bool shouldSkipResource(const WebURL& url) override {
+    std::string digest =
+        crypto::SHA256HashString(params_.salt + GURL(url).spec());
+
+    if (0 != params_.digests_of_uris_to_skip.count(digest))

[dcheng, 2015/12/22 01:01:45]

ContainsKey? Or find()? Either seems to be more commonly used than count(). I
don't feel strongly about this, it's just a more unusual pattern.

[Łukasz Anforowicz, 2015/12/22 21:06:46]

Thanks for pointing out ContainsKey.  Done.

Side-comment: I am a bit surprised that ContainsKey is not in the global, rather
than the base namespace.

Also - I've also used ContainsKey in 2 places in save_package.cc

+      return true;
+
+    uris_of_serialized_resources_->insert(url);

[dcheng, 2015/12/22 01:01:45]

Any particular reason not to just send the hashes back and forth?

Right now, the browser process has to recalculate.

[Łukasz Anforowicz, 2015/12/22 21:06:46]

Done - I changed the code to send digests (instead of uris) in the IPC response.

BTW: In my previous iteration I changed code to avoid recalculating digests
(i.e. to only calculate the digest once) in the browser process.  I guess I
misunderstood the suggestion :-)

+    return false;
+  }
+
+  WebString getContentID(const WebFrame& frame) override {
+    int routing_id = GetRoutingIdForFrameOrProxy(const_cast<WebFrame*>(&frame));
+    auto it = params_.frame_routing_id_to_content_id.find(routing_id);
+    DCHECK(it != params_.frame_routing_id_to_content_id.end());
+    const std::string& content_id = it->second;
+    return WebString::fromUTF8(content_id);
+  }
+
+ private:
+  const FrameMsg_SerializeAsMHTML_Params& params_;
+  std::set<GURL>* uris_of_serialized_resources_;
+};

[dcheng, 2015/12/22 01:01:45]

Nit: DISALLOW_COPY_AND_ASSIGN

[Łukasz Anforowicz, 2015/12/22 21:06:46]

Oh, right.  Done.

+
 bool IsContentWithCertificateErrorsRelevantToUI(
     const blink::WebURL& url,
     const blink::WebCString& security_info,
@@ -4778,28 +4818,17 @@ void RenderFrameImpl::OnGetSerializedHtmlWithLocalLinks(
 }
 
 void RenderFrameImpl::OnSerializeAsMHTML(
-    int job_id,
-    IPC::PlatformFileForTransit file_for_transit,
-    const std::string& std_mhtml_boundary,
-    const std::map<int, std::string>& frame_routing_id_to_content_id,
-    bool is_last_frame) {
+    const FrameMsg_SerializeAsMHTML_Params& params) {
   // Unpack IPC payload.
-  base::File file = IPC::PlatformFileForTransitToFile(file_for_transit);
-  const WebString mhtml_boundary = WebString::fromUTF8(std_mhtml_boundary);
+  base::File file = IPC::PlatformFileForTransitToFile(params.destination_file);
+  const WebString mhtml_boundary =
+      WebString::fromUTF8(params.mhtml_boundary_marker);
   DCHECK(!mhtml_boundary.isEmpty());
-  std::vector<std::pair<WebFrame*, WebString>> web_frame_to_content_id;
-  for (const auto& it : frame_routing_id_to_content_id) {
-    const std::string& content_id = it.second;
-    WebFrame* web_frame = GetWebFrameFromRoutingIdForFrameOrProxy(it.first);
-    if (!web_frame)
-      continue;
-
-    web_frame_to_content_id.push_back(
-        std::make_pair(web_frame, WebString::fromUTF8(content_id)));
-  }
 
   WebData data;
   bool success = true;
+  std::set<GURL> uris_of_serialized_resources;
+  MHTMLPartsGenerationDelegate delegate(params, &uris_of_serialized_resources);
 
   // Generate MHTML header if needed.
   if (IsMainFrame()) {
@@ -4812,8 +4841,8 @@ void RenderFrameImpl::OnSerializeAsMHTML(
 
   // Generate MHTML parts.
   if (success) {
-    data = WebPageSerializer::generateMHTMLParts(
-        mhtml_boundary, GetWebFrame(), false, web_frame_to_content_id);
+    data = WebPageSerializer::generateMHTMLParts(mhtml_boundary, GetWebFrame(),
+                                                 false, delegate);
     // TODO(jcivelli): write the chunks in deferred tasks to give a chance to
     //                 the message loop to process other events.
     if (file.WriteAtCurrentPos(data.data(), data.size()) < 0) {
@@ -4822,7 +4851,7 @@ void RenderFrameImpl::OnSerializeAsMHTML(
   }
 
   // Generate MHTML footer if needed.
-  if (success && is_last_frame) {
+  if (success && params.is_last_frame) {
     data = WebPageSerializer::generateMHTMLFooter(mhtml_boundary);
     if (file.WriteAtCurrentPos(data.data(), data.size()) < 0) {
       success = false;
@@ -4831,7 +4860,8 @@ void RenderFrameImpl::OnSerializeAsMHTML(
 
   // Cleanup and notify the browser process about completion.
   file.Close();  // Need to flush file contents before sending IPC response.
-  Send(new FrameHostMsg_SerializeAsMHTMLResponse(routing_id_, job_id, success));
+  Send(new FrameHostMsg_SerializeAsMHTMLResponse(
+      routing_id_, params.job_id, success, uris_of_serialized_resources));
 }
 
 void RenderFrameImpl::OpenURL(const GURL& url,