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

content/browser/download/save_package.cc

Index: content/browser/download/save_package.cc
diff --git a/content/browser/download/save_package.cc b/content/browser/download/save_package.cc
index 36c6be64a0f3c066ef2a766a72f57240dae0bbac..77329830a00051cd52d4976fea54de7140d382da 100644
--- a/content/browser/download/save_package.cc
+++ b/content/browser/download/save_package.cc
@@ -29,6 +29,7 @@
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/renderer_host/render_view_host_delegate.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
+#include "content/common/frame_messages.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
@@ -336,7 +337,7 @@ void SavePackage::InitWithDownloadItem(
   if (save_type_ == SAVE_PAGE_TYPE_AS_COMPLETE_HTML) {
     // Get directory
     DCHECK(!saved_main_directory_path_.empty());
-    GetAllSavableResourceLinksForCurrentPage();
+    GetSavableResourceLinksForCurrentPage();
   } else if (save_type_ == SAVE_PAGE_TYPE_AS_MHTML) {
     web_contents()->GenerateMHTML(saved_main_file_path_, base::Bind(
         &SavePackage::OnMHTMLGenerated, this));
@@ -1003,8 +1004,6 @@ void SavePackage::DoSavingProcess() {
 bool SavePackage::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(SavePackage, message)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_SendCurrentPageAllSavableResourceLinks,
-                        OnReceivedSavableResourceLinksForCurrentPage)
     IPC_MESSAGE_HANDLER(ViewHostMsg_SendSerializedHtmlData,
                         OnReceivedSerializedHtmlData)
     IPC_MESSAGE_UNHANDLED(handled = false)
@@ -1012,6 +1011,19 @@ bool SavePackage::OnMessageReceived(const IPC::Message& message) {
   return handled;
 }
 
+bool SavePackage::OnMessageReceived(const IPC::Message& message,
+                                    RenderFrameHost* render_frame_host) {
+  bool handled = true;
+  IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(SavePackage, message, render_frame_host)
+    IPC_MESSAGE_HANDLER(FrameHostMsg_SavableResourceLinksResponse,
+                        OnReceivedSavableResourceLinksForFrame)
+    IPC_MESSAGE_HANDLER(FrameHostMsg_NonSavableResponse,
+                        OnReceivedNonSavableFrameIndication)
+    IPC_MESSAGE_UNHANDLED(handled = false)
+  IPC_END_MESSAGE_MAP()
+  return handled;
+}
+
 // After finishing all SaveItems which need to get data from net.
 // We collect all URLs which have local storage and send the
 // map:(originalURL:currentLocalPath) to render process (backend).
@@ -1142,30 +1154,87 @@ void SavePackage::OnReceivedSerializedHtmlData(const GURL& frame_url,
 
 // Ask for all savable resource links from backend, include main frame and
 // sub-frame.
-void SavePackage::GetAllSavableResourceLinksForCurrentPage() {
+void SavePackage::GetSavableResourceLinksForCurrentPage() {
   if (wait_state_ != START_PROCESS)
     return;
 
+  WebContents* web_contents = WebContentsObserver::web_contents();
+  if (!web_contents) {
+    // Tab got closed - treat it as user cancel.
+    Cancel(true);
+    return;
+  }
+
   wait_state_ = RESOURCES_LIST;
-  Send(new ViewMsg_GetAllSavableResourceLinksForCurrentPage(routing_id(),
-                                                            page_url_));
+  number_of_frames_with_pending_get_savable_resource_links_ = 0;
+  web_contents->ForEachFrame(base::Bind(
+      &SavePackage::GetSavableResourceLinksForFrame, base::Unretained(this)));
+
+  if (0 == number_of_frames_with_pending_get_savable_resource_links_) {
+    // No frames in the web contents - treat it as user cancel.
+    Cancel(true);
+    return;
+  }
+}
+
+void SavePackage::GetSavableResourceLinksForFrame(
+    RenderFrameHost* render_frame_host) {
+  number_of_frames_with_pending_get_savable_resource_links_++;
+  Send(new FrameMsg_GetSavableResourceLinks(render_frame_host->GetRoutingID()));
 }
 
-// Give backend the lists which contain all resource links that have local
-// storage, after which, render process will serialize DOM for generating
-// HTML data.
-void SavePackage::OnReceivedSavableResourceLinksForCurrentPage(
+void SavePackage::OnReceivedSavableResourceLinksForFrame(
+    RenderFrameHost* render_frame_host,
     const std::vector<GURL>& resources_list,
-    const std::vector<Referrer>& referrers_list,
-    const std::vector<GURL>& frames_list) {
+    const std::vector<Referrer>& referrers_list) {
   if (wait_state_ != RESOURCES_LIST)
     return;
 
   if (resources_list.size() != referrers_list.size())
     return;
 
-  all_save_items_count_ = static_cast<int>(resources_list.size()) +
-                           static_cast<int>(frames_list.size());
+  // Add all sub-resources to wait list.
+  for (int i = 0; i < static_cast<int>(resources_list.size()); ++i) {
+    const GURL& u = resources_list[i];
+    DCHECK(u.is_valid());
+    if (unique_urls_to_save_.count(u)) {
+      continue;

[Łukasz Anforowicz, 2015/09/02 18:03:34]

In some cases referrers_list[i] might be preferred over what was already saved
before (and recorded in waiting_item_queue_ and unique_urls_to_save_).  This is
because we can get a savable url multiple times from either resources_list of
from frame_url - each of those savable urls can come with a different referrer. 
We probably should pick the "best" referrer.

I don't know what "best" means, but the default referrer created for frame_url
case below is probably not "best".

Note that the old code preferred referrers from resources_list over ones from
frame_list, but still more or less randomly (via SavableResourcesUniqueCheck)
picked referrers from within the resources_list (without comparing which
referrer is "best").

+    }
+    unique_urls_to_save_.insert(u);
+
+    SaveFileCreateInfo::SaveFileSource save_source =
+        u.SchemeIsFile() ? SaveFileCreateInfo::SAVE_FILE_FROM_FILE
+                         : SaveFileCreateInfo::SAVE_FILE_FROM_NET;
+    SaveItem* save_item = new SaveItem(u, referrers_list[i], this, save_source);
+    waiting_item_queue_.push(save_item);
+  }
+
+  // Add the frame to wait list.
+  GURL frame_url = render_frame_host->GetLastCommittedURL();
+  DCHECK(frame_url.is_valid());
+  if (0 == unique_urls_to_save_.count(frame_url)) {
+    unique_urls_to_save_.insert(frame_url);
+    SaveItem* save_item = new SaveItem(frame_url, Referrer(), this,
+                                       SaveFileCreateInfo::SAVE_FILE_FROM_DOM);
+    waiting_item_queue_.push(save_item);
+  }
+
+  CompleteSavableResourceLinksResponseFromFrame();
+}
+
+void SavePackage::OnReceivedNonSavableFrameIndication(
+    RenderFrameHost* render_frame_host) {
+  CompleteSavableResourceLinksResponseFromFrame();
+}
+
+void SavePackage::CompleteSavableResourceLinksResponseFromFrame() {
+  --number_of_frames_with_pending_get_savable_resource_links_;
+  if (0 < number_of_frames_with_pending_get_savable_resource_links_) {
+    return;
+  }
+  DCHECK_EQ(0, number_of_frames_with_pending_get_savable_resource_links_);
+
+  all_save_items_count_ = static_cast<int>(unique_urls_to_save_.size());
 
   // We use total bytes as the total number of files we want to save.
   // Hack to avoid touching download_ after user cancel.
@@ -1175,26 +1244,11 @@ void SavePackage::OnReceivedSavableResourceLinksForCurrentPage(
     download_->SetTotalBytes(all_save_items_count_);
 
   if (all_save_items_count_) {
-    // Put all sub-resources to wait list.
-    for (int i = 0; i < static_cast<int>(resources_list.size()); ++i) {
-      const GURL& u = resources_list[i];
-      DCHECK(u.is_valid());
-      SaveFileCreateInfo::SaveFileSource save_source = u.SchemeIsFile() ?
-          SaveFileCreateInfo::SAVE_FILE_FROM_FILE :
-          SaveFileCreateInfo::SAVE_FILE_FROM_NET;
-      SaveItem* save_item = new SaveItem(u, referrers_list[i],
-                                         this, save_source);
-      waiting_item_queue_.push(save_item);
-    }
-    // Put all HTML resources to wait list.
-    for (int i = 0; i < static_cast<int>(frames_list.size()); ++i) {
-      const GURL& u = frames_list[i];
-      DCHECK(u.is_valid());
-      SaveItem* save_item = new SaveItem(
-          u, Referrer(), this, SaveFileCreateInfo::SAVE_FILE_FROM_DOM);
-      waiting_item_queue_.push(save_item);
-    }
     wait_state_ = NET_FILES;
+
+    // Give backend the lists which contain all resource links that have local
+    // storage, after which, render process will serialize DOM for generating
+    // HTML data.
     DoSavingProcess();
   } else {
     // No resource files need to be saved, treat it as user cancel.