Save-Page-As-Complete-Html: Rewriting of srcdoc attribute.
BUG=106364, 106158, 538188
Review URL: https://codereview.chromium.org/1500103002
Cr-Commit-Position: refs/heads/master@{#373361}
diff --git a/chrome/browser/download/save_page_browsertest.cc b/chrome/browser/download/save_page_browsertest.cc
index 6ee552f..040898a 100644
--- a/chrome/browser/download/save_page_browsertest.cc
+++ b/chrome/browser/download/save_page_browsertest.cc
@@ -1119,23 +1119,6 @@
};
std::vector<std::string> expected_substrings(std::begin(arr), std::end(arr));
- if (save_page_type == content::SAVE_PAGE_TYPE_AS_COMPLETE_HTML) {
- // TODO(lukasza): crbug.com/106364: Expand complete-html test to cover all
- // test frames. In particular, the |complete_html_arr| below should be the
- // same as the |arr| above (and at this point the special-casing of
- // complete-html can be removed).
- // Draft CLs with fix proposals that should accomplish this:
- // - crrev.com/1502563004
- // - crrev.com/1500103002
- std::string complete_html_arr[] = {
- "frames-runtime-changes.htm: 4388232f-8d45-4d2e-9807-721b381be153",
- "subframe1: 21595339-61fc-4854-b6df-0668328ea263",
- "subframe2: adf55719-15e7-45be-9eda-d12fe782a1bd",
- };
- expected_substrings = std::vector<std::string>(
- std::begin(complete_html_arr), std::end(complete_html_arr));
- }
-
GURL url(embedded_test_server()->GetURL(
"a.com", "/save_page/frames-runtime-changes.htm?do_runtime_changes=1"));
diff --git a/content/browser/download/save_package.cc b/content/browser/download/save_package.cc
index 8454625..3260e3d 100644
--- a/content/browser/download/save_package.cc
+++ b/content/browser/download/save_package.cc
@@ -553,7 +553,7 @@
// We have received a message from SaveFileManager about a new saving job. We
// find a SaveItem and store it in our in_progress list.
void SavePackage::StartSave(const SaveFileCreateInfo* info) {
- DCHECK(info && !info->url.is_empty());
+ DCHECK(info);
SaveItemIdMap::iterator it = in_progress_items_.find(info->save_item_id);
if (it == in_progress_items_.end()) {
@@ -1175,8 +1175,6 @@
const GURL& url,
const Referrer& referrer,
SaveFileCreateInfo::SaveFileSource save_source) {
- DCHECK(url.is_valid()); // |url| should be validated by the callers.
-
SaveItem* save_item;
Referrer sanitized_referrer = Referrer::SanitizeForRequest(url, referrer);
save_item = new SaveItem(url, sanitized_referrer, this, save_source,
@@ -1232,9 +1230,6 @@
void SavePackage::EnqueueFrame(int container_frame_tree_node_id,
int frame_tree_node_id,
const GURL& frame_original_url) {
- if (!frame_original_url.is_valid())
- return;
-
SaveItem* save_item = CreatePendingSaveItem(
container_frame_tree_node_id, frame_tree_node_id, frame_original_url,
Referrer(), SaveFileCreateInfo::SAVE_FILE_FROM_DOM);
diff --git a/content/renderer/savable_resources.cc b/content/renderer/savable_resources.cc
index eb6c9da..c1a702b 100644
--- a/content/renderer/savable_resources.cc
+++ b/content/renderer/savable_resources.cc
@@ -61,13 +61,9 @@
const WebElement& element,
const WebDocument& current_doc,
SavableResourcesResult* result) {
- // Check whether the node has sub resource URL or not.
- WebString value = GetSubResourceLinkFromElement(element);
- if (value.isNull())
- return;
-
// Get absolute URL.
- GURL element_url = current_doc.completeURL(value);
+ WebString link_attribute_value = GetSubResourceLinkFromElement(element);
+ GURL element_url = current_doc.completeURL(link_attribute_value);
// See whether to report this element as a subframe.
WebFrame* web_frame = WebFrame::fromFrameOwnerElement(element);
@@ -79,6 +75,10 @@
return;
}
+ // Check whether the node has sub resource URL or not.
+ if (link_attribute_value.isNull())
+ return;
+
// Ignore invalid URL.
if (!element_url.is_valid())
return;
diff --git a/third_party/WebKit/Source/web/WebFrameSerializerImpl.cpp b/third_party/WebKit/Source/web/WebFrameSerializerImpl.cpp
index b1ce352..c60ddd5 100644
--- a/third_party/WebKit/Source/web/WebFrameSerializerImpl.cpp
+++ b/third_party/WebKit/Source/web/WebFrameSerializerImpl.cpp
@@ -86,6 +86,7 @@
#include "core/html/HTMLAllCollection.h"
#include "core/html/HTMLElement.h"
#include "core/html/HTMLFormElement.h"
+#include "core/html/HTMLFrameElementBase.h"
#include "core/html/HTMLFrameOwnerElement.h"
#include "core/html/HTMLHtmlElement.h"
#include "core/html/HTMLMetaElement.h"
@@ -304,18 +305,26 @@
result.append('<');
result.append(element->nodeName().lower());
- // Find out if this element owns a frame.
+ // Find out if we need to do frame-specific link rewriting.
WebFrame* frame = nullptr;
if (element->isFrameOwnerElement()) {
frame = WebFrame::fromFrame(
toHTMLFrameOwnerElement(element)->contentFrame());
}
+ WebString rewrittenFrameLink;
+ bool shouldRewriteFrameSrc =
+ frame && m_delegate->rewriteFrameSource(frame, &rewrittenFrameLink);
+ bool didRewriteFrameSrc = false;
// Go through all attributes and serialize them.
for (const auto& it : element->attributes()) {
const QualifiedName& attrName = it.name();
String attrValue = it.value();
+ // Skip srcdoc attribute if we will emit src attribute (for frames).
+ if (shouldRewriteFrameSrc && attrName == HTMLNames::srcdocAttr)
+ continue;
+
// Rewrite the attribute value if requested.
if (element->hasLegalLinkAttribute(attrName)) {
// For links start with "javascript:", we do not change it.
@@ -324,11 +333,12 @@
KURL completeURL = param->document->completeURL(attrValue);
// Check whether we have a local file to link to.
- WebString rewrittenLink;
- if (frame && m_delegate->rewriteFrameSource(frame, &rewrittenLink)) {
- attrValue = rewrittenLink;
- } else if (m_delegate->rewriteLink(completeURL, &rewrittenLink)) {
- attrValue = rewrittenLink;
+ WebString rewrittenURL;
+ if (shouldRewriteFrameSrc) {
+ attrValue = rewrittenFrameLink;
+ didRewriteFrameSrc = true;
+ } else if (m_delegate->rewriteLink(completeURL, &rewrittenURL)) {
+ attrValue = rewrittenURL;
} else {
attrValue = completeURL;
}
@@ -338,6 +348,14 @@
appendAttribute(result, param->isHTMLDocument, attrName.toString(), attrValue);
}
+ // For frames where link rewriting was requested, ensure that src attribute
+ // is written even if the original document didn't have that attribute
+ // (mainly needed for iframes with srcdoc, but with no src attribute).
+ if (shouldRewriteFrameSrc && !didRewriteFrameSrc && isHTMLIFrameElement(element)) {
+ appendAttribute(
+ result, param->isHTMLDocument, HTMLNames::srcAttr.toString(), rewrittenFrameLink);
+ }
+
// Do post action for open tag.
String addedContents = postActionAfterSerializeOpenTag(element, param);
// Complete the open tag for element when it has child/children.
