OOPIFs: Deduplicating MHTML parts across frames.

content/renderer/render_frame_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/render_frame_impl.h"
 
 #include <map>
 #include <string>
 #include <vector>
 
@@ skipping 107 matching lines @@
 #include "content/renderer/savable_resources.h"
 #include "content/renderer/screen_orientation/screen_orientation_dispatcher.h"
 #include "content/renderer/shared_worker_repository.h"
 #include "content/renderer/skia_benchmarking_extension.h"
 #include "content/renderer/stats_collection_controller.h"
 #include "content/renderer/usb/web_usb_client_impl.h"
 #include "content/renderer/wake_lock/wake_lock_dispatcher.h"
 #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"
 #include "media/base/media_log.h"
 #include "media/base/media_switches.h"
 #include "media/blink/webencryptedmediaclient_impl.h"
 #include "media/blink/webmediaplayer_impl.h"
 #include "media/renderers/gpu_video_accelerator_factories.h"
 #include "mojo/common/url_type_converters.h"
 #include "net/base/data_url.h"
@@ skipping 4515 matching lines @@
         WebURL(url), ConvertRelativePathToHtmlAttribute(local_path)));
   }
 
   // Serialize the frame (without recursing into subframes).
   WebPageSerializer::serialize(GetWebFrame(),
                                this,  // WebPageSerializerClient.
                                weburl_to_local_path);
 }
 
 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);
-  DCHECK(!mhtml_boundary.isEmpty());
+  base::File file = IPC::PlatformFileForTransitToFile(params.destination_file);
+  WebString mhtml_boundary = WebString::fromUTF8(params.mhtml_boundary_marker);
   std::vector<std::pair<WebFrame*, WebString>> web_frame_to_content_id;
-  for (const auto& it : frame_routing_id_to_content_id) {
+  for (const auto& it : params.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)));
   }
+  const std::string& salt = params.salt;
 
   WebData data;
   bool success = true;
+  std::set<GURL> uris_of_generated_mhtml_parts;
 
   // Generate MHTML header if needed.
-  if (IsMainFrame()) {
+  if (success && IsMainFrame()) {

[dcheng, 2015/12/11 07:36:05]

Revert this.

[dcheng, 2015/12/11 07:36:05]

Please revert this =)

[Łukasz Anforowicz, 2015/12/14 19:39:02]

Ooops.  Done (here and for the DCHECK below).

     data =
         WebPageSerializer::generateMHTMLHeader(mhtml_boundary, GetWebFrame());
     if (file.WriteAtCurrentPos(data.data(), data.size()) < 0) {
       success = false;
     }
   }
+  DCHECK(!mhtml_boundary.isEmpty());
 
   // Generate MHTML parts.
   if (success) {
-    data = WebPageSerializer::generateMHTMLParts(
-        mhtml_boundary, GetWebFrame(), false, web_frame_to_content_id);
+    std::vector<WebURL> uris_to_skip;
+    WebVector<WebURL> all_urls =
+        WebPageSerializer::enumerateMHTMLResources(GetWebFrame());

[dcheng, 2015/12/11 07:36:05]

I think I would prefer to see a new WebPageSerializerClient callback rather than
enumerating all the resources, hashing to figure it out, and then re-iterate
over all the resources.

[Łukasz Anforowicz, 2015/12/14 19:39:02]

Let me think a little bit more about this.

Some random notes:

- WebPageSerializerClient doesn't quite fit (it reports binary blobs for
Complete-Html case and this is something we don't need/do for MHTML case).

- PageSerializer::Delegate is hidden below Blink's public API layer

- std::function is on a yet-to-be-discussed list at
https://chromium-cpp.appspot.com/

- I am not quite happy with the serialize-twice approach;  it just seemed like
the simplest (and still quite clean I think) thing that gets the job done.

[Łukasz Anforowicz, 2015/12/18 17:38:48]

Some more ideas:

- Stupid idea: Pass a "callback" (bound to a "lambda" in render_frame_impl.cc)? 
"Lambda" in quotes, because lambdas are not allowed, but I can write a
lambda-like object (closure that closes over the set of digests to skip)
manually.  "Callback" in quotes, because AFAIK base::Callback and std::function
are not allowed in Blink... :-/

- Maybe a good idea: Have a boolean enumerateOnly flag that
WebPageSerializer::enumerateMHTMLResources passes to the constructor of
PageSerializer and which causes PageSerializer to populate the vector of
SerializedResource, but without populating SerializedResource::data. 

  - Pros:
    - The first traversal doesn't descend into subresources at all (only
enumerates their urls).

  - Cons:
    - Still 2 traversals of the frame's html dom.
    - Second traversal still needlessly serializes some resources that will be
skipped at WebPageSerializer::generateMHTMLParts level.  This can be solved by
passing the set-of-urls-to-skip from WebPageSerializer::generateMHTMLParts into
PageSerializer.

WDYT?  Should I try coding any of these?  Any other ideas that I missed?  Or is
the current code "not just good, but good enough!" :-) ?

+    for (const WebURL url : all_urls) {
+      std::string digest = crypto::SHA256HashString(salt + GURL(url).spec());
+      if (0 != params.digests_of_uris_to_skip.count(digest))
+        uris_to_skip.push_back(url);
+      else
+        uris_of_generated_mhtml_parts.insert(url);
+    }
+
+    data = WebPageSerializer::generateMHTMLParts(mhtml_boundary, GetWebFrame(),
+                                                 false, web_frame_to_content_id,
+                                                 uris_to_skip);
     // 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) {
       success = false;
     }
   }
 
   // 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;
     }
   }
 
   // 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_generated_mhtml_parts));
 }
 
 void RenderFrameImpl::OpenURL(const GURL& url,
                               const Referrer& referrer,
                               WebNavigationPolicy policy,
                               bool should_replace_current_entry,
                               bool is_history_navigation_in_new_child) {
   FrameHostMsg_OpenURL_Params params;
   params.url = url;
   params.referrer = referrer;
@@ skipping 792 matching lines @@
       media::ConvertToSwitchOutputDeviceCB(web_callbacks);
   scoped_refptr<media::AudioOutputDevice> device =
       AudioDeviceFactory::NewOutputDevice(routing_id_, 0, sink_id.utf8(),
                                           security_origin);
   media::OutputDeviceStatus status = device->GetDeviceStatus();
   device->Stop();
   callback.Run(status);
 }
 
 }  // namespace content