Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(928)

Side by Side Diff: chrome/browser/download/download_manager_unittest.cc

Issue 6969009: Reduced the lifetime of DownloadCreateInfo. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Removed structure accessors from DownloadItem, per request. Created 9 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch | Annotate | Revision Log
OLDNEW
1 // Copyright (c) 2011 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include <string> 5 #include <string>
6 #include <set> 6 #include <set>
7 7
8 #include "base/file_util.h" 8 #include "base/file_util.h"
9 #include "base/memory/scoped_ptr.h" 9 #include "base/memory/scoped_ptr.h"
10 #include "base/stl_util-inl.h" 10 #include "base/stl_util-inl.h"
(...skipping 43 matching lines...) Expand 10 before | Expand all | Expand 10 after
54 } 54 }
55 55
56 void OnAllDataSaved(int32 download_id, int64 size, const std::string& hash) { 56 void OnAllDataSaved(int32 download_id, int64 size, const std::string& hash) {
57 download_manager_->OnAllDataSaved(download_id, size, hash); 57 download_manager_->OnAllDataSaved(download_id, size, hash);
58 } 58 }
59 59
60 void FileSelected(const FilePath& path, int index, void* params) { 60 void FileSelected(const FilePath& path, int index, void* params) {
61 download_manager_->FileSelected(path, index, params); 61 download_manager_->FileSelected(path, index, params);
62 } 62 }
63 63
64 void AttachDownloadItem(DownloadCreateInfo* info) { 64 void ContinueDownloadWithPath(DownloadCreateInfo* info) {
65 download_manager_->AttachDownloadItem(info); 65 download_manager_->ContinueDownloadWithPath(info->download_id, info->path);
66 } 66 }
67 67
68 void OnDownloadError(int32 download_id, int64 size, int os_error) { 68 void OnDownloadError(int32 download_id, int64 size, int os_error) {
69 download_manager_->OnDownloadError(download_id, size, os_error); 69 download_manager_->OnDownloadError(download_id, size, os_error);
70 } 70 }
71 71
72 // Get the download item with ID |id|. 72 // Get the download item with ID |id|.
73 DownloadItem* GetActiveDownloadItem(int32 id) { 73 DownloadItem* GetActiveDownloadItem(int32 id) {
74 if (ContainsKey(download_manager_->active_downloads_, id)) 74 if (ContainsKey(download_manager_->active_downloads_, id))
75 return download_manager_->active_downloads_[id]; 75 return download_manager_->active_downloads_[id];
(...skipping 229 matching lines...) Expand 10 before | Expand all | Expand 10 after
305 DownloadCreateInfo* info = new DownloadCreateInfo; 305 DownloadCreateInfo* info = new DownloadCreateInfo;
306 info->download_id = static_cast<int>(i); 306 info->download_id = static_cast<int>(i);
307 info->prompt_user_for_save_location = kStartDownloadCases[i].save_as; 307 info->prompt_user_for_save_location = kStartDownloadCases[i].save_as;
308 info->url_chain.push_back(GURL(kStartDownloadCases[i].url)); 308 info->url_chain.push_back(GURL(kStartDownloadCases[i].url));
309 info->mime_type = kStartDownloadCases[i].mime_type; 309 info->mime_type = kStartDownloadCases[i].mime_type;
310 download_manager_->CreateDownloadItem(info); 310 download_manager_->CreateDownloadItem(info);
311 311
312 DownloadFile* download_file(new DownloadFile(info, download_manager_)); 312 DownloadFile* download_file(new DownloadFile(info, download_manager_));
313 AddDownloadToFileManager(info->download_id, download_file); 313 AddDownloadToFileManager(info->download_id, download_file);
314 download_file->Initialize(false); 314 download_file->Initialize(false);
315 download_manager_->StartDownload(info); 315 download_manager_->StartDownload(info->download_id);
316 message_loop_.RunAllPending(); 316 message_loop_.RunAllPending();
317 317
318 // NOTE: At this point, |AttachDownloadItem| will have been run if we don't 318 // NOTE: At this point, |ContinueDownloadWithPath| will have been run if
319 // need to prompt the user, so |info| could have been destructed. 319 // we don't need to prompt the user, so |info| could have been destructed.
320 // This means that we can't check any of its values. 320 // This means that we can't check any of its values.
321 // However, SelectFileObserver will have recorded any attempt to open the 321 // However, SelectFileObserver will have recorded any attempt to open the
322 // select file dialog. 322 // select file dialog.
323 EXPECT_EQ(kStartDownloadCases[i].expected_save_as, 323 EXPECT_EQ(kStartDownloadCases[i].expected_save_as,
324 observer.ShowedFileDialogForId(i)); 324 observer.ShowedFileDialogForId(i));
325 325
326 // If the Save As dialog pops up, it never reached 326 // If the Save As dialog pops up, it never reached
327 // DownloadManager::AttachDownloadItem(), and never deleted info or 327 // DownloadManager::ContinueDownloadWithPath(), and never deleted info or
328 // completed. This cleans up info. 328 // completed. This cleans up info.
329 // Note that DownloadManager::FileSelectionCanceled() is never called. 329 // Note that DownloadManager::FileSelectionCanceled() is never called.
330 if (observer.ShowedFileDialogForId(i)) { 330 if (observer.ShowedFileDialogForId(i)) {
331 delete info; 331 delete info;
332 } 332 }
333 } 333 }
334 } 334 }
335 335
336 TEST_F(DownloadManagerTest, DownloadRenameTest) { 336 TEST_F(DownloadManagerTest, DownloadRenameTest) {
337 using ::testing::_; 337 using ::testing::_;
338 using ::testing::CreateFunctor; 338 using ::testing::CreateFunctor;
339 using ::testing::Invoke; 339 using ::testing::Invoke;
340 using ::testing::Return; 340 using ::testing::Return;
341 341
342 for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kDownloadRenameCases); ++i) { 342 for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kDownloadRenameCases); ++i) {
343 // |info| will be destroyed in download_manager_. 343 // |info| will be destroyed in download_manager_.
344 DownloadCreateInfo* info(new DownloadCreateInfo); 344 DownloadCreateInfo* info(new DownloadCreateInfo);
345 info->download_id = static_cast<int>(i); 345 info->download_id = static_cast<int>(i);
346 info->prompt_user_for_save_location = false; 346 info->prompt_user_for_save_location = false;
347 info->url_chain.push_back(GURL()); 347 info->url_chain.push_back(GURL());
348 info->is_dangerous_file = kDownloadRenameCases[i].is_dangerous_file; 348 info->is_dangerous_file = kDownloadRenameCases[i].is_dangerous_file;
349 info->is_dangerous_url = kDownloadRenameCases[i].is_dangerous_url; 349 info->is_dangerous_url = kDownloadRenameCases[i].is_dangerous_url;
350 FilePath new_path(kDownloadRenameCases[i].suggested_path); 350 const FilePath new_path(kDownloadRenameCases[i].suggested_path);
351 351
352 MockDownloadFile* download_file( 352 MockDownloadFile* download_file(
353 new MockDownloadFile(info, download_manager_)); 353 new MockDownloadFile(info, download_manager_));
354 AddDownloadToFileManager(info->download_id, download_file); 354 AddDownloadToFileManager(info->download_id, download_file);
355 355
356 // |download_file| is owned by DownloadFileManager. 356 // |download_file| is owned by DownloadFileManager.
357 ::testing::Mock::AllowLeak(download_file); 357 ::testing::Mock::AllowLeak(download_file);
358 EXPECT_CALL(*download_file, Destructed()).Times(1); 358 EXPECT_CALL(*download_file, Destructed()).Times(1);
359 359
360 if (kDownloadRenameCases[i].expected_rename_count == 1) { 360 if (kDownloadRenameCases[i].expected_rename_count == 1) {
361 EXPECT_CALL(*download_file, Rename(new_path)).WillOnce(Return(true)); 361 EXPECT_CALL(*download_file, Rename(new_path)).WillOnce(Return(true));
362 } else { 362 } else {
363 ASSERT_EQ(2, kDownloadRenameCases[i].expected_rename_count); 363 ASSERT_EQ(2, kDownloadRenameCases[i].expected_rename_count);
364 FilePath crdownload(download_util::GetCrDownloadPath(new_path)); 364 FilePath crdownload(download_util::GetCrDownloadPath(new_path));
365 EXPECT_CALL(*download_file, Rename(_)) 365 EXPECT_CALL(*download_file, Rename(_))
366 .WillOnce(testing::WithArgs<0>(Invoke(CreateFunctor( 366 .WillOnce(testing::WithArgs<0>(Invoke(CreateFunctor(
367 download_file, &MockDownloadFile::TestMultipleRename, 367 download_file, &MockDownloadFile::TestMultipleRename,
368 1, crdownload)))) 368 1, crdownload))))
369 .WillOnce(testing::WithArgs<0>(Invoke(CreateFunctor( 369 .WillOnce(testing::WithArgs<0>(Invoke(CreateFunctor(
370 download_file, &MockDownloadFile::TestMultipleRename, 370 download_file, &MockDownloadFile::TestMultipleRename,
371 2, new_path)))); 371 2, new_path))));
372 } 372 }
373 download_manager_->CreateDownloadItem(info); 373 download_manager_->CreateDownloadItem(info);
374 374
375 int32* id_ptr = new int32;
376 *id_ptr = i; // Deleted in FileSelected().
375 if (kDownloadRenameCases[i].finish_before_rename) { 377 if (kDownloadRenameCases[i].finish_before_rename) {
376 OnAllDataSaved(i, 1024, std::string("fake_hash")); 378 OnAllDataSaved(i, 1024, std::string("fake_hash"));
377 message_loop_.RunAllPending(); 379 message_loop_.RunAllPending();
378 FileSelected(new_path, i, info); 380 FileSelected(new_path, i, id_ptr);
379 } else { 381 } else {
380 FileSelected(new_path, i, info); 382 FileSelected(new_path, i, id_ptr);
381 message_loop_.RunAllPending(); 383 message_loop_.RunAllPending();
382 OnAllDataSaved(i, 1024, std::string("fake_hash")); 384 OnAllDataSaved(i, 1024, std::string("fake_hash"));
383 } 385 }
384 386
385 message_loop_.RunAllPending(); 387 message_loop_.RunAllPending();
386 EXPECT_TRUE(VerifySafetyState(kDownloadRenameCases[i].is_dangerous_file, 388 EXPECT_TRUE(VerifySafetyState(kDownloadRenameCases[i].is_dangerous_file,
387 kDownloadRenameCases[i].is_dangerous_url, 389 kDownloadRenameCases[i].is_dangerous_url,
388 i)); 390 i));
389 } 391 }
390 } 392 }
(...skipping 28 matching lines...) Expand all
419 421
420 DownloadItem* download = GetActiveDownloadItem(0); 422 DownloadItem* download = GetActiveDownloadItem(0);
421 ASSERT_TRUE(download != NULL); 423 ASSERT_TRUE(download != NULL);
422 424
423 EXPECT_EQ(DownloadItem::IN_PROGRESS, download->state()); 425 EXPECT_EQ(DownloadItem::IN_PROGRESS, download->state());
424 scoped_ptr<ItemObserver> observer(new ItemObserver(download)); 426 scoped_ptr<ItemObserver> observer(new ItemObserver(download));
425 427
426 download_file->AppendDataToFile(kTestData, kTestDataLen); 428 download_file->AppendDataToFile(kTestData, kTestDataLen);
427 429
428 info->path = new_path; 430 info->path = new_path;
429 AttachDownloadItem(info); 431 ContinueDownloadWithPath(info);
430 message_loop_.RunAllPending(); 432 message_loop_.RunAllPending();
431 EXPECT_TRUE(GetActiveDownloadItem(0) != NULL); 433 EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);
432 434
433 OnDownloadError(0, 1024, -6); 435 OnDownloadError(0, 1024, -6);
434 message_loop_.RunAllPending(); 436 message_loop_.RunAllPending();
435 437
436 EXPECT_TRUE(GetActiveDownloadItem(0) == NULL); 438 EXPECT_TRUE(GetActiveDownloadItem(0) == NULL);
437 EXPECT_EQ(DownloadItem::INTERRUPTED, download->state()); 439 EXPECT_EQ(DownloadItem::INTERRUPTED, download->state());
438 EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS)); 440 EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS));
439 EXPECT_TRUE(observer->hit_state(DownloadItem::INTERRUPTED)); 441 EXPECT_TRUE(observer->hit_state(DownloadItem::INTERRUPTED));
(...skipping 43 matching lines...) Expand 10 before | Expand all | Expand 10 after
483 485
484 download_manager_->CreateDownloadItem(info); 486 download_manager_->CreateDownloadItem(info);
485 487
486 DownloadItem* download = GetActiveDownloadItem(0); 488 DownloadItem* download = GetActiveDownloadItem(0);
487 ASSERT_TRUE(download != NULL); 489 ASSERT_TRUE(download != NULL);
488 490
489 EXPECT_EQ(DownloadItem::IN_PROGRESS, download->state()); 491 EXPECT_EQ(DownloadItem::IN_PROGRESS, download->state());
490 scoped_ptr<ItemObserver> observer(new ItemObserver(download)); 492 scoped_ptr<ItemObserver> observer(new ItemObserver(download));
491 493
492 info->path = new_path; 494 info->path = new_path;
493 AttachDownloadItem(info); 495 ContinueDownloadWithPath(info);
494 message_loop_.RunAllPending(); 496 message_loop_.RunAllPending();
495 EXPECT_TRUE(GetActiveDownloadItem(0) != NULL); 497 EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);
496 498
497 download_file->AppendDataToFile(kTestData, kTestDataLen); 499 download_file->AppendDataToFile(kTestData, kTestDataLen);
498 500
499 download->Cancel(false); 501 download->Cancel(false);
500 message_loop_.RunAllPending(); 502 message_loop_.RunAllPending();
501 503
502 EXPECT_TRUE(GetActiveDownloadItem(0) != NULL); 504 EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);
503 EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS)); 505 EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS));
(...skipping 58 matching lines...) Expand 10 before | Expand all | Expand 10 after
562 // properly. 564 // properly.
563 DownloadFile* download_file( 565 DownloadFile* download_file(
564 new DownloadFile(info, download_manager_)); 566 new DownloadFile(info, download_manager_));
565 download_file->Rename(cr_path); 567 download_file->Rename(cr_path);
566 // This creates the .crdownload version of the file. 568 // This creates the .crdownload version of the file.
567 download_file->Initialize(false); 569 download_file->Initialize(false);
568 // |download_file| is owned by DownloadFileManager. 570 // |download_file| is owned by DownloadFileManager.
569 AddDownloadToFileManager(info->download_id, download_file); 571 AddDownloadToFileManager(info->download_id, download_file);
570 572
571 info->path = new_path; 573 info->path = new_path;
572 AttachDownloadItem(info); 574 ContinueDownloadWithPath(info);
573 message_loop_.RunAllPending(); 575 message_loop_.RunAllPending();
574 EXPECT_TRUE(GetActiveDownloadItem(0) != NULL); 576 EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);
575 577
576 download_file->AppendDataToFile(kTestData, kTestDataLen); 578 download_file->AppendDataToFile(kTestData, kTestDataLen);
577 579
578 // Finish the download. 580 // Finish the download.
579 OnAllDataSaved(0, kTestDataLen, ""); 581 OnAllDataSaved(0, kTestDataLen, "");
580 message_loop_.RunAllPending(); 582 message_loop_.RunAllPending();
581 583
582 // Download is complete. 584 // Download is complete.
583 EXPECT_TRUE(GetActiveDownloadItem(0) == NULL); 585 EXPECT_TRUE(GetActiveDownloadItem(0) == NULL);
584 EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS)); 586 EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS));
585 EXPECT_FALSE(observer->hit_state(DownloadItem::CANCELLED)); 587 EXPECT_FALSE(observer->hit_state(DownloadItem::CANCELLED));
586 EXPECT_FALSE(observer->hit_state(DownloadItem::INTERRUPTED)); 588 EXPECT_FALSE(observer->hit_state(DownloadItem::INTERRUPTED));
587 EXPECT_TRUE(observer->hit_state(DownloadItem::COMPLETE)); 589 EXPECT_TRUE(observer->hit_state(DownloadItem::COMPLETE));
588 EXPECT_FALSE(observer->hit_state(DownloadItem::REMOVING)); 590 EXPECT_FALSE(observer->hit_state(DownloadItem::REMOVING));
589 EXPECT_TRUE(observer->was_updated()); 591 EXPECT_TRUE(observer->was_updated());
590 EXPECT_FALSE(observer->was_opened()); 592 EXPECT_FALSE(observer->was_opened());
591 EXPECT_EQ(DownloadItem::COMPLETE, download->state()); 593 EXPECT_EQ(DownloadItem::COMPLETE, download->state());
592 594
593 EXPECT_TRUE(file_util::PathExists(new_path)); 595 EXPECT_TRUE(file_util::PathExists(new_path));
594 EXPECT_FALSE(file_util::PathExists(cr_path)); 596 EXPECT_FALSE(file_util::PathExists(cr_path));
595 EXPECT_FALSE(file_util::PathExists(unique_new_path)); 597 EXPECT_FALSE(file_util::PathExists(unique_new_path));
596 std::string file_contents; 598 std::string file_contents;
597 EXPECT_TRUE(file_util::ReadFileToString(new_path, &file_contents)); 599 EXPECT_TRUE(file_util::ReadFileToString(new_path, &file_contents));
598 EXPECT_EQ(std::string(kTestData), file_contents); 600 EXPECT_EQ(std::string(kTestData), file_contents);
599 } 601 }
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698