diff --git a/atom/browser/api/atom_api_download_item.cc b/atom/browser/api/atom_api_download_item.cc index c3beee5bb094..5a8befc9d1db 100644 --- a/atom/browser/api/atom_api_download_item.cc +++ b/atom/browser/api/atom_api_download_item.cc @@ -48,89 +48,49 @@ namespace atom { namespace api { namespace { + // The wrapDownloadItem funtion which is implemented in JavaScript using WrapDownloadItemCallback = base::Callback)>; WrapDownloadItemCallback g_wrap_download_item; -char kDownloadItemSavePathKey[] = "DownloadItemSavePathKey"; - std::map>> 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) { +DownloadItem::DownloadItem(content::DownloadItem* download_item) + : download_item_(download_item) { download_item_->AddObserver(this); + AttachAsUserData(download_item); } DownloadItem::~DownloadItem() { - Destroy(download_item_); -} - -void DownloadItem::Destroy(content::DownloadItem* item) { - if (item) { - OnDownloadDestroyed(item); - MarkDestroyed(); - - // Destroy the native class in next tick. - base::MessageLoop::current()->PostTask(FROM_HERE, GetDestroyClosure()); + if (download_item_) { + // Destroyed by either garbage collection or destroy(). + download_item_->RemoveObserver(this); + download_item_->Remove(); } + + // Remove from the global map. + auto iter = g_download_item_objects.find(weak_map_id()); + if (iter != g_download_item_objects.end()) + g_download_item_objects.erase(iter); } void DownloadItem::OnDownloadUpdated(content::DownloadItem* item) { - download_item_->IsDone() ? Emit("done", item->GetState()) : Emit("updated"); + if (download_item_->IsDone()) { + Emit("done", item->GetState()); + + // Destroy the item once item is downloaded. + base::MessageLoop::current()->PostTask(FROM_HERE, GetDestroyClosure()); + } else { + Emit("updated"); + } } void DownloadItem::OnDownloadDestroyed(content::DownloadItem* download_item) { - download_item->RemoveObserver(this); - auto iter = g_download_item_objects.find(download_item->GetId()); - if (iter != g_download_item_objects.end()) - g_download_item_objects.erase(iter); download_item_ = nullptr; -} - -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)); + // Destroy the native class immediately when downloadItem is destroyed. + delete this; } void DownloadItem::Pause() { @@ -143,6 +103,48 @@ void DownloadItem::Resume() { void DownloadItem::Cancel() { download_item_->Cancel(true); + 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 @@ -155,29 +157,31 @@ 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 mate::Handle DownloadItem::Create( v8::Isolate* isolate, content::DownloadItem* item) { + auto existing = TrackableObject::FromWrappedClass(isolate, item); + if (existing) + return mate::CreateHandle(isolate, static_cast(existing)); + auto handle = mate::CreateHandle(isolate, new DownloadItem(item)); g_wrap_download_item.Run(handle.ToV8()); - g_download_item_objects[item->GetId()] = make_linked_ptr( + + // Reference this object in case it got garbage collected. + g_download_item_objects[handle->weak_map_id()] = make_linked_ptr( new v8::Global(isolate, handle.ToV8())); return handle; } -// static -void* DownloadItem::UserDataKey() { - return &kDownloadItemSavePathKey; -} - void ClearWrapDownloadItem() { g_wrap_download_item.Reset(); } diff --git a/atom/browser/api/atom_api_download_item.h b/atom/browser/api/atom_api_download_item.h index af388a567513..5806c0181768 100644 --- a/atom/browser/api/atom_api_download_item.h +++ b/atom/browser/api/atom_api_download_item.h @@ -20,23 +20,25 @@ namespace api { class DownloadItem : public mate::TrackableObject, 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 Create(v8::Isolate* isolate, content::DownloadItem* item); - static void* UserDataKey(); // mate::TrackableObject: static void BuildPrototype(v8::Isolate* isolate, v8::Local prototype); - void Destroy(content::DownloadItem* download_item); + 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); @@ -46,19 +48,8 @@ class DownloadItem : public mate::TrackableObject, 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); diff --git a/atom/browser/api/atom_api_session.cc b/atom/browser/api/atom_api_session.cc index 04ab7a9f510e..e5c5198f0341 100644 --- a/atom/browser/api/atom_api_session.cc +++ b/atom/browser/api/atom_api_session.cc @@ -306,14 +306,12 @@ void Session::OnDownloadCreated(content::DownloadManager* manager, auto web_contents = item->GetWebContents(); if (SavePageHandler::IsSavePageTypes(item->GetMimeType())) return; - auto download_item_handle = DownloadItem::Create(isolate(), item); bool prevent_default = Emit( "will-download", - download_item_handle, + DownloadItem::Create(isolate(), item), api::WebContents::CreateFrom(isolate(), web_contents)); if (prevent_default) { item->Cancel(true); - download_item_handle->Destroy(item); item->Remove(); } } diff --git a/atom/browser/atom_download_manager_delegate.cc b/atom/browser/atom_download_manager_delegate.cc index a5f5cc6d8e4d..f5bdbbd8598d 100644 --- a/atom/browser/atom_download_manager_delegate.cc +++ b/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(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( diff --git a/vendor/native_mate b/vendor/native_mate index 26520c5cf4b6..e719eab878c2 160000 --- a/vendor/native_mate +++ b/vendor/native_mate @@ -1 +1 @@ -Subproject commit 26520c5cf4b6a60da2c5cba971393f94b82f5939 +Subproject commit e719eab878c264bb03188d0cd6eb9ad6882bc13a