OOPIFs: Transitioning Get/Send...SavableResourceLinks away from RenderViewHost.

content/renderer/savable_resources.cc

Index: content/renderer/savable_resources.cc
diff --git a/content/renderer/savable_resources.cc b/content/renderer/savable_resources.cc
index 1a9612dd97a39df0cf54ebdbf2e6818d2496fa01..7e70109d9c1b98a4809c68f11304d9c58fb00268 100644
--- a/content/renderer/savable_resources.cc
+++ b/content/renderer/savable_resources.cc
@@ -35,46 +35,17 @@ using blink::WebView;
 namespace content {
 namespace {
 
-// Structure for storage the unique set of all savable resource links for
-// making sure that no duplicated resource link in final result. The consumer
-// of the SavableResourcesUniqueCheck is responsible for keeping these pointers
-// valid for the lifetime of the SavableResourcesUniqueCheck instance.
-struct SavableResourcesUniqueCheck {
-  // Unique set of all sub resource links.
-  std::set<GURL>* resources_set;
-  // Unique set of all frame links.
-  std::set<GURL>* frames_set;
-  // Collection of all frames we go through when getting all savable resource
-  // links.
-  std::vector<WebFrame*>* frames;
-
-  SavableResourcesUniqueCheck()
-      : resources_set(NULL),
-        frames_set(NULL),
-        frames(NULL) {}
-
-  SavableResourcesUniqueCheck(std::set<GURL>* resources_set,
-      std::set<GURL>* frames_set, std::vector<WebFrame*>* frames)
-      : resources_set(resources_set),
-        frames_set(frames_set),
-        frames(frames) {}
-};
-
 // 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.
 void GetSavableResourceLinkForElement(
     const WebElement& element,
     const WebDocument& current_doc,
-    SavableResourcesUniqueCheck* unique_check,
     SavableResourcesResult* result) {
 
   // Handle frame and iframe tag.
   if (element.hasHTMLTagName("iframe") ||
       element.hasHTMLTagName("frame")) {

[nasko, 2015/09/02 23:45:55]

nit: The above fits on one line. Also might be good to update the comment that
we aren't handling the iframe, but rather skipping it.

[Łukasz Anforowicz, 2015/09/03 16:59:57]

Good point.  Done.

-    WebFrame* sub_frame = WebLocalFrame::fromFrameOwnerElement(element);
-    if (sub_frame)
-      unique_check->frames->push_back(sub_frame);
     return;
   }
 
@@ -93,25 +64,23 @@ void GetSavableResourceLinkForElement(
   if (!u.SchemeIsHTTPOrHTTPS() && !u.SchemeIs(url::kFileScheme))
     return;
   // Ignore duplicated resource link.
-  if (!unique_check->resources_set->insert(u).second)
-    return;
   result->resources_list->push_back(u);
   // Insert referrer for above new resource link.
   result->referrer_urls_list->push_back(GURL());
   result->referrer_policies_list->push_back(blink::WebReferrerPolicyDefault);
 }
 
-// Get all savable resource links from current WebFrameImpl object pointer.
-void GetAllSavableResourceLinksForFrame(WebFrame* current_frame,
-    SavableResourcesUniqueCheck* unique_check,
-    SavableResourcesResult* result,
-    const char** savable_schemes) {
+}  // 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();
 
   // If url of current frame is invalid, ignore it.
   if (!current_frame_url.is_valid())
-    return;
+    return false;
 
   // If url of current frame is not a savable protocol, ignore it.
   bool is_valid_protocol = false;
@@ -122,11 +91,7 @@ void GetAllSavableResourceLinksForFrame(WebFrame* current_frame,
     }
   }
   if (!is_valid_protocol)
-    return;
-
-  // If find same frame we have recorded, ignore it.
-  if (!unique_check->frames_set->insert(current_frame_url).second)
-    return;
+    return false;
 
   // Get current using document.
   WebDocument current_doc = current_frame->document();
@@ -137,12 +102,11 @@ void GetAllSavableResourceLinksForFrame(WebFrame* current_frame,
        element = all.nextItem()) {
     GetSavableResourceLinkForElement(element,
                                      current_doc,
-                                     unique_check,
                                      result);
   }
-}
 
-}  // namespace
+  return true;
+}
 
 WebString GetSubResourceLinkFromElement(const WebElement& element) {
   const char* attribute_name = NULL;
@@ -189,51 +153,4 @@ WebString GetSubResourceLinkFromElement(const WebElement& element) {
   return WebString();
 }
 
-// Get all savable resource links from current webview, include main
-// frame and sub-frame
-bool GetAllSavableResourceLinksForCurrentPage(WebView* view,
-    const GURL& page_url, SavableResourcesResult* result,
-    const char** savable_schemes) {
-  WebFrame* main_frame = view->mainFrame();
-  if (!main_frame)
-    return false;
-
-  std::set<GURL> resources_set;
-  std::set<GURL> frames_set;
-  std::vector<WebFrame*> frames;
-  SavableResourcesUniqueCheck unique_check(&resources_set,
-                                           &frames_set,
-                                           &frames);
-
-  GURL main_page_gurl(main_frame->document().url());
-
-  // Make sure we are saving same page between embedder and webkit.
-  // If page has being navigated, embedder will get three empty vector,
-  // which will make the saving page job ended.
-  if (page_url != main_page_gurl)
-    return true;
-
-  // First, process main frame.
-  frames.push_back(main_frame);
-
-  // Check all resource in this page, include sub-frame.
-  for (int i = 0; i < static_cast<int>(frames.size()); ++i) {
-    // Get current frame's all savable resource links.
-    GetAllSavableResourceLinksForFrame(frames[i], &unique_check, result,
-                                       savable_schemes);
-  }
-
-  // Since frame's src can also point to sub-resources link, so it is possible
-  // that some URLs in frames_list are also in resources_list. For those
-  // URLs, we will remove it from frame_list, only keep them in resources_list.
-  for (std::set<GURL>::iterator it = frames_set.begin();
-       it != frames_set.end(); ++it) {
-    // Append unique frame source to savable frame list.
-    if (resources_set.find(*it) == resources_set.end())
-      result->frames_list->push_back(*it);
-  }
-
-  return true;
-}
-
 }  // namespace content