Save-Page-As-Complete-Html: Each frame links to a distinct local file.

third_party/WebKit/Source/web/WebPageSerializerImpl.cpp

Index: third_party/WebKit/Source/web/WebPageSerializerImpl.cpp
diff --git a/third_party/WebKit/Source/web/WebPageSerializerImpl.cpp b/third_party/WebKit/Source/web/WebPageSerializerImpl.cpp
index b2a8a483f661c086297e79b0ebe8f030c92c04cf..357ac297136a4bc742fc2a27324f20e96a7ea59a 100644
--- a/third_party/WebKit/Source/web/WebPageSerializerImpl.cpp
+++ b/third_party/WebKit/Source/web/WebPageSerializerImpl.cpp
@@ -86,6 +86,7 @@
 #include "core/html/HTMLAllCollection.h"
 #include "core/html/HTMLElement.h"
 #include "core/html/HTMLFormElement.h"
+#include "core/html/HTMLFrameOwnerElement.h"
 #include "core/html/HTMLHtmlElement.h"
 #include "core/html/HTMLMetaElement.h"
 #include "core/loader/DocumentLoader.h"
@@ -279,6 +280,21 @@ void WebPageSerializerImpl::encodeAndFlushBuffer(
 // TODO(yosin): We should utilize |MarkupFormatter| here to share code,
 // especially escaping attribute values, done by |WebEntities| |m_htmlEntities|
 // and |m_xmlEntities|.
+void WebPageSerializerImpl::appendAttribute(
+    StringBuilder& result,
+    bool isHTMLDocument,
+    const String& attrName,
+    const String& attrValue) {
+    result.append(' ');
+    result.append(attrName);
+    result.appendLiteral("=\"");
+    if (isHTMLDocument)
+        result.append(m_htmlEntities.convertEntitiesInString(attrValue));
+    else
+        result.append(m_xmlEntities.convertEntitiesInString(attrValue));
+    result.append('\"');
+}
+
 void WebPageSerializerImpl::openTagToString(Element* element,
                                             SerializeDomParam* param)
 {
@@ -291,42 +307,37 @@ void WebPageSerializerImpl::openTagToString(Element* element,
     // Add open tag
     result.append('<');
     result.append(element->nodeName().lower());
+
+    // Find out if this element owns a frame.
+    Frame* frame = nullptr;
+    if (element->isFrameOwnerElement()) {
+        frame = toHTMLFrameOwnerElement(element)->contentFrame();

[Łukasz Anforowicz, 2015/12/16 21:39:02]

There is some risk that this toHTMLFrameOwnerElement might blow-up in a way
similar to crbug.com/570073...  I can't repro this bug locally, but I am not
sure about if the predicate below is really correct (as my usage of
HTMLFrameOwnerElement is agnostic to local-vs-remote distinction):

  HTMLFrameOwnerElement.h:
    DEFINE_TYPE_CASTS(HTMLFrameOwnerElement, ..., owner->isLocal()... 

+    }
+
     // Go through all attributes and serialize them.
-    AttributeCollection attributes = element->attributes();
-    AttributeCollection::iterator end = attributes.end();
-    for (AttributeCollection::iterator it = attributes.begin(); it != end; ++it) {
-        result.append(' ');
-        // Add attribute pair
-        result.append(it->name().toString());
-        result.appendLiteral("=\"");
-        if (!it->value().isEmpty()) {
-            const String& attrValue = it->value();
-
-            // Check whether we need to replace some resource links
-            // with local resource paths.
-            const QualifiedName& attrName = it->name();
-            if (element->hasLegalLinkAttribute(attrName)) {
-                // For links start with "javascript:", we do not change it.
-                if (attrValue.startsWith("javascript:", TextCaseInsensitive)) {
-                    result.append(m_htmlEntities.convertEntitiesInString(attrValue));
+    for (const auto& it : element->attributes()) {
+        const QualifiedName& attrName = it.name();
+        String attrValue = it.value();
+
+        // Rewrite the attribute value if requested.
+        if (element->hasLegalLinkAttribute(attrName)) {
+            // For links start with "javascript:", we do not change it.
+            if (!attrValue.startsWith("javascript:", TextCaseInsensitive)) {
+                // Get the absolute link.
+                String completeURL = param->document->completeURL(attrValue);
+
+                // Check whether we have a local file to link to.
+                if (frame && m_frameToLocalPath.contains(frame)) {
+                    attrValue = m_frameToLocalPath.get(frame);
+                } else if (m_urlToLocalPath.contains(completeURL)) {
+                    attrValue = m_urlToLocalPath.get(completeURL);
                 } else {
-                    // Get the absolute link
-                    String completeURL = param->document->completeURL(attrValue);
-                    // Check whether we have local files for those link.
-                    if (m_localLinks.contains(completeURL)) {
-                        result.append(m_htmlEntities.convertEntitiesInString(m_localLinks.get(completeURL)));
-                    } else {
-                        result.append(m_htmlEntities.convertEntitiesInString(completeURL));
-                    }
+                    attrValue = completeURL;
                 }
-            } else {
-                if (param->isHTMLDocument)
-                    result.append(m_htmlEntities.convertEntitiesInString(attrValue));
-                else
-                    result.append(m_xmlEntities.convertEntitiesInString(attrValue));
             }
         }
-        result.append('\"');
+
+        appendAttribute(result, param->isHTMLDocument, attrName.toString(), attrValue);
     }
 
     // Do post action for open tag.
@@ -412,7 +423,8 @@ void WebPageSerializerImpl::buildContentForNode(Node* node,
 WebPageSerializerImpl::WebPageSerializerImpl(
     WebLocalFrame* frame,
     WebPageSerializerClient* client,
-    const WebVector<std::pair<WebURL, WebString>>& urlsToLocalPaths)
+    const WebVector<std::pair<WebURL, WebString>>& urlsToLocalPaths,
+    const WebVector<std::pair<WebFrame*, WebString>>& framesToLocalPaths)
     : m_client(client)
     , m_htmlEntities(false)
     , m_xmlEntities(true)
@@ -422,11 +434,19 @@ WebPageSerializerImpl::WebPageSerializerImpl(
     m_specifiedWebLocalFrameImpl = toWebLocalFrameImpl(frame);
     // Make sure we have non 0 client.
     ASSERT(client);
-    // Build local resources map.
+    // Build url to local path map.
     for (const auto& it : urlsToLocalPaths) {
         KURL url = it.first;
-        ASSERT(!m_localLinks.contains(url.string()));
-        m_localLinks.set(url.string(), it.second);
+        ASSERT(!m_urlToLocalPath.contains(url.string()));
+        m_urlToLocalPath.set(url.string(), it.second);
+    }
+    // Build frame to local path map.
+    for (const auto& it : framesToLocalPaths) {
+        WebFrame* webFrame = it.first;
+        ASSERT(webFrame);
+        Frame* frame = webFrame->toImplBase()->frame();
+        ASSERT(!m_frameToLocalPath.contains(frame));
+        m_frameToLocalPath.set(frame, it.second);
     }
 
     ASSERT(m_dataBuffer.isEmpty());