Making PageSerializer operate on single frames only.

third_party/WebKit/Source/web/WebPageSerializer.cpp

Index: third_party/WebKit/Source/web/WebPageSerializer.cpp
diff --git a/third_party/WebKit/Source/web/WebPageSerializer.cpp b/third_party/WebKit/Source/web/WebPageSerializer.cpp
index 193da0a60ca134135d630398d24c567b403cd21d..59120dd291cf171e08b1a34959b273e5785f400c 100644
--- a/third_party/WebKit/Source/web/WebPageSerializer.cpp
+++ b/third_party/WebKit/Source/web/WebPageSerializer.cpp
@@ -87,10 +87,39 @@ bool MHTMLPageSerializerDelegate::shouldIgnoreAttribute(const Attribute& attribu
 static PassRefPtr<SharedBuffer> serializePageToMHTML(Page* page, MHTMLArchive::EncodingPolicy encodingPolicy)
 {
     Vector<SerializedResource> resources;
+    HashSet<KURL> alreadySerializedUrls;

[dcheng, 2015/11/18 03:27:12]

How come we weren't previously deduping?

[Łukasz Anforowicz, 2015/11/18 18:40:41]

You're right - my changes here don't make sense... :-/

PageSerializer is deduping before and after this CL (i.e. via
PageSerializer::m_resourceURLs).  My mistake was that at one point I was
creating a separate instance of PageSerializer per-frame (i.e. inside the loop)
and that was only deduplicating within a single frame.  I should have removed
the extra deduplication when reusing a single PageSerializer instance for all
frames the loop iterates over.

Using a single PageSerializer for all frames is okay even when starting to
generate unique CIDs for all frames (even ones with the same URI), because the
CIDs CL removed frame deduplication inside PageSerializer.  So - resource
deduplication across frames can wait until we actually use a separate instance
of PageSerializer for different frames - this won't happen until
crrev.com/1386873003 (Transitioning MHTML generation from view-oriented to
frame-oriented) and deduplication will be introduced in crrev.com/1417323006
(Deduplicating MHTML parts across frames).

Done - removed url-based-deduplication that the CL was incorrectly adding to
WebPageSerializer::serializePageToMHTML.

PS. Note that deduping is covered by the last verifications done by
SavePageSitePerProcessBrowserTest.SaveAsMHTML, but this test is temporarily
disabled until it works with OOPIFs.  Still, the test passes (= deduplication
works even after yanking out alreadySerializedUrls) when uncommenting out
passing of --site-per-process in
SavePageSitePerProcessBrowserTest::SetUpCommandLine.

     PageSerializer serializer(&resources, adoptPtr(new MHTMLPageSerializerDelegate));
-    serializer.serialize(page);
+
+    RefPtr<SharedBuffer> output = SharedBuffer::create();
+    String boundary = MHTMLArchive::generateMHTMLBoundary();
+
     Document* document = page->deprecatedLocalMainFrame()->document();
-    return MHTMLArchive::generateMHTMLData(resources, encodingPolicy, document->title(), document->suggestedMIMEType());
+    MHTMLArchive::generateMHTMLHeader(
+        boundary, document->title(), document->suggestedMIMEType(), *output);
+
+    Frame* frame = page->deprecatedLocalMainFrame();

[dcheng, 2015/11/18 03:27:12]

I don't think you need this outside the loop, so just initialize it in the for
().

[Łukasz Anforowicz, 2015/11/18 18:40:41]

Done.  (for some reason I was previously worried about the resulting formatting,
but I guess it is not worse than what I had before)

+    for (; frame; frame = frame->tree().traverseNext()) {
+        // TODO(lukasza): This causes incomplete MHTML for OOPIFs.
+        // (crbug.com/538766)
+        if (!frame->isLocalFrame())
+            continue;
+
+        resources.clear();
+        LocalFrame* localFrame = toLocalFrame(frame);
+        serializer.serializeFrame(*localFrame);

[dcheng, 2015/11/18 03:27:12]

Ditto: just inline toLocalFrame(*frame)

[Łukasz Anforowicz, 2015/11/18 18:40:41]

Done.  (for some reason previously I was worried that *functionCall() is
difficult to read, but I guess this is fine)

+
+        for (const auto& resource : resources) {
+            if (alreadySerializedUrls.contains(resource.url))
+                continue;
+            alreadySerializedUrls.add(resource.url);
+
+            MHTMLArchive::generateMHTMLPart(
+                boundary, encodingPolicy, resource, *output);
+        }
+    }
+
+    MHTMLArchive::generateMHTMLFooter(boundary, *output);
+    return output.release();
 }
 
 WebCString WebPageSerializer::serializeToMHTML(WebView* view)