Generating CIDs in Blink during MHTML serialization.

third_party/WebKit/Source/web/WebPageSerializer.cpp

 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 27 matching lines @@
 #include "core/html/HTMLAllCollection.h"
 #include "core/html/HTMLFrameElementBase.h"
 #include "core/html/HTMLFrameOwnerElement.h"
 #include "core/html/HTMLInputElement.h"
 #include "core/html/HTMLTableElement.h"
 #include "core/loader/DocumentLoader.h"
 #include "core/page/Page.h"
 #include "core/page/PageSerializer.h"
 #include "platform/SerializedResource.h"
 #include "platform/mhtml/MHTMLArchive.h"
+#include "platform/mhtml/MHTMLParser.h"
 #include "platform/weborigin/KURL.h"
 #include "public/platform/WebCString.h"
 #include "public/platform/WebString.h"
 #include "public/platform/WebURL.h"
 #include "public/platform/WebVector.h"
 #include "public/web/WebFrame.h"
 #include "public/web/WebPageSerializerClient.h"
 #include "public/web/WebView.h"
 #include "web/WebLocalFrameImpl.h"
 #include "web/WebPageSerializerImpl.h"
 #include "web/WebViewImpl.h"
+#include "wtf/Assertions.h"
+#include "wtf/HashMap.h"
 #include "wtf/Vector.h"
 #include "wtf/text/StringConcatenate.h"
 
 namespace blink {
 
 namespace {
 
 class MHTMLPageSerializerDelegate final : public PageSerializer::Delegate {
 public:

[tkent, 2015/12/02 00:59:59]

Please add STACK_ALLCOATED(); for oilpan

[Łukasz Anforowicz, 2015/12/02 02:57:08]

After doing this I got the following error: [blink-gc] Stack-allocated class
'MHTMLPageSerializerDelegate' derives class 'Delegate' which is not stack
allocated.

I think I can't fix this, because some classes derived from the Delegate (i.e.
the test suite in PageSerializerTest.cpp) will not be stack allocated.

Still - after your comment, I've added the wtf way of saying this class should
not be copyable.

Does that sound ok?

[tkent, 2015/12/02 06:33:54]

Looks ok.  It seems it's hard to make Delegate STACK_ALLOCATED because it is
used with a test fixture.

-    ~MHTMLPageSerializerDelegate() override;
+    MHTMLPageSerializerDelegate(HashMap<Frame*, String>* frameToContentID);

[tkent, 2015/12/02 00:59:59]

The argument and m_frameToContentId should be |const
WillBeHeapHashMap<RawPtrWillBeMember<Frame>, String>&|.

[Łukasz Anforowicz, 2015/12/02 02:57:08]

I made the change you've suggested, but I wanted to double-check if it is ok,
because I am allocating the HashMap on the stack (i.e. see the new code in
WebPageSerializer.cpp in serializePageToMHTML - frameToContentID is allocated on
the stack [although I did change its type to WillBeHeap... in the latest
patchset).

[Łukasz Anforowicz, 2015/12/02 03:11:47]

Actually, I don't know how I managed to confuse myself here - blink::HeapHashMap
has "heap" in its name because it allocates buckets / elements / etc on the
heap, not because the heap map itself is be stack allocated...

So - I think that the latest patchset should be ok.

     bool shouldIgnoreAttribute(const Attribute&) override;
+    bool rewriteLink(const Element&, String& rewrittenLink) override;
+private:

[tkent, 2015/12/02 00:59:59]

nit: add a blank line before |private:|

[Łukasz Anforowicz, 2015/12/02 02:57:08]

Done.

+    HashMap<Frame*, String>* m_frameToContentID;
 };
 
-
-MHTMLPageSerializerDelegate::~MHTMLPageSerializerDelegate()
+MHTMLPageSerializerDelegate::MHTMLPageSerializerDelegate(
+    HashMap<Frame*, String>* frameToContentID)
+    : m_frameToContentID(frameToContentID)
 {
 }
 
 bool MHTMLPageSerializerDelegate::shouldIgnoreAttribute(const Attribute& attribute)
 {
     // TODO(fgorski): Presence of srcset attribute causes MHTML to not display images, as only the value of src
     // is pulled into the archive. Discarding srcset prevents the problem. Long term we should make sure to MHTML
     // plays nicely with srcset.
     return attribute.localName() == HTMLNames::srcsetAttr;
 }
 
+bool MHTMLPageSerializerDelegate::rewriteLink(
+    const Element& element,
+    String& rewrittenLink)
+{
+    if (!element.isFrameOwnerElement())
+        return false;
+
+    auto* frameOwnerElement = toHTMLFrameOwnerElement(&element);
+    Frame* frame = frameOwnerElement->contentFrame();
+    if (!frame)
+        return false;
+
+    KURL cidURI = MHTMLParser::convertContentIDToURI(m_frameToContentID->get(frame));
+    ASSERT(cidURI.isValid());
+
+    if (isHTMLFrameElementBase(&element)) {
+        rewrittenLink = cidURI.string();
+        return true;
+    }
+
+    if (isHTMLObjectElement(&element)) {
+        Document* doc = frameOwnerElement->contentDocument();
+        bool isHandledBySerializer = doc->isHTMLDocument()
+            || doc->isXHTMLDocument() || doc->isImageDocument();
+        if (isHandledBySerializer) {
+            rewrittenLink = cidURI.string();
+            return true;
+        }
+    }
+
+    return false;
+}
+
 } // namespace
 
+static HashMap<Frame*, String> generateFrameContentIDs(Page* page)

[tkent, 2015/12/02 00:59:59]

The function should be in anonymous namespace above.

The return value and |frameToContenID| below should be
WillBeHeapHashMap<RawPtrWillBeMember<Frame>, String>.

[Łukasz Anforowicz, 2015/12/02 02:57:08]

Done (for this and the next function).  I am not sure how that happened... :-/
Ok (but please see the question in my other reply).

+{
+    HashMap<Frame*, String> frameToContentID;
+    int frameID = 0;
+    for (Frame* frame = page->mainFrame(); frame; frame = frame->tree().traverseNext()) {
+        // TODO(lukasza): Move cid generation to the browser + use base/guid.h
+        // (see the draft at crrev.com/1386873003).
+        StringBuilder contentIDBuilder;
+        contentIDBuilder.appendLiteral("<frame");
+        contentIDBuilder.appendNumber(frameID++);
+        contentIDBuilder.appendLiteral("@mhtml.blink>");
+
+        frameToContentID.add(frame, contentIDBuilder.toString());
+    }
+    return frameToContentID;
+}
+
 static PassRefPtr<SharedBuffer> serializePageToMHTML(Page* page, MHTMLArchive::EncodingPolicy encodingPolicy)
 {
     Vector<SerializedResource> resources;
-    PageSerializer serializer(&resources, adoptPtr(new MHTMLPageSerializerDelegate));
+    HashMap<Frame*, String> frameToContentID = generateFrameContentIDs(page);
+    MHTMLPageSerializerDelegate delegate(&frameToContentID);
+    PageSerializer serializer(resources, &delegate);
 
     RefPtr<SharedBuffer> output = SharedBuffer::create();
     String boundary = MHTMLArchive::generateMHTMLBoundary();
 
     Document* document = page->deprecatedLocalMainFrame()->document();
     MHTMLArchive::generateMHTMLHeader(
         boundary, document->title(), document->suggestedMIMEType(), *output);
 
     for (Frame* frame = page->deprecatedLocalMainFrame(); frame; frame = frame->tree().traverseNext()) {
         // TODO(lukasza): This causes incomplete MHTML for OOPIFs.
         // (crbug.com/538766)
         if (!frame->isLocalFrame())
             continue;
 
         resources.clear();
         serializer.serializeFrame(*toLocalFrame(frame));
 
-        for (const auto& resource : resources) {
+        bool isFirstResource = true;
+        for (const SerializedResource& resource : resources) {
+            // Frame is the 1st resource (see PageSerializer::serializeFrame doc
+            // comment). Frames need a Content-ID header.
+            String contentID = isFirstResource ? frameToContentID.get(frame) : String();
+
             MHTMLArchive::generateMHTMLPart(
-                boundary, encodingPolicy, resource, *output);
+                boundary, contentID, encodingPolicy, resource, *output);
+
+            isFirstResource = false;
         }
     }
 
     MHTMLArchive::generateMHTMLFooter(boundary, *output);
     return output.release();
 }
 
 WebCString WebPageSerializer::serializeToMHTML(WebView* view)
 {
     RefPtr<SharedBuffer> mhtml = serializePageToMHTML(toWebViewImpl(view)->page(), MHTMLArchive::UseDefaultEncoding);
@@ skipping 38 matching lines @@
 WebString WebPageSerializer::generateBaseTagDeclaration(const WebString& baseTarget)
 {
     // TODO(yosin) We should call |PageSerializer::baseTagDeclarationOf()|.
     if (baseTarget.isEmpty())
         return String("<base href=\".\">");
     String baseString = "<base href=\".\" target=\"" + static_cast<const String&>(baseTarget) + "\">";
     return baseString;
 }
 
 } // namespace blink