Move MHTML file writing out of the renderer main thread.

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 <utility>
 #include <vector>
 
 #include "base/auto_reset.h"
+#include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/debug/alias.h"
 #include "base/debug/asan_invalid_access.h"
 #include "base/debug/crash_logging.h"
 #include "base/debug/dump_without_crashing.h"
 #include "base/files/file.h"
 #include "base/i18n/char_iterator.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/shared_memory.h"
 #include "base/memory/weak_ptr.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/process/process.h"
 #include "base/stl_util.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/task_runner_util.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "base/trace_event/trace_event.h"
 #include "build/build_config.h"
 #include "cc/base/switches.h"
 #include "content/child/appcache/appcache_dispatcher.h"
 #include "content/child/quota_dispatcher.h"
 #include "content/child/request_extra_data.h"
 #include "content/child/service_worker/service_worker_handle_reference.h"
 #include "content/child/service_worker/service_worker_network_provider.h"
@@ skipping 681 matching lines @@
 
  private:
   const std::map<GURL, base::FilePath>& url_to_local_path_;
   const std::map<int, base::FilePath>& frame_routing_id_to_local_path_;
 };
 
 // Implementation of WebFrameSerializer::MHTMLPartsGenerationDelegate that
 // 1. Bases shouldSkipResource and getContentID responses on contents of
 //    FrameMsg_SerializeAsMHTML_Params.
 // 2. Stores digests of urls of serialized resources (i.e. urls reported via
-//    shouldSkipResource) into |digests_of_uris_of_serialized_resources| passed
+//    shouldSkipResource) into |serialized_resources_uri_digests| passed
 //    to the constructor.
 class MHTMLPartsGenerationDelegate
     : public WebFrameSerializer::MHTMLPartsGenerationDelegate {
  public:
   MHTMLPartsGenerationDelegate(
       const FrameMsg_SerializeAsMHTML_Params& params,
-      std::set<std::string>* digests_of_uris_of_serialized_resources)
+      std::set<std::string>* serialized_resources_uri_digests)
       : params_(params),
-        digests_of_uris_of_serialized_resources_(
-            digests_of_uris_of_serialized_resources) {
-    DCHECK(digests_of_uris_of_serialized_resources_);
+        serialized_resources_uri_digests_(serialized_resources_uri_digests) {
+    DCHECK(serialized_resources_uri_digests_);
   }
 
   bool shouldSkipResource(const WebURL& url) override {
     std::string digest =
         crypto::SHA256HashString(params_.salt + GURL(url).spec());
 
     // Skip if the |url| already covered by serialization of an *earlier* frame.
     if (base::ContainsKey(params_.digests_of_uris_to_skip, digest))
       return true;
 
     // Let's record |url| as being serialized for the *current* frame.
-    auto pair = digests_of_uris_of_serialized_resources_->insert(digest);
+    auto pair = serialized_resources_uri_digests_->insert(digest);
     bool insertion_took_place = pair.second;
     DCHECK(insertion_took_place);  // Blink should dedupe within a frame.
 
     return false;
   }
 
   WebString getContentID(WebFrame* frame) override {
     int routing_id = GetRoutingIdForFrameOrProxy(frame);
 
     auto it = params_.frame_routing_id_to_content_id.find(routing_id);
     if (it == params_.frame_routing_id_to_content_id.end())
       return WebString();
 
     const std::string& content_id = it->second;
     return WebString::fromUTF8(content_id);
   }
 
   blink::WebFrameSerializerCacheControlPolicy cacheControlPolicy() override {
     return params_.mhtml_cache_control_policy;
   }
 
   bool useBinaryEncoding() override { return params_.mhtml_binary_encoding; }
 
  private:
   const FrameMsg_SerializeAsMHTML_Params& params_;
-  std::set<std::string>* digests_of_uris_of_serialized_resources_;
+  std::set<std::string>* serialized_resources_uri_digests_;
 
   DISALLOW_COPY_AND_ASSIGN(MHTMLPartsGenerationDelegate);
 };
 
+bool WriteMHTMLToDisk(std::array<WebData, 3> mhtml_contents, base::File file) {
+  DCHECK(!RenderThread::Get()) << "Should not run in the main renderer thread";
+  // Writes all serialized data to file.
+  TRACE_EVENT0("page-serialization", "WriteMHTMLToDisk (RenderFrameImpl)");
+  SCOPED_UMA_HISTOGRAM_TIMER(
+      "PageSerialization.MhtmlGeneration.WriteToDiskTime.SingleFrame");
+  bool success = true;
+  for (const WebData& data : mhtml_contents) {
+    if (file.WriteAtCurrentPos(data.data(), data.size()) < 0) {
+      success = false;
+      break;
+    }
+  }
+  file.Close();  // Need to flush file contents before sending IPC response.
+  return success;
+}
+
 bool IsHttpPost(const blink::WebURLRequest& request) {
   return request.httpMethod().utf8() == "POST";
 }
 
 #if defined(OS_ANDROID)
 // Returns true if WMPI should be used for playback, false otherwise.
 //
 // Note that HLS and MP4 detection are pre-redirect and path-based. It is
 // possible to load such a URL and find different content.
 bool UseWebMediaPlayerImpl(const GURL& url) {
@@ skipping 4395 matching lines @@
 void RenderFrameImpl::OnSerializeAsMHTML(
     const FrameMsg_SerializeAsMHTML_Params& params) {
   TRACE_EVENT0("page-serialization", "RenderFrameImpl::OnSerializeAsMHTML");
   base::TimeTicks start_time = base::TimeTicks::Now();
   // Unpack IPC payload.
   base::File file = IPC::PlatformFileForTransitToFile(params.destination_file);
   const WebString mhtml_boundary =
       WebString::fromUTF8(params.mhtml_boundary_marker);
   DCHECK(!mhtml_boundary.isEmpty());
 
-  // Three WebData instances for header, parts and footer.
-  WebData mhtml_contents[3];
-  std::set<std::string> digests_of_uris_of_serialized_resources;
-  MHTMLPartsGenerationDelegate delegate(
-      params, &digests_of_uris_of_serialized_resources);
+  // Three WebData instances for header, contents/parts and footer.
+  std::array<WebData, 3> mhtml_contents;
+  std::set<std::string> serialized_resources_uri_digests;
+  MHTMLPartsGenerationDelegate delegate(params,
+                                        &serialized_resources_uri_digests);
 
   bool success = true;
 
   // Generate MHTML header if needed.
   if (IsMainFrame()) {
     TRACE_EVENT0("page-serialization",
                  "RenderFrameImpl::OnSerializeAsMHTML header");
     // |data| can be empty if the main frame should be skipped.  If the main
     // frame is skipped, then the whole archive is bad, so bail to the error
     // condition.
@@ skipping 13 matching lines @@
         mhtml_boundary, GetWebFrame(), &delegate);
   }
 
   // Generate MHTML footer if needed.
   if (success && params.is_last_frame) {
     TRACE_EVENT0("page-serialization",
                  "RenderFrameImpl::OnSerializeAsMHTML footer");
     mhtml_contents[2] = WebFrameSerializer::generateMHTMLFooter(mhtml_boundary);
   }
 
-  // Writes all serialized data to file.
-  // TODO(jcivelli): write the chunks in deferred tasks to give a chance to
-  //                 the message loop to process other events.
-  if (success) {
-    TRACE_EVENT0("page-serialization",
-                 "RenderFrameImpl::OnSerializeAsMHTML writing to file");
-    SCOPED_UMA_HISTOGRAM_TIMER(
-        "PageSerialization.MhtmlGeneration.WriteToDiskTime.SingleFrame");
-    for (const WebData& data : mhtml_contents) {
-      if (file.WriteAtCurrentPos(data.data(), data.size()) < 0) {
-        success = false;
-        break;
-      }
-    }
-  }
-
-  // Cleanup and notify the browser process about completion.
-  file.Close();  // Need to flush file contents before sending IPC response.
+  // Note: we assume RenderFrameImpl::OnWriteMHTMLToDiskComplete and the rest of
+  // this function will be fast enough to not need to be accounted for in this
+  // metric.
   base::TimeDelta main_thread_use_time = base::TimeTicks::Now() - start_time;
-  Send(new FrameHostMsg_SerializeAsMHTMLResponse(
-      routing_id_, params.job_id, success,
-      digests_of_uris_of_serialized_resources, main_thread_use_time));
   UMA_HISTOGRAM_TIMES(
       "PageSerialization.MhtmlGeneration.RendererMainThreadTime.SingleFrame",
       main_thread_use_time);
+
+  if (success &&
+      !(mhtml_contents[0].isEmpty() && mhtml_contents[1].isEmpty() &&
+        mhtml_contents[2].isEmpty())) {
+    base::PostTaskAndReplyWithResult(
+        RenderThreadImpl::current()->GetFileThreadTaskRunner().get(), FROM_HERE,
+        base::Bind(&WriteMHTMLToDisk, base::Passed(&mhtml_contents),
+                   base::Passed(&file)),
+        base::Bind(&RenderFrameImpl::OnWriteMHTMLToDiskComplete,
+                   base::Unretained(this), params.job_id,
+                   base::Passed(&serialized_resources_uri_digests),
+                   main_thread_use_time));
+  } else {

[Łukasz Anforowicz, 2016/09/29 19:56:18]

I think we need to call |file.Close()| here?

Alternatively, we could try to simplify and always go through
PostTaskAndReplyWithResult (rather than special-casing success + 3 empty results
here), because it seems to me that 1) the else branch should happen rarely + 2)
even if the else branch does happen, the performance impact should be relatively
small (yes, 2 thread hops, but no file IO, and there is always the IPC response
later).

WDYT?

[carlosk, 2016/09/30 00:20:18]

Done.

I agree that the else should happen way less often and the cost of 2 thread hops
it minimal when compared to the IPC. So I unified the code paths there.

+    OnWriteMHTMLToDiskComplete(params.job_id,
+                               std::move(serialized_resources_uri_digests),
+                               main_thread_use_time, success);
+  }
+}
+
+void RenderFrameImpl::OnWriteMHTMLToDiskComplete(
+    int job_id,
+    std::set<std::string> serialized_resources_uri_digests,
+    base::TimeDelta main_thread_use_time,
+    bool success) {
+  DCHECK(RenderThread::Get()) << "Must run in the main renderer thread";
+  // Notify the browser process about completion.
+  // Note: this method must be short enough to not need to be accounted for in
+  // PageSerialization.MhtmlGeneration.RendererMainThreadTime.SingleFrame.
+  Send(new FrameHostMsg_SerializeAsMHTMLResponse(
+      routing_id_, job_id, success, serialized_resources_uri_digests,
+      main_thread_use_time));
 }
 
 void RenderFrameImpl::OnFind(int request_id,
                              const base::string16& search_text,
                              const WebFindOptions& options) {
   DCHECK(!search_text.empty());
 
   blink::WebPlugin* plugin = GetWebPluginForFind();
   // Check if the plugin still exists in the document.
   if (plugin) {
@@ skipping 1165 matching lines @@
   // event target. Potentially a Pepper plugin will receive the event.
   // In order to tell whether a plugin gets the last mouse event and which it
   // is, we set |pepper_last_mouse_event_target_| to null here. If a plugin gets
   // the event, it will notify us via DidReceiveMouseEvent() and set itself as
   // |pepper_last_mouse_event_target_|.
   pepper_last_mouse_event_target_ = nullptr;
 #endif
 }
 
 }  // namespace content