Save-Page-As-Complete-HTML: Even better handling of <object> elements.

content/renderer/savable_resources.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/savable_resources.h"
 
 #include <set>
 
 #include "base/compiler_specific.h"
 #include "base/logging.h"
@@ skipping 16 matching lines @@
 using blink::WebInputElement;
 using blink::WebLocalFrame;
 using blink::WebNode;
 using blink::WebString;
 using blink::WebVector;
 using blink::WebView;
 
 namespace content {
 namespace {
 
-// Get all savable resource links from current element. One element might
-// have more than one resource link. It is possible to have some links
-// in one CSS stylesheet.
+// If present and valid, then push the link associated with |element|
+// into either SavableResourcesResult::subframes or
+// SavableResourcesResult::resources_list.
 void GetSavableResourceLinkForElement(
     const WebElement& element,
     const WebDocument& current_doc,
     SavableResourcesResult* result) {
-  if (element.hasHTMLTagName("iframe") || element.hasHTMLTagName("frame")) {
-    GURL complete_url = current_doc.completeURL(element.getAttribute("src"));
-    WebFrame* web_frame = WebFrame::fromFrameOwnerElement(element);
-
-    SavableSubframe subframe;
-    subframe.original_url = complete_url;
-    subframe.routing_id = GetRoutingIdForFrameOrProxy(web_frame);
-
-    result->subframes->push_back(subframe);
-    return;
-  }
-
   // Check whether the node has sub resource URL or not.
   WebString value = GetSubResourceLinkFromElement(element);
   if (value.isNull())
     return;
+
   // Get absolute URL.
-  GURL u = current_doc.completeURL(value);
+  GURL element_url = current_doc.completeURL(value);
+
+  // See whether to report this element as a subframe.
+  WebFrame* web_frame = WebFrame::fromFrameOwnerElement(element);
+  if (web_frame) {
+    bool frameContainsHtmlDoc = false;

[yosin_UTC9, 2015/11/24 01:32:41]

nit: Can we move out this if-statement, L55-L70 as static function taking
|web_frame|
to make |GetSavableResourceLinkForElement()| smaller and encapsulating logic of
|frameContainsHtmlDoc|?

[Łukasz Anforowicz, 2015/11/24 17:57:11]

Done.

For a moment I wondered if we should have
GetWebFrameIfElementContainsHtmlDocument(const WebElement&) rather than
DoesFrameContainHtmlDocument, but I think the latter is more straightforward and
easier to understand (even though having to pass both WebFrame& and WebElement&
feels a bit weird).

+    if (web_frame->isWebLocalFrame()) {
+      WebDocument doc = web_frame->document();
+      frameContainsHtmlDoc = doc.isHTMLDocument() || doc.isXHTMLDocument();
+    } else {
+      // Cannot inspect contents of a remote frame, so we use a heuristic:
+      // Assume that <iframe> and <frame> elements contain a html document,
+      // and other elements (i.e. <object>) contain plugins or other resources.
+      // If the heuristic is wrong (i.e. the remote frame in <object> does
+      // contain an html document), then things will still work, but with the
+      // following caveats: 1) original frame content will be saved and 2) links
+      // in frame's html doc will not be rewritten to point to locally saved
+      // files.
+      frameContainsHtmlDoc = element.hasHTMLTagName("iframe") ||
+                             element.hasHTMLTagName("frame");
+    }
+
+    if (frameContainsHtmlDoc) {
+      SavableSubframe subframe;
+      subframe.original_url = element_url;
+      subframe.routing_id = GetRoutingIdForFrameOrProxy(web_frame);
+      result->subframes->push_back(subframe);
+      return;
+    }
+  }
+
   // ignore invalid URL
-  if (!u.is_valid())
+  if (!element_url.is_valid())
     return;
+
   // Ignore those URLs which are not standard protocols. Because FTP
   // protocol does no have cache mechanism, we will skip all
   // sub-resources if they use FTP protocol.
-  if (!u.SchemeIsHTTPOrHTTPS() && !u.SchemeIs(url::kFileScheme))
+  if (!element_url.SchemeIsHTTPOrHTTPS() &&
+      !element_url.SchemeIs(url::kFileScheme))
     return;
 
-  result->resources_list->push_back(u);
+  result->resources_list->push_back(element_url);
 }
 
 }  // namespace
 
 bool GetSavableResourceLinksForFrame(WebFrame* current_frame,
                                      SavableResourcesResult* result,
                                      const char** savable_schemes) {
   // Get current frame's URL.
   GURL current_frame_url = current_frame->document().url();
 
@@ skipping 23 matching lines @@
                                      current_doc,
                                      result);
   }
 
   return true;
 }
 
 WebString GetSubResourceLinkFromElement(const WebElement& element) {
   const char* attribute_name = NULL;
   if (element.hasHTMLTagName("img") ||
+      element.hasHTMLTagName("frame") ||
+      element.hasHTMLTagName("iframe") ||
       element.hasHTMLTagName("script")) {
     attribute_name = "src";
   } else if (element.hasHTMLTagName("input")) {
     const WebInputElement input = element.toConst<WebInputElement>();
     if (input.isImageButton()) {
       attribute_name = "src";
     }
   } else if (element.hasHTMLTagName("body") ||
              element.hasHTMLTagName("table") ||
              element.hasHTMLTagName("tr") ||
              element.hasHTMLTagName("td")) {
     attribute_name = "background";
   } else if (element.hasHTMLTagName("blockquote") ||
              element.hasHTMLTagName("q") ||
              element.hasHTMLTagName("del") ||
              element.hasHTMLTagName("ins")) {
     attribute_name = "cite";
   } else if (element.hasHTMLTagName("object")) {
-    // TODO(lukasza): When <object> contains a html document, it should be
-    // reported as a subframe, not as a savable resource (reporting as a
-    // savable resource works, but will save original html contents, not
-    // current html contents of the frame).
     attribute_name = "data";
   } else if (element.hasHTMLTagName("link")) {
     // If the link element is not linked to css, ignore it.
     if (base::LowerCaseEqualsASCII(
             base::StringPiece16(element.getAttribute("type")), "text/css") ||
         base::LowerCaseEqualsASCII(
             base::StringPiece16(element.getAttribute("rel")), "stylesheet")) {
       // TODO(jnd): Add support for extracting links of sub-resources which
       // are inside style-sheet such as @import, url(), etc.
       // See bug: http://b/issue?id=1111667.
       attribute_name = "href";
     }
   }
   if (!attribute_name)
     return WebString();
   WebString value = element.getAttribute(WebString::fromUTF8(attribute_name));
   // If value has content and not start with "javascript:" then return it,
   // otherwise return NULL.
   if (!value.isNull() && !value.isEmpty() &&
       !base::StartsWith(value.utf8(), "javascript:",
                         base::CompareCase::INSENSITIVE_ASCII))
     return value;
 
   return WebString();
 }
 
 }  // namespace content