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..26426c729b6d2529f9dcba30233036be155317a6 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);

[dcheng, 2015/10/05 07:53:08]

Nit: my personal preference is to annotate autos holding pointers with auto* to
make it 100% clear.

[Łukasz Anforowicz, 2015/10/05 17:57:56]

Done.

+  return rfhi->frame_tree_node()->frame_tree_node_id();
+}
+
 }  // namespace
 
 const base::FilePath::CharType SavePackage::kDefaultHtmlExtension[] =
@@ -239,15 +246,9 @@ SavePackage::~SavePackage() {
                                    completed_count() +
                                    in_process_count()));
   // Free all SaveItems.
-  while (!waiting_item_queue_.empty()) {
-    // We still have some items which are waiting for start to save.
-    SaveItem* save_item = waiting_item_queue_.front();
-    waiting_item_queue_.pop();
-    delete save_item;
-  }
-
-  STLDeleteValues(&saved_success_items_);
+  STLDeleteElements(&waiting_item_queue_);
   STLDeleteValues(&in_progress_items_);
+  STLDeleteValues(&saved_success_items_);
   STLDeleteValues(&saved_failed_items_);
 
   file_manager_ = NULL;
@@ -349,12 +350,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);
     // Add this item to waiting list.
-    waiting_item_queue_.push(save_item);
+    waiting_item_queue_.push_back(save_item);
     all_save_items_count_ = 1;
     download_->SetTotalBytes(1);
 
@@ -925,7 +923,7 @@ void SavePackage::SaveNextFile(bool process_all_remaining_items) {
   do {
     // Pop SaveItem from waiting list.
     SaveItem* save_item = waiting_item_queue_.front();
-    waiting_item_queue_.pop();
+    waiting_item_queue_.pop_front();
 
     // Add the item to in_progress_items_.
     SaveUrlItemMap::iterator it = in_progress_items_.find(
@@ -1011,8 +1009,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 +1029,47 @@ 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.
-  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())
+  // Count all saved items which have local storage.
+  for (const auto& it : in_progress_items_) {
+    DCHECK_EQ(it.second->save_source(), SaveFileCreateInfo::SAVE_FILE_FROM_DOM);

[dcheng, 2015/10/05 07:53:08]

Nit: there's a weak convention of DCHECK_EQ(expected, actual) to match how the
gtest macros work.

[Łukasz Anforowicz, 2015/10/05 17:57:56]

Thanks for catching this.  I knew about this convention and somehow managed to
forget to apply this here...

+    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 = frame_tree_node_id_to_save_item_[frame_tree_node_id];
+  if (save_item->state() != SaveItem::IN_PROGRESS) {
+    LogWriteToAlreadyCompletedOrFailedSaveItem(save_item);
     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 +1078,122 @@ 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 =
+      static_cast<WebContentsImpl*>(web_contents())
+          ->GetFrameTree()
+          ->FindByRoutingID(sender->GetProcess()->GetID(),
+                            render_frame_or_proxy_routing_id);
+  if (!frame_tree_node)
+    return;
+
+  SaveItem* save_item =
+      frame_tree_node_id_to_save_item_[frame_tree_node->frame_tree_node_id()];
+  SerializeLocalPathForSaveItem(sender, save_item,
+                                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;
+
+  SaveItem* save_item = url_to_save_item_[savable_resource];
+  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 = frame_tree_node_id_to_save_item_[frame_tree_node_id];
+  if (save_item->state() != SaveItem::IN_PROGRESS) {
+    LogWriteToAlreadyCompletedOrFailedSaveItem(save_item);
+    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_) {
       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 =
+        base::FilePath(FILE_PATH_LITERAL("."))
+            .Append(saved_main_directory_path_.BaseName())
+            .Append(save_item->file_name())
+            .NormalizePathSeparatorsTo(FILE_PATH_LITERAL('/'));
+    attr_value = relative_path.AsUTF8Unsafe();
+  } else {
+    attr_value = fallback_url.spec();
+  }
+
+  // TODO(lukasza): Escape attr_value (&quot; + &amp;).
+  OnSerializedHtmlFragment(sender, attr_value);
+}
+
+void SavePackage::LogWriteToAlreadyCompletedOrFailedSaveItem(
+    SaveItem* save_item) {
+  auto already_completed_it =
+      std::find_if(saved_success_items_.begin(), saved_success_items_.end(),
+                   [save_item](const SavedItemMap::value_type& v) {
+                     return v.second->url() == save_item->url();
+                   });
+  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(),
+                   [save_item](const SaveUrlItemMap::value_type& v) {
+                     DCHECK(v.second->has_final_name());
+                     return v.second->url() == save_item->url();
+                   });
+  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 +1230,27 @@ 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];
+    if (save_item == nullptr) {
+      SaveFileCreateInfo::SaveFileSource save_source =
+          u.SchemeIsFile() ? SaveFileCreateInfo::SAVE_FILE_FROM_FILE
+                           : SaveFileCreateInfo::SAVE_FILE_FROM_NET;
+      url_to_save_item_[u] = save_item = new SaveItem(u, this, save_source);
+      waiting_item_queue_.push_back(save_item);
+    }
+    save_item->set_referrer(referrers_list[i]);
   }
 
-  // Store savable frame_url for later processing.
-  if (frame_url.is_valid())

[dcheng, 2015/10/05 07:53:08]

It seems like we don't have this check anymore?

[Łukasz Anforowicz, 2015/10/05 17:57:56]

Thanks for catching this.  I am not sure how this has happened :-(

-    frame_urls_to_save_.push_back(frame_url);
+  // Add the frame to wait list.
+  // TODO(lukasza): Do not dedupe subframes based on url (see crbug.com/538188).
+  SaveItem* save_item = url_to_save_item_[frame_url];
+  if (save_item == nullptr) {
+    url_to_save_item_[frame_url] = save_item =
+        new SaveItem(frame_url, this, SaveFileCreateInfo::SAVE_FILE_FROM_DOM);
+    waiting_item_queue_.push_back(save_item);
+  }
+  frame_tree_node_id_to_save_item_[GetFrameTreeNodeId(sender)] = save_item;
 
   CompleteSavableResourceLinksResponse();
 }
@@ -1234,20 +1265,15 @@ 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);
-    }
-  }
-
+  // Sort |waiting_item_queue_| so that frames go last.
+  std::sort(
+      waiting_item_queue_.begin(), waiting_item_queue_.end(),
+      [](SaveItem* x, SaveItem* y) {
+        DCHECK(x);
+        DCHECK(y);
+        return (x->save_source() != SaveFileCreateInfo::SAVE_FILE_FROM_DOM) &&
+               (y->save_source() == SaveFileCreateInfo::SAVE_FILE_FROM_DOM);
+      });
   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.