Get rid of SavePathData

atom/browser/api/atom_api_download_item.cc

@@ -53,20 +53,10 @@ namespace {
 using WrapDownloadItemCallback = base::Callback<void(v8::Local<v8::Value>)>;
 WrapDownloadItemCallback g_wrap_download_item;
 
-char kDownloadItemSavePathKey[] = "DownloadItemSavePathKey";
-
 std::map<uint32, linked_ptr<v8::Global<v8::Value>>> g_download_item_objects;
 
 }  // namespace
 
-DownloadItem::SavePathData::SavePathData(const base::FilePath& path) :
-  path_(path) {
-}
-
-const base::FilePath& DownloadItem::SavePathData::path() {
-  return path_;
-}
-
 DownloadItem::DownloadItem(content::DownloadItem* download_item)
     : download_item_(download_item) {
   download_item_->AddObserver(this);
@@ -74,7 +64,6 @@ DownloadItem::DownloadItem(content::DownloadItem* download_item)
 }
 
 DownloadItem::~DownloadItem() {
-  LOG(ERROR) << "~DownloadItem";
   if (download_item_) {
     // Destroyed by either garbage collection or destroy().
     download_item_->RemoveObserver(this);
@@ -104,43 +93,6 @@ void DownloadItem::OnDownloadDestroyed(content::DownloadItem* download_item) {
   delete this;
 }
 
-int64 DownloadItem::GetReceivedBytes() {
-  return download_item_->GetReceivedBytes();
-}
-
-int64 DownloadItem::GetTotalBytes() {
-  return download_item_->GetTotalBytes();
-}
-
-const GURL& DownloadItem::GetURL() {
-  return download_item_->GetURL();
-}
-
-std::string DownloadItem::GetMimeType() {
-  return download_item_->GetMimeType();
-}
-
-bool DownloadItem::HasUserGesture() {
-  return download_item_->HasUserGesture();
-}
-
-std::string DownloadItem::GetFilename() {
-  return base::UTF16ToUTF8(net::GenerateFileName(GetURL(),
-                           GetContentDisposition(),
-                           std::string(),
-                           download_item_->GetSuggestedFilename(),
-                           GetMimeType(),
-                           std::string()).LossyDisplayName());
-}
-
-std::string DownloadItem::GetContentDisposition() {
-  return download_item_->GetContentDisposition();
-}
-
-void DownloadItem::SetSavePath(const base::FilePath& path) {
-  download_item_->SetUserData(UserDataKey(), new SavePathData(path));
-}
-
 void DownloadItem::Pause() {
   download_item_->Pause();
 }
@@ -154,6 +106,47 @@ void DownloadItem::Cancel() {
   download_item_->Remove();
 }
 
+int64 DownloadItem::GetReceivedBytes() const {
+  return download_item_->GetReceivedBytes();
+}
+
+int64 DownloadItem::GetTotalBytes() const {
+  return download_item_->GetTotalBytes();
+}
+
+std::string DownloadItem::GetMimeType() const {
+  return download_item_->GetMimeType();
+}
+
+bool DownloadItem::HasUserGesture() const {
+  return download_item_->HasUserGesture();
+}
+
+std::string DownloadItem::GetFilename() const {
+  return base::UTF16ToUTF8(net::GenerateFileName(GetURL(),
+                           GetContentDisposition(),
+                           std::string(),
+                           download_item_->GetSuggestedFilename(),
+                           GetMimeType(),
+                           std::string()).LossyDisplayName());
+}
+
+std::string DownloadItem::GetContentDisposition() const {
+  return download_item_->GetContentDisposition();
+}
+
+const GURL& DownloadItem::GetURL() const {
+  return download_item_->GetURL();
+}
+
+void DownloadItem::SetSavePath(const base::FilePath& path) {
+  save_path_ = path;
+}
+
+base::FilePath DownloadItem::GetSavePath() const {
+  return save_path_;
+}
+
 // static
 void DownloadItem::BuildPrototype(v8::Isolate* isolate,
                                   v8::Local<v8::ObjectTemplate> prototype) {
@@ -164,12 +157,13 @@ void DownloadItem::BuildPrototype(v8::Isolate* isolate,
       .SetMethod("cancel", &DownloadItem::Cancel)
       .SetMethod("getReceivedBytes", &DownloadItem::GetReceivedBytes)
       .SetMethod("getTotalBytes", &DownloadItem::GetTotalBytes)
-      .SetMethod("getURL", &DownloadItem::GetURL)
       .SetMethod("getMimeType", &DownloadItem::GetMimeType)
       .SetMethod("hasUserGesture", &DownloadItem::HasUserGesture)
       .SetMethod("getFilename", &DownloadItem::GetFilename)
       .SetMethod("getContentDisposition", &DownloadItem::GetContentDisposition)
-      .SetMethod("setSavePath", &DownloadItem::SetSavePath);
+      .SetMethod("getURL", &DownloadItem::GetURL)
+      .SetMethod("setSavePath", &DownloadItem::SetSavePath)
+      .SetMethod("getSavePath", &DownloadItem::GetSavePath);
 }
 
 // static
@@ -188,11 +182,6 @@ mate::Handle<DownloadItem> DownloadItem::Create(
   return handle;
 }
 
-// static
-void* DownloadItem::UserDataKey() {
-  return &kDownloadItemSavePathKey;
-}
-
 void ClearWrapDownloadItem() {
   g_wrap_download_item.Reset();
 }

atom/browser/api/atom_api_download_item.h

@@ -20,22 +20,26 @@ namespace api {
 class DownloadItem : public mate::TrackableObject<DownloadItem>,
                      public content::DownloadItem::Observer {
  public:
-  class SavePathData : public base::SupportsUserData::Data {
-   public:
-    explicit SavePathData(const base::FilePath& path);
-    const base::FilePath& path();
-   private:
-    base::FilePath path_;
-  };
-
   static mate::Handle<DownloadItem> Create(v8::Isolate* isolate,
                                            content::DownloadItem* item);
-  static void* UserDataKey();
 
   // mate::TrackableObject:
   static void BuildPrototype(v8::Isolate* isolate,
                              v8::Local<v8::ObjectTemplate> prototype);
 
+  void Pause();
+  void Resume();
+  void Cancel();
+  int64 GetReceivedBytes() const;
+  int64 GetTotalBytes() const;
+  std::string GetMimeType() const;
+  bool HasUserGesture() const;
+  std::string GetFilename() const;
+  std::string GetContentDisposition() const;
+  const GURL& GetURL() const;
+  void SetSavePath(const base::FilePath& path);
+  base::FilePath GetSavePath() const;
+
  protected:
   explicit DownloadItem(content::DownloadItem* download_item);
   ~DownloadItem();
@@ -44,19 +48,8 @@ class DownloadItem : public mate::TrackableObject<DownloadItem>,
   void OnDownloadUpdated(content::DownloadItem* download) override;
   void OnDownloadDestroyed(content::DownloadItem* download) override;
 
-  void Pause();
-  void Resume();
-  void Cancel();
-  int64 GetReceivedBytes();
-  int64 GetTotalBytes();
-  std::string GetMimeType();
-  bool HasUserGesture();
-  std::string GetFilename();
-  std::string GetContentDisposition();
-  const GURL& GetURL();
-  void SetSavePath(const base::FilePath& path);
-
  private:
+  base::FilePath save_path_;
   content::DownloadItem* download_item_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadItem);

atom/browser/atom_download_manager_delegate.cc

@@ -109,16 +109,24 @@ bool AtomDownloadManagerDelegate::DetermineDownloadTarget(
                  download->GetForcedFilePath());
     return true;
   }
-  base::SupportsUserData::Data* save_path = download->GetUserData(
-      atom::api::DownloadItem::UserDataKey());
-  if (save_path) {
-    const base::FilePath& default_download_path =
-        static_cast<api::DownloadItem::SavePathData*>(save_path)->path();
-    callback.Run(default_download_path,
-                 content::DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-                 content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
-                 default_download_path);
-    return true;
+
+  // Try to get the save path from JS wrapper.
+  {
+    v8::Isolate* isolate = v8::Isolate::GetCurrent();
+    v8::Locker locker(isolate);
+    v8::HandleScope handle_scope(isolate);
+    api::DownloadItem* download_item = api::DownloadItem::FromWrappedClass(
+        isolate, download);
+    if (download_item) {
+      base::FilePath save_path = download_item->GetSavePath();
+      if (!save_path.empty()) {
+        callback.Run(save_path,
+                     content::DownloadItem::TARGET_DISPOSITION_OVERWRITE,
+                     content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+                     save_path);
+        return true;
+      }
+    }
   }
 
   AtomBrowserContext* browser_context = static_cast<AtomBrowserContext*>(