Save-Page-As-Complete-Html: Each frame links to a distinct local file.

content/browser/download/save_package.cc

 // Copyright (c) 2012 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/browser/download/save_package.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 336 matching lines @@
     GetSavableResourceLinks();
   } else if (save_type_ == SAVE_PAGE_TYPE_AS_MHTML) {
     web_contents()->GenerateMHTML(saved_main_file_path_, base::Bind(
         &SavePackage::OnMHTMLGenerated, this));
   } else {
     DCHECK_EQ(SAVE_PAGE_TYPE_AS_ONLY_HTML, save_type_);
     wait_state_ = NET_FILES;
     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_, Referrer(), this, save_source,
+                                       FrameTreeNode::kFrameTreeNodeInvalidId);
     // Add this item to waiting list.
     waiting_item_queue_.push_back(save_item);
     all_save_items_count_ = 1;
     download_->SetTotalBytes(1);
 
     DoSavingProcess();
   }
 }
 
 void SavePackage::OnMHTMLGenerated(int64_t size) {
@@ skipping 621 matching lines @@
     // All frames disappeared since gathering of savable resources?
     // Treat this as cancellation.
     Cancel(false);
   }
 }
 
 void SavePackage::GetSerializedHtmlWithLocalLinksForFrame(
     FrameTreeNode* target_tree_node) {
   DCHECK(target_tree_node);
   int target_frame_tree_node_id = target_tree_node->frame_tree_node_id();
+  RenderFrameHostImpl* target = target_tree_node->current_frame_host();
 
   // Collect all saved success items.
   // SECURITY NOTE: We don't send *all* urls / local paths, but only
   // those that the given frame had access to already (because it contained
   // the savable resources / subframes associated with save items).
   std::map<GURL, base::FilePath> url_to_local_path;
+  std::map<int, base::FilePath> routing_id_to_local_path;
   auto it = frame_tree_node_id_to_contained_save_items_.find(
       target_frame_tree_node_id);
   if (it != frame_tree_node_id_to_contained_save_items_.end()) {
     for (SaveItem* save_item : it->second) {
+      // Calculate the local link to use for this |save_item|.
       DCHECK(save_item->has_final_name());
       base::FilePath local_path(base::FilePath::kCurrentDirectory);
       if (target_tree_node->IsMainFrame()) {
         local_path = local_path.Append(saved_main_directory_path_.BaseName());
       }
       local_path = local_path.Append(save_item->file_name());
-      url_to_local_path[save_item->url()] = local_path;
+
+      // Insert the link into |url_to_local_path| or |routing_id_to_local_path|.
+      if (save_item->save_source() != SaveFileCreateInfo::SAVE_FILE_FROM_DOM) {
+        DCHECK_EQ(FrameTreeNode::kFrameTreeNodeInvalidId,
+                  save_item->frame_tree_node_id());
+        url_to_local_path[save_item->url()] = local_path;
+      } else {
+        FrameTreeNode* save_item_frame_tree_node =
+            target_tree_node->frame_tree()->FindByID(
+                save_item->frame_tree_node_id());
+        if (!save_item_frame_tree_node) {
+          // crbug.com/541354: Raciness when saving a dynamically changing page.
+          continue;
+        }
+
+        int routing_id =
+            save_item_frame_tree_node->render_manager()
+                ->GetRoutingIdForSiteInstance(target->GetSiteInstance());
+        if (routing_id == MSG_ROUTING_NONE)

[Randy Smith (Not in Mondays), 2016/01/22 15:28:39]

Is this also due to 541354?  If so, could you duplicate the comment or combine
the if ... continue tests in one place?

[Łukasz Anforowicz, 2016/01/22 17:39:40]

Good point.  Right now I think that if |save_item_frame_tree_node| exists and
|target| exists, then we should never get a MSG_ROUTING_NONE here.  So I think
I'll change this if statement into a DCHECK.  And even if I am wrong, then
having MSG_ROUTING_NONE as a key in the map shouldn't hurt - there should be no
lookups using this key.

[Łukasz Anforowicz, 2016/01/25 19:04:05]

Done (talked with Nasko and used a DCHECK [instead of an if statement +
continue] to verify that we are not getting MSG_ROUTING_NONE).

+          continue;
+
+        routing_id_to_local_path[routing_id] = local_path;
+      }
     }
   }
 
   // Ask target frame to serialize itself.
-  RenderFrameHostImpl* target = target_tree_node->current_frame_host();
   target->Send(new FrameMsg_GetSerializedHtmlWithLocalLinks(
-      target->GetRoutingID(), url_to_local_path));
+      target->GetRoutingID(), url_to_local_path, routing_id_to_local_path));
 }
 
 // Process the serialized HTML content data of a specified frame
 // retrieved from the renderer process.
 void SavePackage::OnSerializedHtmlWithLocalLinksResponse(
     RenderFrameHostImpl* sender,
     const std::string& data,
     bool end_of_data) {
   // Check current state.
   if (wait_state_ != HTML_DATA)
@@ skipping 113 matching lines @@
     EnqueueFrame(container_frame_tree_node_id,
                  subframe_tree_node->frame_tree_node_id(),
                  subframe.original_url);
   }
 
   CompleteSavableResourceLinksResponse();
 }
 
 SaveItem* SavePackage::CreatePendingSaveItem(
     int container_frame_tree_node_id,
+    int save_item_frame_tree_node_id,
     const GURL& url,
     const Referrer& referrer,
     SaveFileCreateInfo::SaveFileSource save_source) {
   DCHECK(url.is_valid());  // |url| should be validated by the callers.
 
   SaveItem* save_item;
   Referrer sanitized_referrer = Referrer::SanitizeForRequest(url, referrer);
-  save_item = new SaveItem(url, sanitized_referrer, this, save_source);
+  save_item = new SaveItem(url, sanitized_referrer, this, save_source,
+                           save_item_frame_tree_node_id);
   waiting_item_queue_.push_back(save_item);
 
   frame_tree_node_id_to_contained_save_items_[container_frame_tree_node_id]
       .push_back(save_item);
   return save_item;
 }
 
 SaveItem* SavePackage::CreatePendingSaveItemDeduplicatingByUrl(
     int container_frame_tree_node_id,
+    int save_item_frame_tree_node_id,
     const GURL& url,
     const Referrer& referrer,
     SaveFileCreateInfo::SaveFileSource save_source) {
   DCHECK(url.is_valid());  // |url| should be validated by the callers.
 
   // Frames should not be deduplicated by URL.
   DCHECK_NE(SaveFileCreateInfo::SAVE_FILE_FROM_DOM, save_source);
 
   SaveItem* save_item;
   auto it = url_to_save_item_.find(url);
   if (it != url_to_save_item_.end()) {
     save_item = it->second;
     frame_tree_node_id_to_contained_save_items_[container_frame_tree_node_id]
         .push_back(save_item);
   } else {
-    save_item = CreatePendingSaveItem(container_frame_tree_node_id, url,
+    save_item = CreatePendingSaveItem(container_frame_tree_node_id,
+                                      save_item_frame_tree_node_id, url,
                                       referrer, save_source);
     url_to_save_item_[url] = save_item;
   }
 
   return save_item;
 }
 
 void SavePackage::EnqueueSavableResource(int container_frame_tree_node_id,
                                          const GURL& url,
                                          const Referrer& referrer) {
   if (!url.is_valid())
     return;
 
   SaveFileCreateInfo::SaveFileSource save_source =
       url.SchemeIsFile() ? SaveFileCreateInfo::SAVE_FILE_FROM_FILE
                          : SaveFileCreateInfo::SAVE_FILE_FROM_NET;
-  CreatePendingSaveItemDeduplicatingByUrl(container_frame_tree_node_id, url,
-                                          referrer, save_source);
+  CreatePendingSaveItemDeduplicatingByUrl(
+      container_frame_tree_node_id, FrameTreeNode::kFrameTreeNodeInvalidId, url,
+      referrer, save_source);
 }
 
 void SavePackage::EnqueueFrame(int container_frame_tree_node_id,
                                int frame_tree_node_id,
                                const GURL& frame_original_url) {
   if (!frame_original_url.is_valid())
     return;
 
-  SaveItem* save_item =
-      CreatePendingSaveItem(container_frame_tree_node_id, frame_original_url,
-                            Referrer(), SaveFileCreateInfo::SAVE_FILE_FROM_DOM);
+  SaveItem* save_item = CreatePendingSaveItem(
+      container_frame_tree_node_id, frame_tree_node_id, frame_original_url,
+      Referrer(), SaveFileCreateInfo::SAVE_FILE_FROM_DOM);
   DCHECK(save_item);
   frame_tree_node_id_to_save_item_[frame_tree_node_id] = save_item;
 }
 
 void SavePackage::OnSavableResourceLinksError(RenderFrameHostImpl* sender) {
   CompleteSavableResourceLinksResponse();
 }
 
 void SavePackage::CompleteSavableResourceLinksResponse() {
   --number_of_frames_pending_response_;
@@ skipping 269 matching lines @@
 
 void SavePackage::FinalizeDownloadEntry() {
   DCHECK(download_);
   DCHECK(download_manager_);
 
   download_manager_->OnSavePackageSuccessfullyFinished(download_);
   StopObservation();
 }
 
 }  // namespace content