[Offline Pages] Generate MHTML header in the browser process.

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>
@@ skipping 5603 matching lines @@
   const WebString mhtml_boundary =
       WebString::FromUTF8(params.mhtml_boundary_marker);
   DCHECK(!mhtml_boundary.IsEmpty());
 
   // Holds WebThreadSafeData instances for some or all of header, contents and
   // footer.
   std::vector<WebThreadSafeData> mhtml_contents;
   std::set<std::string> serialized_resources_uri_digests;
   MHTMLPartsGenerationDelegate delegate(params,
                                         &serialized_resources_uri_digests);
-
   MhtmlSaveStatus save_status = MhtmlSaveStatus::SUCCESS;
-  bool has_some_data = false;
-
-  // Generate MHTML header if needed.
-  if (IsMainFrame()) {
-    TRACE_EVENT0("page-serialization",
-                 "RenderFrameImpl::OnSerializeAsMHTML header");

[carlosk, 2017/04/25 01:28:08]

Could you make this header serialization tracing survive this change like it was
done for the footer change? (see
MHTMLGenerationManager::Job::FinalizeAndCloseFileOnFileThread)

Ideally it would encompass the header generation/serialization too but that
might not be ideal for this case given all the data needed to generate it.

-    // The returned data can be empty if the main frame should be skipped. If
-    // the main frame is skipped, then the whole archive is bad.
-    mhtml_contents.emplace_back(WebFrameSerializer::GenerateMHTMLHeader(
-        mhtml_boundary, GetWebFrame(), &delegate));
-    if (mhtml_contents.back().IsEmpty())
-      save_status = MhtmlSaveStatus::FRAME_SERIALIZATION_FORBIDDEN;
-    else
-      has_some_data = true;
-  }
 
   // Generate MHTML parts.  Note that if this is not the main frame, then even
   // skipping the whole parts generation step is not an error - it simply
   // results in an omitted resource in the final file.
-  if (save_status == MhtmlSaveStatus::SUCCESS) {
-    TRACE_EVENT0("page-serialization",
-                 "RenderFrameImpl::OnSerializeAsMHTML parts serialization");
-    // The returned data can be empty if the frame should be skipped, but this
-    // is OK.
-    mhtml_contents.emplace_back(WebFrameSerializer::GenerateMHTMLParts(
-        mhtml_boundary, GetWebFrame(), &delegate));
-    has_some_data |= !mhtml_contents.back().IsEmpty();
-  }
+  TRACE_EVENT0("page-serialization",
+               "RenderFrameImpl::OnSerializeAsMHTML parts serialization");
+  // The returned data can be empty if the frame should be skipped, but this
+  // is OK.
+  mhtml_contents.emplace_back(WebFrameSerializer::GenerateMHTMLParts(
+      mhtml_boundary, GetWebFrame(), &delegate));
+  bool has_some_data = !mhtml_contents.back().IsEmpty();
+  if (IsMainFrame() && !has_some_data)
+    save_status = MhtmlSaveStatus::FRAME_SERIALIZATION_FORBIDDEN;
 
   // Note: the MHTML footer is written by the browser process, after the last
   // frame is serialized by a renderer process.

[carlosk, 2017/04/25 01:28:08]

nit: we should either remove this comment above or add a similar one for the
MTHML header. I'm slightly favoring the former as this comment only makes sense
because the footer used to be generated here and that doesn't matter anymore.

 
   // 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;
   UMA_HISTOGRAM_TIMES(
       "PageSerialization.MhtmlGeneration.RendererMainThreadTime.SingleFrame",
       main_thread_use_time);
 
-  if (save_status == MhtmlSaveStatus::SUCCESS && has_some_data) {
+  if (has_some_data) {
     base::PostTaskAndReplyWithResult(
         RenderThreadImpl::current()->GetFileThreadTaskRunner().get(), FROM_HERE,
         base::Bind(&WriteMHTMLToDisk, base::Passed(&mhtml_contents),
                    base::Passed(&file)),
         base::Bind(&RenderFrameImpl::OnWriteMHTMLToDiskComplete,
                    weak_factory_.GetWeakPtr(), params.job_id,
                    base::Passed(&serialized_resources_uri_digests),
                    main_thread_use_time));
   } else {
     file.Close();
@@ skipping 1321 matching lines @@
       policy(info.default_policy),
       replaces_current_history_item(info.replaces_current_history_item),
       history_navigation_in_new_child_frame(
           info.is_history_navigation_in_new_child_frame),
       client_redirect(info.is_client_redirect),
       cache_disabled(info.is_cache_disabled),
       form(info.form),
       source_location(info.source_location) {}
 
 }  // namespace content