Get signals working in the EXTRA_DATA section of MHTML

chrome/browser/android/offline_pages/prerendering_offliner.cc

Index: chrome/browser/android/offline_pages/prerendering_offliner.cc
diff --git a/chrome/browser/android/offline_pages/prerendering_offliner.cc b/chrome/browser/android/offline_pages/prerendering_offliner.cc
index a6564ad078d662af7b164c009e039d8dd89aa383..44f6d828d4f85035102c71f275d9da1caa8b9209 100644
--- a/chrome/browser/android/offline_pages/prerendering_offliner.cc
+++ b/chrome/browser/android/offline_pages/prerendering_offliner.cc
@@ -14,10 +14,19 @@
 #include "components/offline_pages/core/background/save_page_request.h"
 #include "components/offline_pages/core/client_namespace_constants.h"
 #include "components/offline_pages/core/downloads/download_ui_adapter.h"
+#include "components/offline_pages/core/offline_page_feature.h"
 #include "components/offline_pages/core/offline_page_model.h"
 #include "content/public/browser/browser_context.h"
+#include "content/public/browser/mhtml_extra_parts.h"
 #include "content/public/browser/web_contents.h"
 
+namespace {
+const char kContentType[] = "text/plain";
+const char kContentTransferEncodingBinary[] =
+    "Content-Transfer-Encoding: binary";
+const char kXHeaderForSignals[] = "X-Chrome-Loading-Metrics-Data: 1";
+}  // namespace
+
 namespace offline_pages {
 
 PrerenderingOffliner::PrerenderingOffliner(
@@ -83,6 +92,24 @@ void PrerenderingOffliner::OnLoadPageDone(
     else if (save_page_params.url != request.url())
       save_page_params.original_url = request.url();
 
+    if (IsOfflinePagesLoadSignalCollectingEnabled()) {
+      // Stash loading signals for writing when we write out the MHTML.
+      std::string headers =
+          base::StringPrintf("%s\r\n%s\r\n\r\n", kContentTransferEncodingBinary,
+                             kXHeaderForSignals);
+      std::string body = headers + SerializeLoadingSignalData();
+      std::string content_type = kContentType;
+      std::string content_location = "cid:signal-data-" +

[fgorski, 2017/04/05 18:59:12]

is there a good reason not to base::StringPrintf this one too?

[Pete Williamson, 2017/04/05 22:11:30]

It was a style choice, in my opinion, it is more readable as is.  Changed to use
base::StringPrintf().

+                                     std::to_string(request.request_id()) +
+                                     "@mhtml.blink";
+
+      content::MHTMLExtraParts* extra_parts =
+          content::MHTMLExtraParts::FromWebContents(web_contents);
+      DCHECK(extra_parts);

[fgorski, 2017/04/05 18:59:12]

this is worrisome:
* DCHECK in debug is great.
* But that is also going to end up being a crash in production if you don't skip
another line...
Do we want that?

[Pete Williamson, 2017/04/05 22:11:30]

It should be safe since FromWebContents creates a new MHTMLExtraParts if there
is not one already, but to be extra safe, I added an if check.

[fgorski, 2017/04/05 22:42:38]

If you are sure this is never null CHECK(extra_parts); is probably better, but
it is up to you. Resulting crash, if any will be easier to pin-point that way.

+
+      extra_parts->AddExtraMHTMLPart(content_type, content_location, body);
+    }
+
     SavePage(save_page_params, std::move(archiver),
              base::Bind(&PrerenderingOffliner::OnSavePageDone,
                         weak_ptr_factory_.GetWeakPtr(), request));
@@ -94,6 +121,19 @@ void PrerenderingOffliner::OnLoadPageDone(
   }
 }
 
+std::string PrerenderingOffliner::SerializeLoadingSignalData() {
+  // Write the signal data into a single string.
+  std::string signal_string;
+  const std::vector<std::string>& signals = loader_->GetLoadingSignalData();
+
+  // TODO(petewil): Convert this to JSON, use json_writer.h
+  for (std::string signal : signals) {
+    signal_string += signal;
+    signal_string += "\n";
+  }
+  return signal_string;
+}
+
 void PrerenderingOffliner::OnSavePageDone(
     const SavePageRequest& request,
     SavePageResult save_result,