Commit 84d5741f authored by achuith@chromium.org's avatar achuith@chromium.org

Replace void* with int32 for passing download_id to SelectFileDialog::Listener.

BUG=NONE
TEST=trybot runs.

Review URL: http://codereview.chromium.org/9589003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@124851 0039d316-1c4b-4281-b951-d872f2087c98
parent 7305c205
...@@ -136,10 +136,10 @@ bool ChromeDownloadManagerDelegate::ShouldStartDownload(int32 download_id) { ...@@ -136,10 +136,10 @@ bool ChromeDownloadManagerDelegate::ShouldStartDownload(int32 download_id) {
void ChromeDownloadManagerDelegate::ChooseDownloadPath( void ChromeDownloadManagerDelegate::ChooseDownloadPath(
WebContents* web_contents, WebContents* web_contents,
const FilePath& suggested_path, const FilePath& suggested_path,
void* data) { int32 download_id) {
// Deletes itself. // Deletes itself.
new DownloadFilePicker( new DownloadFilePicker(
download_manager_, web_contents, suggested_path, data); download_manager_, web_contents, suggested_path, download_id);
} }
FilePath ChromeDownloadManagerDelegate::GetIntermediatePath( FilePath ChromeDownloadManagerDelegate::GetIntermediatePath(
......
...@@ -56,7 +56,7 @@ class ChromeDownloadManagerDelegate ...@@ -56,7 +56,7 @@ class ChromeDownloadManagerDelegate
virtual bool ShouldStartDownload(int32 download_id) OVERRIDE; virtual bool ShouldStartDownload(int32 download_id) OVERRIDE;
virtual void ChooseDownloadPath(content::WebContents* web_contents, virtual void ChooseDownloadPath(content::WebContents* web_contents,
const FilePath& suggested_path, const FilePath& suggested_path,
void* data) OVERRIDE; int32 download_id) OVERRIDE;
virtual FilePath GetIntermediatePath(const FilePath& suggested_path) OVERRIDE; virtual FilePath GetIntermediatePath(const FilePath& suggested_path) OVERRIDE;
virtual content::WebContents* virtual content::WebContents*
GetAlternativeWebContentsToNotifyForDownload() OVERRIDE; GetAlternativeWebContentsToNotifyForDownload() OVERRIDE;
......
...@@ -138,9 +138,9 @@ class PickSuggestedFileDelegate : public ChromeDownloadManagerDelegate { ...@@ -138,9 +138,9 @@ class PickSuggestedFileDelegate : public ChromeDownloadManagerDelegate {
virtual void ChooseDownloadPath(WebContents* web_contents, virtual void ChooseDownloadPath(WebContents* web_contents,
const FilePath& suggested_path, const FilePath& suggested_path,
void* data) OVERRIDE { int32 download_id) OVERRIDE {
if (download_manager_) if (download_manager_)
download_manager_->FileSelected(suggested_path, data); download_manager_->FileSelected(suggested_path, download_id);
} }
}; };
......
...@@ -56,8 +56,9 @@ DownloadFilePicker::DownloadFilePicker( ...@@ -56,8 +56,9 @@ DownloadFilePicker::DownloadFilePicker(
DownloadManager* download_manager, DownloadManager* download_manager,
WebContents* web_contents, WebContents* web_contents,
const FilePath& suggested_path, const FilePath& suggested_path,
void* params) int32 download_id)
: download_manager_(download_manager), : download_manager_(download_manager),
download_id_(download_id),
suggested_path_(suggested_path) { suggested_path_(suggested_path) {
DCHECK(download_manager_); DCHECK(download_manager_);
select_file_dialog_ = SelectFileDialog::Create(this); select_file_dialog_ = SelectFileDialog::Create(this);
...@@ -76,7 +77,7 @@ DownloadFilePicker::DownloadFilePicker( ...@@ -76,7 +77,7 @@ DownloadFilePicker::DownloadFilePicker(
string16(), string16(),
suggested_path, suggested_path,
&file_type_info, 0, FILE_PATH_LITERAL(""), &file_type_info, 0, FILE_PATH_LITERAL(""),
web_contents, owning_window, params); web_contents, owning_window, NULL);
} }
DownloadFilePicker::~DownloadFilePicker() { DownloadFilePicker::~DownloadFilePicker() {
...@@ -96,13 +97,13 @@ void DownloadFilePicker::FileSelected(const FilePath& path, ...@@ -96,13 +97,13 @@ void DownloadFilePicker::FileSelected(const FilePath& path,
FilePickerResult result = ComparePaths(suggested_path_, path); FilePickerResult result = ComparePaths(suggested_path_, path);
RecordFilePickerResult(download_manager_, result); RecordFilePickerResult(download_manager_, result);
if (download_manager_) if (download_manager_)
download_manager_->FileSelected(path, params); download_manager_->FileSelected(path, download_id_);
delete this; delete this;
} }
void DownloadFilePicker::FileSelectionCanceled(void* params) { void DownloadFilePicker::FileSelectionCanceled(void* params) {
RecordFilePickerResult(download_manager_, FILE_PICKER_CANCEL); RecordFilePickerResult(download_manager_, FILE_PICKER_CANCEL);
if (download_manager_) if (download_manager_)
download_manager_->FileSelectionCanceled(params); download_manager_->FileSelectionCanceled(download_id_);
delete this; delete this;
} }
...@@ -22,7 +22,7 @@ class DownloadFilePicker : public content::DownloadManager::Observer, ...@@ -22,7 +22,7 @@ class DownloadFilePicker : public content::DownloadManager::Observer,
DownloadFilePicker(content::DownloadManager* download_manager, DownloadFilePicker(content::DownloadManager* download_manager,
content::WebContents* web_contents, content::WebContents* web_contents,
const FilePath& suggested_path, const FilePath& suggested_path,
void* params); int32 download_id);
virtual ~DownloadFilePicker(); virtual ~DownloadFilePicker();
private: private:
...@@ -37,6 +37,7 @@ class DownloadFilePicker : public content::DownloadManager::Observer, ...@@ -37,6 +37,7 @@ class DownloadFilePicker : public content::DownloadManager::Observer,
virtual void FileSelectionCanceled(void* params) OVERRIDE; virtual void FileSelectionCanceled(void* params) OVERRIDE;
content::DownloadManager* download_manager_; content::DownloadManager* download_manager_;
int32 download_id_;
FilePath suggested_path_; FilePath suggested_path_;
......
...@@ -360,10 +360,6 @@ void DownloadManagerImpl::RestartDownload(int32 download_id) { ...@@ -360,10 +360,6 @@ void DownloadManagerImpl::RestartDownload(int32 download_id) {
// We must ask the user for the place to put the download. // We must ask the user for the place to put the download.
WebContents* contents = download->GetWebContents(); WebContents* contents = download->GetWebContents();
// |id_ptr| will be deleted in either FileSelected() or
// FileSelectionCancelled().
int32* id_ptr = new int32;
*id_ptr = download_id;
FilePath target_path; FilePath target_path;
// If |download| is a potentially dangerous file, then |suggested_path| // If |download| is a potentially dangerous file, then |suggested_path|
// contains the intermediate name instead of the final download // contains the intermediate name instead of the final download
...@@ -375,7 +371,7 @@ void DownloadManagerImpl::RestartDownload(int32 download_id) { ...@@ -375,7 +371,7 @@ void DownloadManagerImpl::RestartDownload(int32 download_id) {
target_path = suggested_path; target_path = suggested_path;
delegate_->ChooseDownloadPath(contents, target_path, delegate_->ChooseDownloadPath(contents, target_path,
reinterpret_cast<void*>(id_ptr)); download_id);
FOR_EACH_OBSERVER(Observer, observers_, FOR_EACH_OBSERVER(Observer, observers_,
SelectFileDialogDisplayed(this, download_id)); SelectFileDialogDisplayed(this, download_id));
} else { } else {
...@@ -862,14 +858,10 @@ void DownloadManagerImpl::RemoveObserver(Observer* observer) { ...@@ -862,14 +858,10 @@ void DownloadManagerImpl::RemoveObserver(Observer* observer) {
observers_.RemoveObserver(observer); observers_.RemoveObserver(observer);
} }
void DownloadManagerImpl::FileSelected(const FilePath& path, void* params) { void DownloadManagerImpl::FileSelected(const FilePath& path,
int32 download_id) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
int32* id_ptr = reinterpret_cast<int32*>(params);
DCHECK(id_ptr != NULL);
int32 download_id = *id_ptr;
delete id_ptr;
DownloadItem* download = GetActiveDownloadItem(download_id); DownloadItem* download = GetActiveDownloadItem(download_id);
if (!download) if (!download)
return; return;
...@@ -883,14 +875,10 @@ void DownloadManagerImpl::FileSelected(const FilePath& path, void* params) { ...@@ -883,14 +875,10 @@ void DownloadManagerImpl::FileSelected(const FilePath& path, void* params) {
ContinueDownloadWithPath(download, path); ContinueDownloadWithPath(download, path);
} }
void DownloadManagerImpl::FileSelectionCanceled(void* params) { void DownloadManagerImpl::FileSelectionCanceled(int32 download_id) {
// The user didn't pick a place to save the file, so need to cancel the // The user didn't pick a place to save the file, so need to cancel the
// download that's already in progress to the temporary location. // download that's already in progress to the temporary location.
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
int32* id_ptr = reinterpret_cast<int32*>(params);
DCHECK(id_ptr != NULL);
int32 download_id = *id_ptr;
delete id_ptr;
DownloadItem* download = GetActiveDownloadItem(download_id); DownloadItem* download = GetActiveDownloadItem(download_id);
if (!download) if (!download)
......
...@@ -81,8 +81,8 @@ class CONTENT_EXPORT DownloadManagerImpl ...@@ -81,8 +81,8 @@ class CONTENT_EXPORT DownloadManagerImpl
bool is_otr, bool is_otr,
content::DownloadItem::Observer* observer) OVERRIDE; content::DownloadItem::Observer* observer) OVERRIDE;
virtual void ClearLastDownloadPath() OVERRIDE; virtual void ClearLastDownloadPath() OVERRIDE;
virtual void FileSelected(const FilePath& path, void* params) OVERRIDE; virtual void FileSelected(const FilePath& path, int32 download_id) OVERRIDE;
virtual void FileSelectionCanceled(void* params) OVERRIDE; virtual void FileSelectionCanceled(int32 download_id) OVERRIDE;
virtual void RestartDownload(int32 download_id) OVERRIDE; virtual void RestartDownload(int32 download_id) OVERRIDE;
virtual void CheckForHistoryFilesRemoval() OVERRIDE; virtual void CheckForHistoryFilesRemoval() OVERRIDE;
virtual content::DownloadItem* GetDownloadItem(int id) OVERRIDE; virtual content::DownloadItem* GetDownloadItem(int id) OVERRIDE;
......
...@@ -132,7 +132,7 @@ class TestDownloadManagerDelegate : public content::DownloadManagerDelegate { ...@@ -132,7 +132,7 @@ class TestDownloadManagerDelegate : public content::DownloadManagerDelegate {
virtual void ChooseDownloadPath(WebContents* web_contents, virtual void ChooseDownloadPath(WebContents* web_contents,
const FilePath& suggested_path, const FilePath& suggested_path,
void* data) OVERRIDE { int32 download_id) OVERRIDE {
if (!expected_suggested_path_.empty()) { if (!expected_suggested_path_.empty()) {
EXPECT_STREQ(expected_suggested_path_.value().c_str(), EXPECT_STREQ(expected_suggested_path_.value().c_str(),
suggested_path.value().c_str()); suggested_path.value().c_str());
...@@ -142,14 +142,14 @@ class TestDownloadManagerDelegate : public content::DownloadManagerDelegate { ...@@ -142,14 +142,14 @@ class TestDownloadManagerDelegate : public content::DownloadManagerDelegate {
BrowserThread::UI, FROM_HERE, BrowserThread::UI, FROM_HERE,
base::Bind(&DownloadManager::FileSelectionCanceled, base::Bind(&DownloadManager::FileSelectionCanceled,
download_manager_, download_manager_,
base::Unretained(data))); download_id));
} else { } else {
BrowserThread::PostTask( BrowserThread::PostTask(
BrowserThread::UI, FROM_HERE, BrowserThread::UI, FROM_HERE,
base::Bind(&DownloadManager::FileSelected, base::Bind(&DownloadManager::FileSelected,
download_manager_, download_manager_,
file_selection_response_, file_selection_response_,
base::Unretained(data))); download_id));
} }
expected_suggested_path_.clear(); expected_suggested_path_.clear();
file_selection_response_.clear(); file_selection_response_.clear();
...@@ -248,8 +248,8 @@ class DownloadManagerTest : public testing::Test { ...@@ -248,8 +248,8 @@ class DownloadManagerTest : public testing::Test {
download_manager_->OnResponseCompleted(download_id, size, hash); download_manager_->OnResponseCompleted(download_id, size, hash);
} }
void FileSelected(const FilePath& path, void* params) { void FileSelected(const FilePath& path, int32 download_id) {
download_manager_->FileSelected(path, params); download_manager_->FileSelected(path, download_id);
} }
void ContinueDownloadWithPath(DownloadItem* download, const FilePath& path) { void ContinueDownloadWithPath(DownloadItem* download, const FilePath& path) {
...@@ -911,14 +911,12 @@ TEST_F(DownloadManagerTest, DownloadRenameTest) { ...@@ -911,14 +911,12 @@ TEST_F(DownloadManagerTest, DownloadRenameTest) {
state.danger = kDownloadRenameCases[i].danger; state.danger = kDownloadRenameCases[i].danger;
download->SetFileCheckResults(state); download->SetFileCheckResults(state);
int32* id_ptr = new int32;
*id_ptr = i; // Deleted in FileSelected().
if (kDownloadRenameCases[i].finish_before_rename) { if (kDownloadRenameCases[i].finish_before_rename) {
OnResponseCompleted(i, 1024, std::string("fake_hash")); OnResponseCompleted(i, 1024, std::string("fake_hash"));
message_loop_.RunAllPending(); message_loop_.RunAllPending();
FileSelected(new_path, id_ptr); FileSelected(new_path, i);
} else { } else {
FileSelected(new_path, id_ptr); FileSelected(new_path, i);
message_loop_.RunAllPending(); message_loop_.RunAllPending();
OnResponseCompleted(i, 1024, std::string("fake_hash")); OnResponseCompleted(i, 1024, std::string("fake_hash"));
} }
......
...@@ -226,8 +226,8 @@ class CONTENT_EXPORT DownloadManager ...@@ -226,8 +226,8 @@ class CONTENT_EXPORT DownloadManager
virtual void ClearLastDownloadPath() = 0; virtual void ClearLastDownloadPath() = 0;
// Called by the delegate after the save as dialog is closed. // Called by the delegate after the save as dialog is closed.
virtual void FileSelected(const FilePath& path, void* params) = 0; virtual void FileSelected(const FilePath& path, int32 download_id) = 0;
virtual void FileSelectionCanceled(void* params) = 0; virtual void FileSelectionCanceled(int32 download_id) = 0;
// Called by the delegate if it delayed the download in // Called by the delegate if it delayed the download in
// DownloadManagerDelegate::ShouldStartDownload and now is ready. // DownloadManagerDelegate::ShouldStartDownload and now is ready.
......
...@@ -43,7 +43,7 @@ class CONTENT_EXPORT DownloadManagerDelegate { ...@@ -43,7 +43,7 @@ class CONTENT_EXPORT DownloadManagerDelegate {
// give the answer. // give the answer.
virtual void ChooseDownloadPath(WebContents* web_contents, virtual void ChooseDownloadPath(WebContents* web_contents,
const FilePath& suggested_path, const FilePath& suggested_path,
void* data) {} int32 download_id) {}
// Allows the embedder to set an intermediate name for the download until it's // Allows the embedder to set an intermediate name for the download until it's
// complete. If the embedder doesn't want this return the suggested path. // complete. If the embedder doesn't want this return the suggested path.
......
// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Copyright (c) 2012 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
...@@ -104,7 +104,7 @@ void ShellDownloadManagerDelegate::RestartDownload( ...@@ -104,7 +104,7 @@ void ShellDownloadManagerDelegate::RestartDownload(
void ShellDownloadManagerDelegate::ChooseDownloadPath( void ShellDownloadManagerDelegate::ChooseDownloadPath(
WebContents* web_contents, WebContents* web_contents,
const FilePath& suggested_path, const FilePath& suggested_path,
void* data) { int32 download_id) {
FilePath result; FilePath result;
#if defined(OS_WIN) #if defined(OS_WIN)
std::wstring file_part = FilePath(suggested_path).BaseName().value(); std::wstring file_part = FilePath(suggested_path).BaseName().value();
...@@ -132,9 +132,9 @@ void ShellDownloadManagerDelegate::ChooseDownloadPath( ...@@ -132,9 +132,9 @@ void ShellDownloadManagerDelegate::ChooseDownloadPath(
#endif #endif
if (result.empty()) { if (result.empty()) {
download_manager_->FileSelectionCanceled(data); download_manager_->FileSelectionCanceled(download_id);
} else { } else {
download_manager_->FileSelected(result, data); download_manager_->FileSelected(result, download_id);
} }
} }
......
// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Copyright (c) 2012 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
...@@ -28,7 +28,7 @@ class ShellDownloadManagerDelegate ...@@ -28,7 +28,7 @@ class ShellDownloadManagerDelegate
virtual bool ShouldStartDownload(int32 download_id) OVERRIDE; virtual bool ShouldStartDownload(int32 download_id) OVERRIDE;
virtual void ChooseDownloadPath(WebContents* web_contents, virtual void ChooseDownloadPath(WebContents* web_contents,
const FilePath& suggested_path, const FilePath& suggested_path,
void* data) OVERRIDE; int32 download_id) OVERRIDE;
private: private:
friend class base::RefCountedThreadSafe<ShellDownloadManagerDelegate>; friend class base::RefCountedThreadSafe<ShellDownloadManagerDelegate>;
......
...@@ -76,8 +76,8 @@ class MockDownloadManager : public content::DownloadManager { ...@@ -76,8 +76,8 @@ class MockDownloadManager : public content::DownloadManager {
bool is_otr, bool is_otr,
content::DownloadItem::Observer* observer)); content::DownloadItem::Observer* observer));
MOCK_METHOD0(ClearLastDownloadPath, void()); MOCK_METHOD0(ClearLastDownloadPath, void());
MOCK_METHOD2(FileSelected, void(const FilePath& path, void* params)); MOCK_METHOD2(FileSelected, void(const FilePath& path, int32 download_id));
MOCK_METHOD1(FileSelectionCanceled, void(void* params)); MOCK_METHOD1(FileSelectionCanceled, void(int32 download_id));
MOCK_METHOD1(RestartDownload, void(int32 download_id)); MOCK_METHOD1(RestartDownload, void(int32 download_id));
MOCK_METHOD0(CheckForHistoryFilesRemoval, void()); MOCK_METHOD0(CheckForHistoryFilesRemoval, void());
MOCK_METHOD1(GetDownloadItem, content::DownloadItem*(int id)); MOCK_METHOD1(GetDownloadItem, content::DownloadItem*(int id));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment