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

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

Issue 6973052: When the download folder does not exist, change the download folder to a user's "Downloads" (Closed) Base URL: http://git.chromium.org/git/chromium.git@trunk
Patch Set: Applied Pawel's comments Created 9 years, 4 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/save_page_browsertest.cc
diff --git a/chrome/browser/download/save_page_browsertest.cc b/chrome/browser/download/save_page_browsertest.cc
index bb923a3e7307583b0a68bffcb0522d94631039af..da5790542c90f934f0cfab198b7611db07d79a9d 100644
--- a/chrome/browser/download/save_page_browsertest.cc
+++ b/chrome/browser/download/save_page_browsertest.cc
@@ -4,18 +4,26 @@
#include "base/file_path.h"
#include "base/file_util.h"
+#include "base/i18n/file_util_icu.h"
#include "base/path_service.h"
#include "base/scoped_temp_dir.h"
+#include "base/string_util.h"
+#include "base/utf_string_conversions.h"
#include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/download/download_history.h"
#include "chrome/browser/download/download_item.h"
#include "chrome/browser/download/download_manager.h"
#include "chrome/browser/history/download_history_info.h"
+#include "chrome/browser/download/download_prefs.h"
+#include "chrome/browser/download/download_util.h"
+#include "chrome/browser/prefs/pref_service.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/webui/active_downloads_ui.h"
#include "chrome/common/chrome_paths.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/common/random.h"
#include "chrome/common/url_constants.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
@@ -23,12 +31,16 @@
#include "content/browser/tab_contents/tab_contents.h"
#include "content/common/content_notification_types.h"
#include "content/common/notification_service.h"
+#include "net/url_request/url_request_filter.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace {
static const FilePath::CharType* kTestDir = FILE_PATH_LITERAL("save_page");
+// The test file used in SavePageBrowserTest.SaveFolder3.
+static const FilePath kTestFile(FILE_PATH_LITERAL("a.htm"));
+
static const char* kAppendedExtension =
#if defined(OS_WIN)
".htm";
@@ -81,6 +93,59 @@ class SavePageBrowserTest : public InProcessBrowserTest {
#endif
}
+ // Changes the default folder prefs. This method saves the current folder
+ // for saving HTML, the current folder for saving downloaded files,
+ // the current user's "Downloads" folder and a save type (HTML only or
+ // complete HTML files), and then changes them to |website_save_dir|,
+ // |download_save_dir| and |save_type|, respectively.
+ // If we call this method, we must call RestoreDirectoryPrefs()
+ // after the test to restore the default folder prefs.
+ void ChangeDirectoryPrefs(
+ Profile* profile,
+ const FilePath& website_save_dir,
+ const FilePath& download_save_dir,
+ const SavePackage::SavePackageType save_type) {
+ DCHECK(profile);
+ PrefService* prefs = profile->GetPrefs();
+
+ ASSERT_TRUE(prefs->FindPreference(prefs::kDownloadDefaultDirectory));
+ prev_download_save_dir_ = prefs->GetFilePath(
+ prefs::kDownloadDefaultDirectory);
+
+ // Check whether the preference has the default folder for saving HTML.
+ // If not, initialize it with the default folder for downloaded files.
+ if (!prefs->FindPreference(prefs::kSaveFileDefaultDirectory)) {
+ prefs->RegisterFilePathPref(prefs::kSaveFileDefaultDirectory,
+ prev_download_save_dir_,
+ PrefService::UNSYNCABLE_PREF);
+ }
+ prev_website_save_dir_ = prefs->GetFilePath(
+ prefs::kSaveFileDefaultDirectory);
+
+ DownloadPrefs* download_prefs =
+ profile->GetDownloadManager()->download_prefs();
+ prev_save_type_ =
+ static_cast<SavePackage::SavePackageType>
+ (download_prefs->save_file_type());
+
+ prefs->SetFilePath(
+ prefs::kSaveFileDefaultDirectory, website_save_dir);
+ prefs->SetFilePath(
+ prefs::kDownloadDefaultDirectory, download_save_dir);
+ prefs->SetInteger(prefs::kSaveFileType, save_type);
+ }
+
+ // Restores the default folder prefs.
+ void RestoreDirectoryPrefs(Profile* profile) {
+ DCHECK(profile);
+ PrefService* prefs = profile->GetPrefs();
+ prefs->SetFilePath(
+ prefs::kSaveFileDefaultDirectory, prev_website_save_dir_);
+ prefs->SetFilePath(
+ prefs::kDownloadDefaultDirectory, prev_download_save_dir_);
+ prefs->SetInteger(prefs::kSaveFileType, prev_save_type_);
+ }
+
DownloadManager* GetDownloadManager() const {
DownloadManager* download_manager =
browser()->profile()->GetDownloadManager();
@@ -107,6 +172,15 @@ class SavePageBrowserTest : public InProcessBrowserTest {
MessageLoopForUI::current()->Quit();
}
+ // Returns "src/chrome/test/data/{kTestFile}", whatever URL is given.
+ static net::URLRequestJob* FactoryForTestFile(
+ net::URLRequest* request, const std::string& scheme) {
+ FilePath test_dir;
+ EXPECT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_dir));
+ return new URLRequestMockHTTPJob(
+ request, test_dir.Append(kTestDir).Append(kTestFile));
+ }
+
struct DownloadHistoryInfoMatch
: public std::unary_function<DownloadHistoryInfo, bool> {
@@ -149,8 +223,15 @@ class SavePageBrowserTest : public InProcessBrowserTest {
// Temporary directory we will save pages to.
ScopedTempDir save_dir_;
+
+ // Temporarily stores the default folder prefs.
+ FilePath prev_website_save_dir_;
+ FilePath prev_download_save_dir_;
+ SavePackage::SavePackageType prev_save_type_;
};
+} // namespace
+
IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveHTMLOnly) {
FilePath file_name(FILE_PATH_LITERAL("a.htm"));
GURL url = URLRequestMockHTTPJob::GetMockUrl(
@@ -238,6 +319,214 @@ IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveCompleteHTML) {
dir.AppendASCII("1.css")));
}
+// Checks if an HTML page is saved to the default folder for saving HTML
+// in the following situation:
+// The default folder for saving HTML exists.
+// The default folder for downloaded files exists.
+// The user's "Downloads" folder exists.
+IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveFolder1) {
+ FilePath file(FILE_PATH_LITERAL("a.htm"));
+ GURL url = URLRequestMockHTTPJob::GetMockUrl(
+ FilePath(kTestDir).Append(file));
+ ui_test_utils::NavigateToURL(browser(), url);
+
+ TabContents* current_tab = browser()->GetSelectedTabContents();
+ ASSERT_TRUE(current_tab);
+ ASSERT_TRUE(browser()->profile());
+
+ ScopedTempDir website_save_dir, download_save_dir;
+ // Prepare the default folder for saving HTML.
+ ASSERT_TRUE(website_save_dir.CreateUniqueTempDir());
+ // Prepare the default folder for downloaded files.
+ ASSERT_TRUE(download_save_dir.CreateUniqueTempDir());
+
+ // Changes the default prefs.
+ ChangeDirectoryPrefs(
+ browser()->profile(),
+ website_save_dir.path(),
+ download_save_dir.path(),
+ SavePackage::SAVE_AS_ONLY_HTML);
+
+ string16 title = current_tab->SavePageBasedOnDefaultPrefs();
+ FilePath::StringType basename;
+#if defined(OS_WIN)
+ basename = UTF16ToWide(title);
+ basename.append(FILE_PATH_LITERAL(".htm"));
+#else
+ basename = UTF16ToASCII(title);
+ basename.append(FILE_PATH_LITERAL(".html"));
+#endif
+ file_util::ReplaceIllegalCharactersInPath(&basename, ' ');
+ FilePath downloaded_file = website_save_dir.path().Append(FilePath(basename));
+
+ EXPECT_EQ(url, WaitForSavePackageToFinish());
+
+ CheckDownloadUI(downloaded_file);
+
+ // Is the file downloaded to the default folder for saving HTML?
+ EXPECT_TRUE(file_util::PathExists(downloaded_file));
+ EXPECT_TRUE(file_util::ContentsEqual(
+ test_dir_.Append(FilePath(kTestDir)).Append(file),
+ downloaded_file));
+
+ RestoreDirectoryPrefs(browser()->profile());
+}
+
+// Checks if an HTML page is saved to the default folder for downloaded files
+// in the following situation:
+// The default folder for saving HTML does not exist.
+// The default folder for downloaded files exists.
+// The user's "Downloads" folder exists.
+IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveFolder2) {
+ FilePath file(FILE_PATH_LITERAL("a.htm"));
+ GURL url = URLRequestMockHTTPJob::GetMockUrl(
+ FilePath(kTestDir).Append(file));
+ ui_test_utils::NavigateToURL(browser(), url);
+
+ TabContents* current_tab = browser()->GetSelectedTabContents();
+ ASSERT_TRUE(current_tab);
+ ASSERT_TRUE(browser()->profile());
+
+ ScopedTempDir download_save_dir;
+ // Prepare the default folder for saving downloaded files.
+ ASSERT_TRUE(download_save_dir.CreateUniqueTempDir());
+ // Prepare non-existent folder.
+ FilePath nonexistent_path(
+ FILE_PATH_LITERAL("/tmp/koakuma_mikity_moemoe_nyannyan"));
+ ASSERT_FALSE(file_util::PathExists(nonexistent_path));
+
+ // Changes the default prefs.
+ ChangeDirectoryPrefs(
+ browser()->profile(),
+ nonexistent_path,
+ download_save_dir.path(),
+ SavePackage::SAVE_AS_ONLY_HTML);
+
+ string16 title = current_tab->SavePageBasedOnDefaultPrefs();
+ FilePath::StringType basename;
+#if defined(OS_WIN)
+ basename = UTF16ToWide(title);
+ basename.append(FILE_PATH_LITERAL(".htm"));
+#else
+ basename = UTF16ToASCII(title);
+ basename.append(FILE_PATH_LITERAL(".html"));
+#endif
+ file_util::ReplaceIllegalCharactersInPath(&basename, ' ');
+ FilePath downloaded_file =
+ download_save_dir.path().Append(FilePath(basename));
+
+ EXPECT_EQ(url, WaitForSavePackageToFinish());
+
+ CheckDownloadUI(downloaded_file);
+
+ // Is the file downloaded to the default folder for downloaded files?
+ EXPECT_TRUE(file_util::PathExists(downloaded_file));
+ EXPECT_FALSE(file_util::PathExists(nonexistent_path));
+ EXPECT_TRUE(file_util::ContentsEqual(
+ test_dir_.Append(FilePath(kTestDir)).Append(file),
+ downloaded_file));
+
+ RestoreDirectoryPrefs(browser()->profile());
+}
+
+// Checks if an HTML page is saved to the user's "Downloads" folder
+// in the following situation:
+// The default folder for saving HTML does not exist.
+// The default folder for downloaded files does not exist.
+// The user's "Downloads" folder exists.
+//
+// This test creates and deletes a file on the user's real "Downloads" folder,
+// which is globally shared among all tests on the testing environment.
+// Therefore, if we run browser tests in parallel, the file created by one
+// browser test may be deleted by another broswer test when the file name
+// conflicts. In order to avoid this problem, we use a special mock URL
+// "http://mock.testfile.http/<random path>" for this download test.
+// Since we redirect "http://mock.testfile.http/<random path>" to
+// "chrome/test/data/{kTestFile}" using FactoryForTestFile(),
+// "chrome/test/data/{kTestFile}" is used for the file to be downloaded.
+// Then, the downloaded file is saved as a name "Downloads/<random path>",
+// which is a unique file name in the user's real "Downloads" folder.
+//
+// Ideally, in the first place, we should not use the user's "Downloads" folder.
+// Instead, we should create a temporary "Downloads" folder for each test.
+// However, we concluded that creating temporary "Downloads" folder for each
+// test requires very invasive code changes to many places. See also here:
+// http://codereview.chromium.org/6973052/
+IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveFolder3) {
+ FilePath file(kTestFile);
+
+ // Redirects "http://mock.testfile.http/<random path>"
+ // to "src/chrome/test/data/{kTestFile}", whatever the <random path> is.
+ std::string kMockHostnameForTestFile = "mock.testfile.http";
+ net::URLRequestFilter* filter = net::URLRequestFilter::GetInstance();
+ filter->AddHostnameHandler(
+ "http", kMockHostnameForTestFile, FactoryForTestFile);
+ std::string random_string = Generate128BitRandomBase64String();
+ RemoveChars(random_string, "/", &random_string);
+ GURL url("http://" + kMockHostnameForTestFile + "/" + random_string);
+ ui_test_utils::NavigateToURL(browser(), url);
+
+ TabContents* current_tab = browser()->GetSelectedTabContents();
+ ASSERT_TRUE(current_tab);
+ ASSERT_TRUE(browser()->profile());
+
+ // Prepare non-existent folder.
+ FilePath nonexistent_path1(
+ FILE_PATH_LITERAL("/tmp/koakuma_mikity_moemoe_nyannyan"));
+ FilePath nonexistent_path2(
+ FILE_PATH_LITERAL("/tmp/koakuma_mikity_moemoe_pyonpyon"));
+ ASSERT_FALSE(file_util::PathExists(nonexistent_path1));
+ ASSERT_FALSE(file_util::PathExists(nonexistent_path2));
+
+ // Changes the default prefs.
+ ChangeDirectoryPrefs(
+ browser()->profile(),
+ nonexistent_path1,
+ nonexistent_path2,
+ SavePackage::SAVE_AS_ONLY_HTML);
+
+ string16 title = current_tab->SavePageBasedOnDefaultPrefs();
+ FilePath::StringType basename;
+#if defined(OS_WIN)
+ basename = UTF16ToWide(title);
+ basename.append(FILE_PATH_LITERAL(".htm"));
+#else
+ basename = UTF16ToASCII(title);
+ basename.append(FILE_PATH_LITERAL(".html"));
+#endif
+ file_util::ReplaceIllegalCharactersInPath(&basename, ' ');
+ FilePath default_download_dir =
+ download_util::GetDefaultDownloadDirectoryFromPathService();
+ FilePath downloaded_file =
+ default_download_dir.Append(FilePath(basename));
+ // Make sure that the target file does not exist.
+ // This fails when the file does not exist, but it is OK.
+ file_util::Delete(downloaded_file, false);
+ // Make sure that the temporary file does not exist.
+ FilePath temporary_file =
+ default_download_dir.Append(FilePath(
+ basename + FILE_PATH_LITERAL(".crdownload")));
+ // This fails when the file does not exist, but it is OK.
+ file_util::Delete(temporary_file, false);
+
+ EXPECT_EQ(url, WaitForSavePackageToFinish());
+
+ CheckDownloadUI(downloaded_file);
+
+ // Is the file downloaded to the user's "Downloads" directory?
+ EXPECT_TRUE(file_util::PathExists(downloaded_file));
+ EXPECT_FALSE(file_util::PathExists(nonexistent_path1));
+ EXPECT_FALSE(file_util::PathExists(nonexistent_path2));
+ EXPECT_TRUE(file_util::ContentsEqual(
+ test_dir_.Append(FilePath(kTestDir)).Append(file), downloaded_file));
+
+ // Clean up the generated files.
+ ASSERT_TRUE(file_util::Delete(downloaded_file, false));
+ ASSERT_TRUE(file_util::Delete(temporary_file, false));
+
+ RestoreDirectoryPrefs(browser()->profile());
+}
+
IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, NoSave) {
ui_test_utils::NavigateToURL(browser(), GURL(chrome::kAboutBlankURL));
ASSERT_TRUE(browser()->command_updater()->SupportsCommand(IDC_SAVE_PAGE));
@@ -317,5 +606,3 @@ IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, RemoveFromList) {
test_dir_.Append(FilePath(kTestDir)).Append(file_name),
full_file_name));
}
-
-}
« no previous file with comments | « chrome/browser/download/download_util.cc ('k') | chrome/browser/ui/webui/options/advanced_options_handler.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698