Remove the content-length member variable from SaveFileResourceHandler

The content-length related member variables occurring in a few classes
(SaveFileResourceHandler and SaveItem) is not used. Removing it. Also
now passing the correct content-description (it was always empty).
A few minor clean-ups as well.
This work is in preparation of changing that code to use the network
service.

Bug: 816644
Change-Id: I14f999fc3ce5b0bb77ecf9e1d35311ef7704db8a
Reviewed-on: https://chromium-review.googlesource.com/938386
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539919}

content/browser/download/save_file.cc

@@ -14,12 +14,13 @@ namespace content {
 //               the default download directory when initializing |file_|.
 //               Unfortunately, as it is, constructors of SaveFile don't always
 //               have access to the SavePackage at this point.
-SaveFile::SaveFile(const SaveFileCreateInfo* info, bool calculate_hash)
-    : file_(download::DownloadItem::kInvalidId), info_(info) {
+SaveFile::SaveFile(std::unique_ptr<SaveFileCreateInfo> info,
+                   bool calculate_hash)
+    : file_(download::DownloadItem::kInvalidId), info_(std::move(info)) {
   DCHECK(download::GetDownloadTaskRunner()->RunsTasksInCurrentSequence());
 
-  DCHECK(info);
-  DCHECK(info->path.empty());
+  DCHECK(info_);
+  DCHECK(info_->path.empty());
 }
 
 SaveFile::~SaveFile() {
@@ -27,9 +28,12 @@ SaveFile::~SaveFile() {
 }
 
 download::DownloadInterruptReason SaveFile::Initialize() {
-  return file_.Initialize(base::FilePath(), base::FilePath(), base::File(), 0,
-                          std::string(), std::unique_ptr<crypto::SecureHash>(),
-                          false);
+  download::DownloadInterruptReason reason = file_.Initialize(
+      /*full_path=*/base::FilePath(), /*default_directory=*/base::FilePath(),
+      /*file=*/base::File(), /*bytes_so_far=*/0, /*hash_so_far=*/std::string(),
+      /*hash_state=*/nullptr, /*is_sparse_file=*/false);
+  info_->path = FullPath();
+  return reason;
 }
 
 download::DownloadInterruptReason SaveFile::AppendDataToFile(const char* data,

content/browser/download/save_file.h

@@ -5,9 +5,6 @@
 #ifndef CONTENT_BROWSER_DOWNLOAD_SAVE_FILE_H_
 #define CONTENT_BROWSER_DOWNLOAD_SAVE_FILE_H_
 
-#include <stddef.h>
-#include <stdint.h>
-
 #include <memory>
 #include <string>
 
@@ -26,7 +23,7 @@ namespace content {
 // represents one item in a save session.
 class SaveFile {
  public:
-  explicit SaveFile(const SaveFileCreateInfo* info, bool calculate_hash);
+  SaveFile(std::unique_ptr<SaveFileCreateInfo> info, bool calculate_hash);
   virtual ~SaveFile();
 
   // BaseFile delegated functions.
@@ -50,10 +47,11 @@ class SaveFile {
   SaveFileCreateInfo::SaveFileSource save_source() const {
     return info_->save_source;
   }
+  const SaveFileCreateInfo& create_info() const { return *info_; }
 
  private:
   BaseFile file_;
-  std::unique_ptr<const SaveFileCreateInfo> info_;
+  std::unique_ptr<SaveFileCreateInfo> info_;
 
   DISALLOW_COPY_AND_ASSIGN(SaveFile);
 };

content/browser/download/save_file_manager.cc

@@ -115,14 +115,15 @@ void SaveFileManager::SaveURL(SaveItemId save_item_id,
                        context));
   } else {
     // We manually start the save job.
-    SaveFileCreateInfo* info = new SaveFileCreateInfo(
+    auto info = std::make_unique<SaveFileCreateInfo>(
         file_full_path, url, save_item_id, save_package->id(),
         render_process_host_id, render_frame_routing_id, save_source);
 
     // Since the data will come from render process, so we need to start
     // this kind of save job by ourself.
     download::GetDownloadTaskRunner()->PostTask(
-        FROM_HERE, base::BindOnce(&SaveFileManager::StartSave, this, info));
+        FROM_HERE,
+        base::BindOnce(&SaveFileManager::StartSave, this, std::move(info)));
   }
 }
 
@@ -179,22 +180,23 @@ void SaveFileManager::SendCancelRequest(SaveItemId save_item_id) {
 // The IO thread created |info|, but the file thread (this method) uses it
 // to create a SaveFile which will hold and finally destroy |info|. It will
 // then passes |info| to the UI thread for reporting saving status.
-void SaveFileManager::StartSave(SaveFileCreateInfo* info) {
+void SaveFileManager::StartSave(std::unique_ptr<SaveFileCreateInfo> info) {
   DCHECK(download::GetDownloadTaskRunner()->RunsTasksInCurrentSequence());
   DCHECK(info);
   // No need to calculate hash.
-  std::unique_ptr<SaveFile> save_file = std::make_unique<SaveFile>(info, false);
+  auto save_file =
+      std::make_unique<SaveFile>(std::move(info), /*calculate_hash=*/false);
 
   // TODO(phajdan.jr): We should check the return value and handle errors here.
   save_file->Initialize();
-  info->path = save_file->FullPath();
 
-  DCHECK(!LookupSaveFile(info->save_item_id));
-  save_file_map_[info->save_item_id] = std::move(save_file);
+  const SaveFileCreateInfo& save_file_create_info = save_file->create_info();
+  DCHECK(!LookupSaveFile(save_file->save_item_id()));
+  save_file_map_[save_file->save_item_id()] = std::move(save_file);
 
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::BindOnce(&SaveFileManager::OnStartSave, this, *info));
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                          base::BindOnce(&SaveFileManager::OnStartSave, this,
+                                         save_file_create_info));
 }
 
 // We do forward an update to the UI thread here, since we do not use timer to

content/browser/download/save_file_manager.h

@@ -59,6 +59,7 @@
 
 #include <stdint.h>
 
+#include <memory>
 #include <string>
 #include <unordered_map>
 
@@ -110,7 +111,7 @@ class CONTENT_EXPORT SaveFileManager
                SavePackage* save_package);
 
   // Notifications sent from the IO thread and run on the file thread:
-  void StartSave(SaveFileCreateInfo* info);
+  void StartSave(std::unique_ptr<SaveFileCreateInfo> info);
   void UpdateSaveProgress(SaveItemId save_item_id,
                           net::IOBuffer* data,
                           int size);

content/browser/download/save_file_resource_handler.cc

@@ -4,6 +4,8 @@
 
 #include "content/browser/download/save_file_resource_handler.h"
 
+#include <string>
+
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
@@ -12,8 +14,10 @@
 #include "content/browser/download/save_file_manager.h"
 #include "content/browser/loader/resource_controller.h"
 #include "net/base/io_buffer.h"
+#include "net/http/http_response_headers.h"
 #include "net/url_request/redirect_info.h"
 #include "net/url_request/url_request_status.h"
+#include "services/network/public/cpp/resource_response.h"
 
 namespace content {
 
@@ -31,7 +35,6 @@ SaveFileResourceHandler::SaveFileResourceHandler(
       render_process_id_(render_process_host_id),
       render_frame_routing_id_(render_frame_routing_id),
       url_(url),
-      content_length_(0),
       save_manager_(SaveFileManager::Get()),
       authorization_state_(authorization_state) {}
 
@@ -49,14 +52,18 @@ void SaveFileResourceHandler::OnRequestRedirected(
 void SaveFileResourceHandler::OnResponseStarted(
     network::ResourceResponse* response,
     std::unique_ptr<ResourceController> controller) {
-  // |save_manager_| consumes (deletes):
-  SaveFileCreateInfo* info = new SaveFileCreateInfo(
+  std::string content_disposition;
+  if (response->head.headers) {
+    response->head.headers->GetNormalizedHeader("Content-Disposition",
+                                                &content_disposition);
+  }
+
+  auto info = std::make_unique<SaveFileCreateInfo>(
       url_, final_url_, save_item_id_, save_package_id_, render_process_id_,
-      render_frame_routing_id_, GetRequestID(), content_disposition_,
-      content_length_);
+      render_frame_routing_id_, GetRequestID(), content_disposition);
   download::GetDownloadTaskRunner()->PostTask(
-      FROM_HERE,
-      base::BindOnce(&SaveFileManager::StartSave, save_manager_, info));
+      FROM_HERE, base::BindOnce(&SaveFileManager::StartSave, save_manager_,
+                                std::move(info)));
   controller->Resume();
 }
 
@@ -118,9 +125,4 @@ void SaveFileResourceHandler::OnDataDownloaded(int bytes_downloaded) {
   NOTREACHED();
 }
 
-void SaveFileResourceHandler::set_content_length(
-    const std::string& content_length) {
-  base::StringToInt64(content_length, &content_length_);
-}
-
 }  // namespace content

content/browser/download/save_file_resource_handler.h

@@ -5,10 +5,6 @@
 #ifndef CONTENT_BROWSER_DOWNLOAD_SAVE_FILE_RESOURCE_HANDLER_H_
 #define CONTENT_BROWSER_DOWNLOAD_SAVE_FILE_RESOURCE_HANDLER_H_
 
-#include <stdint.h>
-
-#include <string>
-
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "content/browser/download/save_types.h"
@@ -82,25 +78,14 @@ class SaveFileResourceHandler : public ResourceHandler {
   // N/A to this flavor of SaveFileResourceHandler.
   void OnDataDownloaded(int bytes_downloaded) override;
 
-  // If the content-length header is not present (or contains something other
-  // than numbers), StringToInt64 returns 0, which indicates 'unknown size' and
-  // is handled correctly by the SaveManager.
-  void set_content_length(const std::string& content_length);
-
-  void set_content_disposition(const std::string& content_disposition) {
-    content_disposition_ = content_disposition;
-  }
-
  private:
   SaveItemId save_item_id_;
   SavePackageId save_package_id_;
   int render_process_id_;
   int render_frame_routing_id_;
   scoped_refptr<net::IOBuffer> read_buffer_;
-  std::string content_disposition_;
   GURL url_;
   GURL final_url_;
-  int64_t content_length_;
   SaveFileManager* save_manager_;
 
   AuthorizationState authorization_state_;

content/browser/download/save_item.cc

@@ -35,7 +35,6 @@ SaveItem::SaveItem(const GURL& url,
       referrer_(referrer),
       frame_tree_node_id_(frame_tree_node_id),
       container_frame_tree_node_id_(container_frame_tree_node_id),
-      total_bytes_(0),
       received_bytes_(0),
       state_(WAIT_START),
       is_success_(false),
@@ -52,12 +51,8 @@ void SaveItem::Start() {
   state_ = IN_PROGRESS;
 }
 
-// If we've received more data than we were expecting (bad server info?),
-// revert to 'unknown size mode'.
 void SaveItem::UpdateSize(int64_t bytes_so_far) {
   received_bytes_ = bytes_so_far;
-  if (received_bytes_ >= total_bytes_)
-    total_bytes_ = 0;
 }
 
 // Updates from the file thread may have been posted while this saving job
@@ -100,9 +95,4 @@ void SaveItem::SetTargetPath(const base::FilePath& full_path) {
   full_path_ = full_path;
 }
 
-void SaveItem::SetTotalBytes(int64_t total_bytes) {
-  DCHECK_EQ(0, total_bytes_);
-  total_bytes_ = total_bytes;
-}
-
 }  // namespace content

content/browser/download/save_item.h

@@ -50,8 +50,6 @@ class SaveItem {
   // Update path for SaveItem, the actual file is renamed on the file thread.
   void SetTargetPath(const base::FilePath& full_path);
 
-  void SetTotalBytes(int64_t total_bytes);
-
   // Accessors.
   SaveItemId id() const { return save_item_id_; }
   SaveState state() const { return state_; }
@@ -92,9 +90,6 @@ class SaveItem {
   // main frame, which obviously doesn't have a containing/parent frame).
   int container_frame_tree_node_id_;
 
-  // Total bytes expected.
-  int64_t total_bytes_;
-
   // Current received bytes.
   int64_t received_bytes_;
 

content/browser/download/save_package.cc

@@ -514,8 +514,6 @@ void SavePackage::StartSave(const SaveFileCreateInfo* info) {
 
   DCHECK(!saved_main_file_path_.empty());
 
-  save_item->SetTotalBytes(info->total_bytes);
-
   // Determine the proper path for a saving job, by choosing either the default
   // save directory, or prompting the user.
   DCHECK(!save_item->has_final_name());

content/browser/download/save_types.cc

@@ -22,7 +22,6 @@ SaveFileCreateInfo::SaveFileCreateInfo(const base::FilePath& path,
       render_process_id(render_process_id),
       render_frame_routing_id(render_frame_routing_id),
       request_id(-1),
-      total_bytes(0),
       save_source(save_source) {}
 
 SaveFileCreateInfo::SaveFileCreateInfo(const GURL& url,
@@ -32,8 +31,7 @@ SaveFileCreateInfo::SaveFileCreateInfo(const GURL& url,
                                        int render_process_id,
                                        int render_frame_routing_id,
                                        int request_id,
-                                       const std::string& content_disposition,
-                                       int64_t total_bytes)
+                                       const std::string& content_disposition)
     : url(url),
       final_url(final_url),
       save_item_id(save_item_id),
@@ -42,7 +40,6 @@ SaveFileCreateInfo::SaveFileCreateInfo(const GURL& url,
       render_frame_routing_id(render_frame_routing_id),
       request_id(request_id),
       content_disposition(content_disposition),
-      total_bytes(total_bytes),
       save_source(SaveFileCreateInfo::SAVE_FILE_FROM_NET) {}
 
 SaveFileCreateInfo::SaveFileCreateInfo(const SaveFileCreateInfo& other) =

content/browser/download/save_types.h

@@ -58,8 +58,7 @@ struct SaveFileCreateInfo {
                      int render_process_id,
                      int render_frame_routing_id,
                      int request_id,
-                     const std::string& content_disposition,
-                     int64_t total_bytes);
+                     const std::string& content_disposition);
 
   SaveFileCreateInfo(const SaveFileCreateInfo& other);
 
@@ -83,8 +82,6 @@ struct SaveFileCreateInfo {
   int request_id;
   // Disposition info from HTTP response.
   std::string content_disposition;
-  // Total bytes of saved file.
-  int64_t total_bytes;
   // Source type of saved file.
   SaveFileSource save_source;
 };