Introduces a new MHTML generation parameter specifying different behvaior for cache-control headers.

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 087cd75e2a4adf3622d2a000c478cc7cb4e8a640..74de5229d9d9e15c2c264713bda39788601ec6cd 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -69,6 +69,7 @@
 #include "content/public/common/content_switches.h"
 #include "content/public/common/context_menu_params.h"
 #include "content/public/common/isolated_world_ids.h"
+#include "content/public/common/mhtml_generation.h"
 #include "content/public/common/page_state.h"
 #include "content/public/common/resource_response.h"
 #include "content/public/common/url_constants.h"
@@ -4899,43 +4900,45 @@ void RenderFrameImpl::OnSerializeAsMHTML(
   DCHECK(!mhtml_boundary.isEmpty());
 
   WebData data;
-  bool success = true;
+  MHTMLSerializationResult result = MHTMLSerializationResult::OK;
   std::set<std::string> digests_of_uris_of_serialized_resources;
   MHTMLPartsGenerationDelegate delegate(
       params, &digests_of_uris_of_serialized_resources);
 
   // Generate MHTML header if needed.
   if (IsMainFrame()) {
-    data =
-        WebFrameSerializer::generateMHTMLHeader(mhtml_boundary, GetWebFrame());
-    if (file.WriteAtCurrentPos(data.data(), data.size()) < 0) {
-      success = false;
+    bool success = WebFrameSerializer::generateMHTMLHeader(

[Łukasz Anforowicz, 2016/05/12 18:56:49]

Hmmm... conveying MHTMLSerializationResult::NO_STORE_MAIN_FRAME by a bool result
from WebFrameSerializer::generateMHTMLHeader seems wrong (especially given that
a comment for WebFrameSerializer::generateMHTMLHeader doesn't talk about this
semantics at all).

We could change the return type of WebFrameSerializer::generateMHTMLHeader from
bool to an enum, but maybe handling of FAIL_FOR_NO_STORE_MAIN_FRAME can simply
happen here (i.e. if we expose a new bool
WebLocalFrame::cacheControlContainsNoCache() method).  WDYT?

[dewittj, 2016/05/12 22:58:01]

Since I don't expose more granular errors any more, it doesn't make as much
sense to have an enum for this particular thing.  I returned bool as
success/fail - the only thing that currently makes it fail today is this cache
control header.

I updated the comment; does this look better to you now?

[Łukasz Anforowicz, 2016/05/12 23:56:27]

Yes - looks good.  Thanks.

+        mhtml_boundary,
+        mhtml::toCacheControlPolicy(params.mhtml_cache_control_policy),
+        GetWebFrame(), &data);
+    if (!success) {
+      result = MHTMLSerializationResult::NO_STORE_MAIN_FRAME;
+    } else if (file.WriteAtCurrentPos(data.data(), data.size()) < 0) {
+      result = MHTMLSerializationResult::FILE_WRITE_FAILED_IN_HEADER;
     }
   }
 
   // Generate MHTML parts.
-  if (success) {
+  if (result == MHTMLSerializationResult::OK) {
     data = WebFrameSerializer::generateMHTMLParts(
         mhtml_boundary, GetWebFrame(), params.mhtml_binary_encoding, &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) {
-      success = false;
-    }
+    if (file.WriteAtCurrentPos(data.data(), data.size()) < 0)
+      result = MHTMLSerializationResult::FILE_WRITE_FAILED_IN_PARTS_GEN;
   }
 
   // Generate MHTML footer if needed.
-  if (success && params.is_last_frame) {
+  if (result == MHTMLSerializationResult::OK && params.is_last_frame) {
     data = WebFrameSerializer::generateMHTMLFooter(mhtml_boundary);
-    if (file.WriteAtCurrentPos(data.data(), data.size()) < 0) {
-      success = false;
-    }
+    if (file.WriteAtCurrentPos(data.data(), data.size()) < 0)
+      result = MHTMLSerializationResult::FILE_WRITE_FAILED_IN_FOOTER;
   }
 
   // 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_, params.job_id, success,
+      routing_id_, params.job_id, result,
       digests_of_uris_of_serialized_resources));
 }