ABANDONED: OOPIFs: Moving stitching of local paths from renderer to browser process.

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 96871950bac431327eb405ebbbcbef4711ef8d96..2ffa24f1cdad2b53b0df1abdd97da9acb472a2b1 100644
--- a/content/browser/download/save_package.cc
+++ b/content/browser/download/save_package.cc
@@ -25,12 +25,14 @@
 #include "content/browser/download/save_file.h"
 #include "content/browser/download/save_file_manager.h"
 #include "content/browser/download/save_item.h"
+#include "content/browser/frame_host/frame_tree.h"
+#include "content/browser/frame_host/frame_tree_node.h"
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
 #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/browser/web_contents/web_contents_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"
 #include "content/public/browser/content_browser_client.h"
@@ -131,6 +133,11 @@ class SavePackageRequestHandle : public DownloadRequestHandleInterface {
   base::WeakPtr<SavePackage> save_package_;
 };
 
+int GetFrameTreeNodeId(RenderFrameHost* render_frame_host) {
+  auto rfhi = static_cast<RenderFrameHostImpl*>(render_frame_host);
+  return rfhi->frame_tree_node()->frame_tree_node_id();
+}
+
 }  // namespace
 
 const base::FilePath::CharType SavePackage::kDefaultHtmlExtension[] =
@@ -349,10 +356,9 @@ void SavePackage::InitWithDownloadItem(
     SaveFileCreateInfo::SaveFileSource save_source = page_url_.SchemeIsFile() ?
         SaveFileCreateInfo::SAVE_FILE_FROM_FILE :
         SaveFileCreateInfo::SAVE_FILE_FROM_NET;
-    SaveItem* save_item = new SaveItem(page_url_,
-                                       Referrer(),
-                                       this,
-                                       save_source);
+    SaveItem* save_item = new SaveItem(page_url_, this, save_source);
+    save_item->set_frame_tree_node_id(
+        GetFrameTreeNodeId(web_contents()->GetMainFrame()));
     // Add this item to waiting list.
     waiting_item_queue_.push(save_item);
     all_save_items_count_ = 1;
@@ -1011,8 +1017,14 @@ bool SavePackage::OnMessageReceived(const IPC::Message& message,
                         OnSavableResourceLinksResponse)
     IPC_MESSAGE_HANDLER(FrameHostMsg_SavableResourceLinksError,
                         OnSavableResourceLinksError)
-    IPC_MESSAGE_HANDLER(FrameHostMsg_SerializedHtmlWithLocalLinksResponse,
-                        OnSerializedHtmlWithLocalLinksResponse)
+    IPC_MESSAGE_HANDLER(FrameHostMsg_SerializedHtmlFragment,
+                        OnSerializedHtmlFragment)
+    IPC_MESSAGE_HANDLER(FrameHostMsg_SerializedLocalPathForSubFrame,
+                        OnSerializedLocalPathForSubFrame)
+    IPC_MESSAGE_HANDLER(FrameHostMsg_SerializedLocalPathForSavableResource,
+                        OnSerializedLocalPathForSavableResource)
+    IPC_MESSAGE_HANDLER(FrameHostMsg_SerializedEndOfFrame,
+                        OnSerializedEndOfFrame)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
@@ -1025,103 +1037,50 @@ bool SavePackage::OnMessageReceived(const IPC::Message& message,
 void SavePackage::GetSerializedHtmlWithLocalLinks() {
   if (wait_state_ != HTML_DATA)
     return;
-  std::vector<GURL> saved_links;
-  std::vector<base::FilePath> saved_file_paths;
   int successful_started_items_count = 0;
 
-  // Collect all saved items which have local storage.
-  // First collect the status of all the resource files and check whether they
-  // have created local files although they have not been completely saved.
-  // If yes, the file can be saved. Otherwise, there is a disk error, so we
-  // need to cancel the page saving job.
+  // Count all saved items which have local storage.
   for (SaveUrlItemMap::iterator it = in_progress_items_.begin();
        it != in_progress_items_.end(); ++it) {
     DCHECK(it->second->save_source() ==
            SaveFileCreateInfo::SAVE_FILE_FROM_DOM);
     if (it->second->has_final_name())
       successful_started_items_count++;
-    saved_links.push_back(it->second->url());
-    saved_file_paths.push_back(it->second->file_name());
   }
 
   // If not all file of HTML resource have been started, then wait.
   if (successful_started_items_count != in_process_count())
     return;
 
-  // Collect all saved success items.
-  for (SavedItemMap::iterator it = saved_success_items_.begin();
-       it != saved_success_items_.end(); ++it) {
-    DCHECK(it->second->has_final_name());
-    saved_links.push_back(it->second->url());
-    saved_file_paths.push_back(it->second->file_name());
-  }
-
-  // Get the relative directory name.
-  base::FilePath relative_dir_name = saved_main_directory_path_.BaseName();
-
   // Ask all frames for their serialized data.
   DCHECK_EQ(0, number_of_frames_pending_response_);
   web_contents()->ForEachFrame(base::Bind(
       &SavePackage::GetSerializedHtmlWithLocalLinksForFrame,
-      base::Unretained(this),  // Safe, because ForEachFrame is synchronous.
-      saved_links, saved_file_paths, relative_dir_name));
+      base::Unretained(this)));  // Safe, because ForEachFrame is synchronous.
   DCHECK_LT(0, number_of_frames_pending_response_);
 }
 
 void SavePackage::GetSerializedHtmlWithLocalLinksForFrame(
-    const std::vector<GURL>& saved_links,
-    const std::vector<base::FilePath>& saved_file_paths,
-    const base::FilePath& relative_dir_name,
     RenderFrameHost* target) {
   number_of_frames_pending_response_++;
-  target->Send(new FrameMsg_GetSerializedHtmlWithLocalLinks(
-      target->GetRoutingID(), saved_links, saved_file_paths,
-      relative_dir_name));
+  target->Send(
+      new FrameMsg_GetSerializedHtmlWithLocalLinks(target->GetRoutingID()));
 }
 
-// Process the serialized HTML content data of a specified frame
-// retrieved from the renderer process.
-void SavePackage::OnSerializedHtmlWithLocalLinksResponse(
-    RenderFrameHost* sender,
-    const GURL& frame_url,
-    const std::string& data,
-    int32 status) {
-  WebPageSerializerClient::PageSerializationStatus flag =
-      static_cast<WebPageSerializerClient::PageSerializationStatus>(status);
-
-  // When calling WebPageSerializer::serialize in non-recursive mode, the
-  // AllFramesAreFinished is redundant - it is sent by each frame right after
-  // CurrentFrameIsFinished.  Therefore we ignore AllFramesAreFinished and
-  // instead track pending frames in |number_of_frames_pending_response_|.
-  if (flag == WebPageSerializerClient::AllFramesAreFinished)
-    return;
-
-  // Check current state.
+void SavePackage::OnSerializedHtmlFragment(RenderFrameHost* sender,
+                                           const std::string& data) {
   if (wait_state_ != HTML_DATA)
     return;
 
-  int id = contents_id();
-
-  SaveUrlItemMap::iterator it = in_progress_items_.find(frame_url.spec());
-  if (it == in_progress_items_.end()) {
-    for (SavedItemMap::iterator saved_it = saved_success_items_.begin();
-      saved_it != saved_success_items_.end(); ++saved_it) {
-      if (saved_it->second->url() == frame_url) {
-        wrote_to_completed_file_ = true;
-        break;
-      }
-    }
-
-    it = saved_failed_items_.find(frame_url.spec());
-    if (it != saved_failed_items_.end())
-      wrote_to_failed_file_ = true;
-
+  int frame_tree_node_id = GetFrameTreeNodeId(sender);
+  SaveItem* save_item =
+      FindInProgressSaveItemByFrameTreeNodeID(frame_tree_node_id);
+  if (!save_item) {
+    LogWriteToAlreadyCompletedOrFailedSaveItem(frame_tree_node_id);
     return;
   }
 
-  SaveItem* save_item = it->second;
   DCHECK(save_item->save_source() == SaveFileCreateInfo::SAVE_FILE_FROM_DOM);
-
   if (!data.empty()) {
     // Prepare buffer for saving HTML data.
     scoped_refptr<net::IOBuffer> new_data(new net::IOBuffer(data.size()));
@@ -1130,46 +1089,141 @@ void SavePackage::OnSerializedHtmlWithLocalLinksResponse(
     // Call write file functionality in file thread.
     BrowserThread::PostTask(
         BrowserThread::FILE, FROM_HERE,
-        base::Bind(&SaveFileManager::UpdateSaveProgress,
-                   file_manager_,
-                   save_item->save_id(),
-                   new_data,
+        base::Bind(&SaveFileManager::UpdateSaveProgress, file_manager_,
+                   save_item->save_id(), new_data,
                    static_cast<int>(data.size())));
   }
+}
 
-  // Current frame is completed saving, call finish in file thread.
-  if (flag == WebPageSerializerClient::CurrentFrameIsFinished) {
-    DVLOG(20) << " " << __FUNCTION__ << "()"
-              << " save_id = " << save_item->save_id()
-              << " url = \"" << save_item->url().spec() << "\"";
-    BrowserThread::PostTask(
-        BrowserThread::FILE, FROM_HERE,
-        base::Bind(&SaveFileManager::SaveFinished,
-                   file_manager_,
-                   save_item->save_id(),
-                   save_item->url(),
-                   id,
-                   true));
-    number_of_frames_pending_response_--;
-    DCHECK_LE(0, number_of_frames_pending_response_);
+void SavePackage::OnSerializedLocalPathForSubFrame(
+    RenderFrameHost* sender,
+    int render_frame_or_proxy_routing_id) {
+  if (wait_state_ != HTML_DATA)
+    return;
+
+  FrameTreeNode* frame_tree_node =

[dcheng, 2015/09/30 05:37:32]

It seems like this code would benefit from a FTN -> SaveItem map.

Also, I think that frames should probably be indexed by FTN ID rather than the
frame URL, but that can change in a separate patch.

[Łukasz Anforowicz, 2015/10/01 20:39:58]

Good point.  Done.

I also found out that looking inside in_progress_items_ was wrong (because a
subresource or subframe might have already finished saving and be moved from
in_progress_items_ into saved_success_items_).  Having a map that points to
SaveItem* helps with this as well.

Introducing the map also means that I don't really need to modify SaveItem to
store frame_tree_node_id_ as a field.  I am keeping other SaveItem modifications
(to set referrer separately from the construction);  OTOH, preferring
referrer-from-savable-resources might not be needed once we don't dedupe frames
based on their URL (but then you are right that this should probably be in a
separate CL).
Right.  I've added a TODO and opened crbug.com/538188 to track this.

+      static_cast<WebContentsImpl*>(web_contents())
+          ->GetFrameTree()
+          ->FindByRoutingID(sender->GetProcess()->GetID(),
+                            render_frame_or_proxy_routing_id);
+  if (!frame_tree_node)
+    return;
+
+  SerializeLocalPathForSaveItem(sender,
+                                FindInProgressSaveItemByFrameTreeNodeID(
+                                    frame_tree_node->frame_tree_node_id()),
+                                frame_tree_node->current_url());
+}
+
+void SavePackage::OnSerializedLocalPathForSavableResource(
+    RenderFrameHost* sender,
+    const GURL& savable_resource) {
+  if (wait_state_ != HTML_DATA)
+    return;
+  if (!savable_resource.is_valid())
+    return;
+
+  // Find SaveItem by URL.
+  auto it =
+      std::find_if(in_progress_items_.begin(), in_progress_items_.end(),
+                   [&savable_resource](const SaveUrlItemMap::value_type& v) {
+                     DCHECK(v.second->has_final_name());
+                     return v.second->url() == savable_resource;
+                   });
+  SaveItem* save_item = (it == in_progress_items_.end()) ? nullptr : it->second;
+
+  SerializeLocalPathForSaveItem(sender, save_item, savable_resource);
+}
+
+void SavePackage::OnSerializedEndOfFrame(RenderFrameHost* sender) {
+  if (wait_state_ != HTML_DATA)
+    return;
+
+  int frame_tree_node_id = GetFrameTreeNodeId(sender);
+  SaveItem* save_item =
+      FindInProgressSaveItemByFrameTreeNodeID(frame_tree_node_id);
+  if (!save_item) {
+    LogWriteToAlreadyCompletedOrFailedSaveItem(frame_tree_node_id);
+    return;
   }
 
-  // If all frames are finished saving, we need to close the remaining
-  // SaveItems.
+  int id = contents_id();
+
+  // Current frame is completed saving, call finish in file thread.
+  DVLOG(20) << " " << __FUNCTION__ << "()"
+            << " save_id = " << save_item->save_id() << " url = \""
+            << save_item->url().spec() << "\"";
+  BrowserThread::PostTask(
+      BrowserThread::FILE, FROM_HERE,
+      base::Bind(&SaveFileManager::SaveFinished, file_manager_,
+                 save_item->save_id(), save_item->url(), id, true));
+  number_of_frames_pending_response_--;
+  DCHECK_LE(0, number_of_frames_pending_response_);
+
+  // If the all frames are finished saving, we need to close the
+  // remaining SaveItems.
   if (number_of_frames_pending_response_ == 0) {
-    for (SaveUrlItemMap::iterator it = in_progress_items_.begin();
-         it != in_progress_items_.end(); ++it) {
+    for (const auto pair : in_progress_items_) {

[dcheng, 2015/09/30 05:37:32]

const auto&, since this copies otherwise.

       DVLOG(20) << " " << __FUNCTION__ << "()"
-                << " save_id = " << it->second->save_id() << " url = \""
-                << it->second->url().spec() << "\"";
+                << " save_id = " << pair.second->save_id() << " url = \""
+                << pair.second->url().spec() << "\"";
       BrowserThread::PostTask(
           BrowserThread::FILE, FROM_HERE,
           base::Bind(&SaveFileManager::SaveFinished, file_manager_,
-                     it->second->save_id(), it->second->url(), id, true));
+                     pair.second->save_id(), pair.second->url(), id, true));
     }
   }
 }
 
+void SavePackage::SerializeLocalPathForSaveItem(RenderFrameHost* sender,
+                                                SaveItem* save_item,
+                                                const GURL& fallback_url) {
+  std::string attr_value;
+  if (save_item) {
+    base::FilePath relative_path(FILE_PATH_LITERAL("."));
+    relative_path = relative_path.Append(saved_main_directory_path_.BaseName());
+    relative_path = relative_path.Append(save_item->file_name());
+    attr_value = relative_path.AsUTF8Unsafe();
+  } else {
+    attr_value = fallback_url.spec();
+  }
+
+  // TODO(lukasza): Escape attr_value (&quot; + &amp;).
+  OnSerializedHtmlFragment(sender, attr_value);
+}
+
+SaveItem* SavePackage::FindInProgressSaveItemByFrameTreeNodeID(
+    int frame_tree_node_id) {
+  auto it = std::find_if(
+      in_progress_items_.begin(), in_progress_items_.end(),
+      [frame_tree_node_id](const SaveUrlItemMap::value_type& v) {
+        DCHECK(v.second->has_final_name());
+        return v.second->frame_tree_node_id() == frame_tree_node_id;
+      });
+
+  return (it == in_progress_items_.end()) ? nullptr : it->second;
+}
+
+void SavePackage::LogWriteToAlreadyCompletedOrFailedSaveItem(
+    int frame_tree_node_id) {
+  auto already_completed_it = std::find_if(
+      saved_success_items_.begin(), saved_success_items_.end(),
+      [frame_tree_node_id](const SavedItemMap::value_type& v) {
+        return v.second->frame_tree_node_id() == frame_tree_node_id;
+      });
+  if (already_completed_it != saved_success_items_.end())
+    wrote_to_completed_file_ = true;
+
+  auto failed_it = std::find_if(
+      saved_failed_items_.begin(), saved_failed_items_.end(),
+      [frame_tree_node_id](const SaveUrlItemMap::value_type& v) {
+        DCHECK(v.second->has_final_name());
+        return v.second->frame_tree_node_id() == frame_tree_node_id;
+      });
+  if (failed_it != saved_failed_items_.end())
+    wrote_to_failed_file_ = true;
+}
+
 // Ask for all savable resource links from backend, include main frame and
 // sub-frame.
 void SavePackage::GetSavableResourceLinks() {
@@ -1206,20 +1260,26 @@ void SavePackage::OnSavableResourceLinksResponse(
     const GURL& u = resources_list[i];
     if (!u.is_valid())
       continue;
-    if (unique_urls_to_save_.count(u))
-      continue;
-    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);
+    SaveItem*& save_item = url_to_save_item_[u];

[dcheng, 2015/09/30 05:37:32]

I think this is a bit too clever =)

[Łukasz Anforowicz, 2015/10/01 20:39:58]

Done.  (fixed by assigning to a new invocation of map[], rather than assigning
to the stored reference)

+    if (save_item == nullptr) {
+      SaveFileCreateInfo::SaveFileSource save_source =
+          u.SchemeIsFile() ? SaveFileCreateInfo::SAVE_FILE_FROM_FILE
+                           : SaveFileCreateInfo::SAVE_FILE_FROM_NET;
+      save_item = new SaveItem(u, this, save_source);
+      waiting_item_queue_.push(save_item);
+    }
+    save_item->set_referrer(referrers_list[i]);
   }
 
-  // Store savable frame_url for later processing.
-  if (frame_url.is_valid())
-    frame_urls_to_save_.push_back(frame_url);
+  // Add the frame to wait list.
+  SaveItem*& save_item = url_to_save_item_[frame_url];
+  if (save_item == nullptr) {
+    save_item =
+        new SaveItem(frame_url, this, SaveFileCreateInfo::SAVE_FILE_FROM_DOM);
+    waiting_item_queue_.push(save_item);
+  }
+  save_item->set_frame_tree_node_id(GetFrameTreeNodeId(sender));
 
   CompleteSavableResourceLinksResponse();
 }
@@ -1234,20 +1294,6 @@ void SavePackage::CompleteSavableResourceLinksResponse() {
   if (number_of_frames_pending_response_ != 0)
     return;  // Need to wait for more responses from RenderFrames.
 
-  // Add frame urls to the waiting_item_queue_.  This is done *after* processing
-  // all savable resource links (i.e. in OnSavableResourceLinksResponse), to
-  // prefer their referrers in cases where the frame url has already been
-  // covered by savable resource links.
-  for (auto& frame_url : frame_urls_to_save_) {
-    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);
-    }
-  }
-
   all_save_items_count_ = static_cast<int>(waiting_item_queue_.size());
 
   // We use total bytes as the total number of files we want to save.