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

Unified Diff: chrome/browser/download/download_target_determiner_unittest.cc

Issue 14640020: [Resumption 9/11] Handle filename determination for resumed downloads. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Address comments Created 7 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 side-by-side diff with in-line comments
Download patch
Index: chrome/browser/download/download_target_determiner_unittest.cc
diff --git a/chrome/browser/download/download_target_determiner_unittest.cc b/chrome/browser/download/download_target_determiner_unittest.cc
index 15b543a31f26b9cbe17449264cbebe2cf26ff56a..0070b11ad155232e6f9b257ba7bfe86183e1befb 100644
--- a/chrome/browser/download/download_target_determiner_unittest.cc
+++ b/chrome/browser/download/download_target_determiner_unittest.cc
@@ -24,6 +24,7 @@
#include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "chrome/test/base/testing_pref_service_syncable.h"
#include "chrome/test/base/testing_profile.h"
+#include "content/public/browser/download_interrupt_reasons.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_contents_delegate.h"
#include "content/public/test/mock_download_item.h"
@@ -208,6 +209,7 @@ class DownloadTargetDeterminerTest : public ChromeRenderViewHostTestHarness {
// Run |test_case| using |item|.
void RunTestCase(const DownloadTestCase& test_case,
+ const base::FilePath& initial_virtual_path,
content::MockDownloadItem* item);
// Run through |test_case_count| tests in |test_cases|. A new MockDownloadItem
@@ -310,6 +312,8 @@ DownloadTargetDeterminerTest::CreateActiveDownloadItem(
.WillByDefault(ReturnRefOfCopy(std::string()));
ON_CALL(*item, GetId())
.WillByDefault(Return(id));
+ ON_CALL(*item, GetLastReason())
+ .WillByDefault(Return(content::DOWNLOAD_INTERRUPT_REASON_NONE));
ON_CALL(*item, GetMimeType())
.WillByDefault(Return(test_case.mime_type));
ON_CALL(*item, GetReferrerUrl())
@@ -372,12 +376,14 @@ base::FilePath DownloadTargetDeterminerTest::GetPathInDownloadDir(
void DownloadTargetDeterminerTest::RunTestCase(
const DownloadTestCase& test_case,
+ const base::FilePath& initial_virtual_path,
content::MockDownloadItem* item) {
// Kick off the test.
base::WeakPtrFactory<DownloadTargetDeterminerTest> factory(this);
base::RunLoop run_loop;
DownloadTargetDeterminer::Start(
- item, download_prefs_.get(), last_selected_directory_, delegate(),
+ item, initial_virtual_path, download_prefs_.get(),
+ last_selected_directory_, delegate(),
base::Bind(&DownloadTargetDeterminerTest::DownloadTargetVerifier,
factory.GetWeakPtr(), run_loop.QuitClosure(), test_case));
run_loop.Run();
@@ -391,7 +397,7 @@ void DownloadTargetDeterminerTest::RunTestCasesWithActiveItem(
scoped_ptr<content::MockDownloadItem> item(
CreateActiveDownloadItem(i, test_cases[i]));
SCOPED_TRACE(testing::Message() << "Running test case " << i);
- RunTestCase(test_cases[i], item.get());
+ RunTestCase(test_cases[i], base::FilePath(), item.get());
}
}
@@ -976,7 +982,7 @@ TEST_F(DownloadTargetDeterminerTest, TargetDeterminer_InactiveDownload) {
// the user because the download is inactive.
EXPECT_CALL(*delegate(), PromptUserForDownloadPath(_, _, _))
.Times(0);
- RunTestCase(kInactiveTestCases[i], item.get());
+ RunTestCase(test_case, base::FilePath(), item.get());
}
}
@@ -1425,7 +1431,7 @@ TEST_F(DownloadTargetDeterminerTest,
_)).WillOnce(WithArg<4>(
ScheduleCallback2(full_overridden_path, true)));
- RunTestCase(test_case, item.get());
+ RunTestCase(test_case, base::FilePath(), item.get());
// Second case: An extension sets the conflict_action to PROMPT.
EXPECT_CALL(*delegate(), NotifyExtensions(_, _, _))
@@ -1436,7 +1442,7 @@ TEST_F(DownloadTargetDeterminerTest,
_, full_overridden_path, true, DownloadPathReservationTracker::PROMPT, _))
.WillOnce(WithArg<4>(
ScheduleCallback2(full_overridden_path, true)));
- RunTestCase(test_case, item.get());
+ RunTestCase(test_case, base::FilePath(), item.get());
}
// Test that relative paths returned by extensions are always relative to the
@@ -1476,6 +1482,262 @@ TEST_F(DownloadTargetDeterminerTest,
PromptUserForDownloadPath(_, full_overridden_path, _))
.WillOnce(WithArg<2>(
ScheduleCallback(full_overridden_path)));
- RunTestCase(test_case, item.get());
+ RunTestCase(test_case, base::FilePath(), item.get());
}
+
+TEST_F(DownloadTargetDeterminerTest,
+ TargetDeterminer_InitialVirtualPathUnsafe) {
+ const base::FilePath::CharType* kInitialPath =
+ FILE_PATH_LITERAL("some_path/bar.html");
+
+ const DownloadTestCase kInitialPathTestCases[] = {
+ {
+ // 0: Save As Save. The path generated based on the DownloadItem is safe,
+ // but the initial path is unsafe. However, the download is not considered
+ // dangerous since the user has been prompted.
+ SAVE_AS,
+ content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+ "http://example.com/foo.txt", "text/plain",
+ FILE_PATH_LITERAL(""),
+
+ FILE_PATH_LITERAL(""),
+ kInitialPath,
+ DownloadItem::TARGET_DISPOSITION_PROMPT,
+
+ EXPECT_CRDOWNLOAD
+ },
+ };
+
+ for (size_t i = 0; i < arraysize(kInitialPathTestCases); ++i) {
Randy Smith (Not in Mondays) 2013/05/24 01:53:55 Why bother to have the test case loop if we're onl
asanka 2013/05/29 22:30:11 Removed loop. It was supposed to contain multiple
+ SCOPED_TRACE(testing::Message() << "Running test case " << i);
+ const DownloadTestCase& test_case = kInitialPathTestCases[i];
+ scoped_ptr<content::MockDownloadItem> item(
+ CreateActiveDownloadItem(i, test_case));
+ EXPECT_CALL(*item, GetLastReason())
+ .WillRepeatedly(Return(
+ content::DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED));
+ EXPECT_CALL(*item, GetTargetDisposition())
+ .WillRepeatedly(Return(DownloadItem::TARGET_DISPOSITION_PROMPT));
+ RunTestCase(test_case, GetPathInDownloadDir(kInitialPath), item.get());
+ }
+}
+
+// Prompting behavior for resumed downloads is based on the last interrupt
+// reason. If the reason indicates that the target path may not be suitable for
+// the download (ACCESS_DENIED, NO_SPACE, etc..), then the user should be
+// prompted, and not otherwise. These test cases shouldn't result in prompting
+// since the error is set to NETWORK_FAILED.
+TEST_F(DownloadTargetDeterminerTest, TargetDeterminer_ResumedNoPrompt) {
+ // All test cases run with GetPathInDownloadDir(kInitialPath) as the inital
+ // path.
+ const base::FilePath::CharType* kInitialPath =
+ FILE_PATH_LITERAL("some_path/bar.txt");
+
+ const DownloadTestCase kResumedTestCases[] = {
+ {
+ // 0: Automatic Safe: Initial path is ignored since the user has not been
+ // prompted before.
+ AUTOMATIC,
+ content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+ "http://example.com/foo.txt", "text/plain",
+ FILE_PATH_LITERAL(""),
+
+ FILE_PATH_LITERAL(""),
+ FILE_PATH_LITERAL("foo.txt"),
+ DownloadItem::TARGET_DISPOSITION_OVERWRITE,
+
+ EXPECT_CRDOWNLOAD
+ },
+
+ {
+ // 1: Save_As Safe: Initial path used.
+ SAVE_AS,
+ content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+ "http://example.com/foo.txt", "text/plain",
+ FILE_PATH_LITERAL(""),
+
+ FILE_PATH_LITERAL(""),
+ kInitialPath,
+ DownloadItem::TARGET_DISPOSITION_PROMPT,
+
+ EXPECT_CRDOWNLOAD
+ },
+
+ {
+ // 2: Automatic Dangerous: Initial path is ignored since the user hasn't
+ // been prompted before.
+ AUTOMATIC,
+ content::DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE,
+ "http://example.com/foo.html", "",
+ FILE_PATH_LITERAL(""),
+
+ FILE_PATH_LITERAL(""),
+ FILE_PATH_LITERAL("foo.html"),
+ DownloadItem::TARGET_DISPOSITION_OVERWRITE,
+
+ EXPECT_UNCONFIRMED
+ },
+
+ {
+ // 3: Forced Safe: Initial path is ignored due to the forced path.
+ FORCED,
+ content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+ "http://example.com/foo.txt", "",
+ FILE_PATH_LITERAL("forced-foo.txt"),
+
+ FILE_PATH_LITERAL(""),
+ FILE_PATH_LITERAL("forced-foo.txt"),
+ DownloadItem::TARGET_DISPOSITION_OVERWRITE,
+
+ EXPECT_LOCAL_PATH
+ },
+ };
+
+ // The test assumes that .html files have a danger level of
+ // AllowOnUserGesture.
+ ASSERT_EQ(download_util::AllowOnUserGesture,
+ download_util::GetFileDangerLevel(
+ base::FilePath(FILE_PATH_LITERAL("foo.html"))));
+ for (size_t i = 0; i < arraysize(kResumedTestCases); ++i) {
+ SCOPED_TRACE(testing::Message() << "Running test case " << i);
+ const DownloadTestCase& test_case = kResumedTestCases[i];
+ scoped_ptr<content::MockDownloadItem> item(
+ CreateActiveDownloadItem(i, test_case));
+ base::FilePath expected_path =
+ GetPathInDownloadDir(test_case.expected_local_path);
+ ON_CALL(*item.get(), GetLastReason())
+ .WillByDefault(Return(
+ content::DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED));
+ // Extensions should be notified if a new path is being generated and there
+ // is no forced path. In the test cases above, this is true for tests with
+ // type == AUTOMATIC.
+ EXPECT_CALL(*delegate(), NotifyExtensions(_, _, _))
+ .Times(test_case.test_type == AUTOMATIC ? 1 : 0);
+ EXPECT_CALL(*delegate(), ReserveVirtualPath(_, expected_path, false, _, _));
+ EXPECT_CALL(*delegate(), PromptUserForDownloadPath(_, expected_path, _))
+ .Times(0);
+ EXPECT_CALL(*delegate(), DetermineLocalPath(_, expected_path, _));
+ EXPECT_CALL(*delegate(), CheckDownloadUrl(_, expected_path, _));
+ RunTestCase(test_case, GetPathInDownloadDir(kInitialPath), item.get());
+ }
+
+}
+
+// Test that a forced download doesn't prompt, even if the interrupt reason
+// suggests that the target path may not be suitable for downloads.
+TEST_F(DownloadTargetDeterminerTest, TargetDeterminer_ResumedForcedDownload) {
+ const base::FilePath::CharType* kInitialPath =
+ FILE_PATH_LITERAL("some_path/bar.txt");
+ const DownloadTestCase kResumedForcedDownload = {
+ // 3: Forced Safe
+ FORCED,
+ content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+ "http://example.com/foo.txt", "",
+ FILE_PATH_LITERAL("forced-foo.txt"),
+
+ FILE_PATH_LITERAL(""),
+ FILE_PATH_LITERAL("forced-foo.txt"),
+ DownloadItem::TARGET_DISPOSITION_OVERWRITE,
+
+ EXPECT_LOCAL_PATH
+ };
+
+ const DownloadTestCase& test_case = kResumedForcedDownload;
+ base::FilePath expected_path =
+ GetPathInDownloadDir(test_case.expected_local_path);
+ scoped_ptr<content::MockDownloadItem> item(
+ CreateActiveDownloadItem(0, test_case));
+ ON_CALL(*item.get(), GetLastReason())
+ .WillByDefault(Return(
+ content::DOWNLOAD_INTERRUPT_REASON_FILE_NO_SPACE));
+ EXPECT_CALL(*delegate(), NotifyExtensions(_, _, _))
+ .Times(test_case.test_type == AUTOMATIC ? 1 : 0);
+ EXPECT_CALL(*delegate(), ReserveVirtualPath(_, expected_path, false, _, _));
+ EXPECT_CALL(*delegate(), PromptUserForDownloadPath(_, _, _))
+ .Times(0);
+ EXPECT_CALL(*delegate(), DetermineLocalPath(_, expected_path, _));
+ EXPECT_CALL(*delegate(), CheckDownloadUrl(_, expected_path, _));
+ RunTestCase(test_case, GetPathInDownloadDir(kInitialPath), item.get());
+}
+
+// Prompting behavior for resumed downloads is based on the last interrupt
+// reason. If the reason indicates that the target path may not be suitable for
+// the download (ACCESS_DENIED, NO_SPACE, etc..), then the user should be
+// prompted, and not otherwise. These test cases result in prompting since the
+// error is set to NO_SPACE.
+TEST_F(DownloadTargetDeterminerTest, TargetDeterminer_ResumedWithPrompt) {
+ // All test cases run with GetPathInDownloadDir(kInitialPath) as the inital
+ // path.
+ const base::FilePath::CharType* kInitialPath =
+ FILE_PATH_LITERAL("some_path/bar.txt");
+
+ const DownloadTestCase kResumedTestCases[] = {
+ {
+ // 0: Automatic Safe
+ AUTOMATIC,
+ content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+ "http://example.com/foo.txt", "text/plain",
+ FILE_PATH_LITERAL(""),
+
+ FILE_PATH_LITERAL(""),
+ FILE_PATH_LITERAL("foo.txt"),
+ DownloadItem::TARGET_DISPOSITION_PROMPT,
+
+ EXPECT_CRDOWNLOAD
+ },
+
+ {
+ // 1: Save_As Safe
+ SAVE_AS,
+ content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+ "http://example.com/foo.txt", "text/plain",
+ FILE_PATH_LITERAL(""),
+
+ FILE_PATH_LITERAL(""),
+ kInitialPath,
+ DownloadItem::TARGET_DISPOSITION_PROMPT,
+
+ EXPECT_CRDOWNLOAD
+ },
+
+ {
+ // 2: Automatic Dangerous
+ AUTOMATIC,
+ content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+ "http://example.com/foo.html", "",
+ FILE_PATH_LITERAL(""),
+
+ FILE_PATH_LITERAL(""),
+ FILE_PATH_LITERAL("foo.html"),
+ DownloadItem::TARGET_DISPOSITION_PROMPT,
+
+ EXPECT_CRDOWNLOAD
+ },
+ };
+
+ // The test assumes that .html files have a danger level of
+ // AllowOnUserGesture.
+ ASSERT_EQ(download_util::AllowOnUserGesture,
+ download_util::GetFileDangerLevel(
+ base::FilePath(FILE_PATH_LITERAL("foo.html"))));
+ for (size_t i = 0; i < arraysize(kResumedTestCases); ++i) {
+ SCOPED_TRACE(testing::Message() << "Running test case " << i);
+ const DownloadTestCase& test_case = kResumedTestCases[i];
+ base::FilePath expected_path =
+ GetPathInDownloadDir(test_case.expected_local_path);
+ scoped_ptr<content::MockDownloadItem> item(
+ CreateActiveDownloadItem(i, test_case));
+ ON_CALL(*item.get(), GetLastReason())
+ .WillByDefault(Return(
+ content::DOWNLOAD_INTERRUPT_REASON_FILE_NO_SPACE));
+ EXPECT_CALL(*delegate(), NotifyExtensions(_, _, _))
+ .Times(test_case.test_type == AUTOMATIC ? 1 : 0);
+ EXPECT_CALL(*delegate(), ReserveVirtualPath(_, expected_path, false, _, _));
+ EXPECT_CALL(*delegate(), PromptUserForDownloadPath(_, expected_path, _));
+ EXPECT_CALL(*delegate(), DetermineLocalPath(_, expected_path, _));
+ EXPECT_CALL(*delegate(), CheckDownloadUrl(_, expected_path, _));
+ RunTestCase(test_case, GetPathInDownloadDir(kInitialPath), item.get());
+ }
+}
+
} // namespace

Powered by Google App Engine
This is Rietveld 408576698