Adds a feature to MHTML serialization that omits subframes and subresources marked no-store.

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 4a09e2253c41ddda7c3546282474515d521e3a9e..c1a49050bb2a5c6b23fad62c1ecca99a5609967d 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -707,6 +707,13 @@ class MHTMLPartsGenerationDelegate
     return WebString::fromUTF8(content_id);
   }
 
+  blink::WebFrameSerializerCacheControlPolicy cacheControlPolicy() override {
+    return static_cast<blink::WebFrameSerializerCacheControlPolicy>(
+        params_.mhtml_cache_control_policy);
+  }
+
+  bool useBinaryEncoding() override { return params_.mhtml_binary_encoding; }

[dcheng, 2016/05/20 20:25:02]

Is this just incidental cleanup to make the policy checks more consistent? Or
was it required for some other part of this patch?

[dewittj, 2016/05/20 21:22:22]

Incidental.  Do you prefer I revert it?  It cleaned up some of the API
signatures of WebFrameSerializer.

+
  private:
   const FrameMsg_SerializeAsMHTML_Params& params_;
   std::set<std::string>* digests_of_uris_of_serialized_resources_;
@@ -4948,30 +4955,36 @@ void RenderFrameImpl::OnSerializeAsMHTML(
   DCHECK(!mhtml_boundary.isEmpty());
 
   WebData data;
-  bool success = true;
   std::set<std::string> digests_of_uris_of_serialized_resources;
   MHTMLPartsGenerationDelegate delegate(
       params, &digests_of_uris_of_serialized_resources);
 
+  bool success = true;
+
   // Generate MHTML header if needed.
   if (IsMainFrame()) {
-    blink::WebFrameSerializerCacheControlPolicy policy =
-        static_cast<blink::WebFrameSerializerCacheControlPolicy>(
-            params.mhtml_cache_control_policy);
-    success = WebFrameSerializer::generateMHTMLHeader(mhtml_boundary, policy,
-                                                      GetWebFrame(), &data);
-    if (success && file.WriteAtCurrentPos(data.data(), data.size()) < 0) {
+    // |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.
+    WebData data = WebFrameSerializer::generateMHTMLHeader(
+        mhtml_boundary, GetWebFrame(), &delegate);
+    if (data.size() == 0 ||

[dcheng, 2016/05/20 20:25:02]

data.isEmpty()

[dcheng, 2016/05/20 20:25:02]

isEmpty()

[dewittj, 2016/05/20 21:22:22]

Done.

+        file.WriteAtCurrentPos(data.data(), data.size()) < 0) {
       success = false;
     }
   }
 
-  // Generate MHTML parts.
+  // 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 (success) {
-    data = WebFrameSerializer::generateMHTMLParts(
-        mhtml_boundary, GetWebFrame(), params.mhtml_binary_encoding, &delegate);
+    // |data| can be empty if the frame should be skipped, but this is OK.
+    data = WebFrameSerializer::generateMHTMLParts(mhtml_boundary, GetWebFrame(),
+                                                  &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) {
+    if (data.size() > 0 &&

[dcheng, 2016/05/20 20:25:02]

!data.isEmpty()

[dcheng, 2016/05/20 20:25:02]

!isEmpty()

[dewittj, 2016/05/20 21:22:22]

Done.

+        file.WriteAtCurrentPos(data.data(), data.size()) < 0) {
       success = false;
     }
   }