Add a UI test to make sure we sanitize the filename

of files saved using the "Save page as..." menu item.

This involves adding a new automation provider method
for disabling the dialog prompting for a filename.

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@7683 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 121 insertions, 15 deletions

diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
index 4b3c2e4..fcc43fa 100644
--- a/chrome/browser/automation/automation_provider.cc
+++ b/chrome/browser/automation/automation_provider.cc
@@ -820,6 +820,8 @@ void AutomationProvider::OnMessageReceived(const IPC::Message& message) {
                         GetPageCurrentEncoding)
     IPC_MESSAGE_HANDLER(AutomationMsg_OverrideEncodingRequest,
                         OverrideEncoding)
+    IPC_MESSAGE_HANDLER(AutomationMsg_SavePackageShouldPromptUser,
+                        SavePackageShouldPromptUser)
   IPC_END_MESSAGE_MAP()
 }
 
@@ -2578,3 +2580,8 @@ void AutomationProvider::OverrideEncoding(const IPC::Message& message,
   Send(new AutomationMsg_OverrideEncodingResponse(message.routing_id(),
                                                   succeed));
 }
+
+void AutomationProvider::SavePackageShouldPromptUser(
+    const IPC::Message& message, bool should_prompt) {
+  SavePackage::SetShouldPromptUser(should_prompt);
+}
diff --git a/chrome/browser/automation/automation_provider.h b/chrome/browser/automation/automation_provider.h
index f539841..5467b79 100644
--- a/chrome/browser/automation/automation_provider.h
+++ b/chrome/browser/automation/automation_provider.h
@@ -359,6 +359,9 @@ class AutomationProvider : public base::RefCounted<AutomationProvider>,
                         int tab_handle,
                         const std::wstring& encoding_name);
 
+  void SavePackageShouldPromptUser(const IPC::Message& message,
+                                   bool should_prompt);
+
   // Convert a tab handle into a WebContents. If |tab| is non-NULL a pointer
   // to the tab is also returned. Returns NULL in case of failure or if the tab
   // is not of the WebContents type.
diff --git a/chrome/browser/download/save_package.cc b/chrome/browser/download/save_package.cc
index a67d890..f8b85af 100644
--- a/chrome/browser/download/save_package.cc
+++ b/chrome/browser/download/save_package.cc
@@ -40,23 +40,31 @@
 
 using base::Time;
 
+namespace {
+
 // Default name which will be used when we can not get proper name from
 // resource URL.
-static const wchar_t kDefaultSaveName[] = L"saved_resource";
+const wchar_t kDefaultSaveName[] = L"saved_resource";
 
 // Maximum number of file ordinal number. I think it's big enough for resolving
 // name-conflict files which has same base file name.
-static const int32 kMaxFileOrdinalNumber = 9999;
+const int32 kMaxFileOrdinalNumber = 9999;
 
 // Maximum length for file path. Since Windows have MAX_PATH limitation for
 // file path, we need to make sure length of file path of every saved file
 // is less than MAX_PATH
-static const uint32 kMaxFilePathLength = MAX_PATH - 1;
+const uint32 kMaxFilePathLength = MAX_PATH - 1;
 
 // Maximum length for file ordinal number part. Since we only support the
 // maximum 9999 for ordinal number, which means maximum file ordinal number part
 // should be "(9998)", so the value is 6.
-static const uint32 kMaxFileOrdinalNumberPartLength = 6;
+const uint32 kMaxFileOrdinalNumberPartLength = 6;
+
+// If false, we don't prompt the user as to where to save the file.  This
+// exists only for testing.
+bool g_should_prompt_for_filename = true;
+
+}  // namespace
 
 SavePackage::SavePackage(WebContents* web_content,
                          SavePackageType save_type,
@@ -885,6 +893,10 @@ void SavePackage::OnReceivedSavableResourceLinksForCurrentPage(
   }
 }
 
+void SavePackage::SetShouldPromptUser(bool should_prompt) {
+  g_should_prompt_for_filename = should_prompt;
+}
+
 std::wstring SavePackage::GetSuggestNameForSaveAs(PrefService* prefs,
                                                   const std::wstring& name) {
   // Check whether the preference has the preferred directory for saving file.
@@ -934,17 +946,25 @@ bool SavePackage::GetSaveInfo(const std::wstring& suggest_name,
     filter[filter.size() - 1] = L'\0';
     filter[filter.size() - 2] = L'\0';
 
-    if (!win_util::SaveFileAsWithFilter(container_hwnd,
-                                        suggest_name,
-                                        filter,
-                                        L"htm",
-                                        &index,
-                                        &param->saved_main_file_path))
-      return false;
+    if (g_should_prompt_for_filename) {
+      if (!win_util::SaveFileAsWithFilter(container_hwnd,
+                                          suggest_name,
+                                          filter,
+                                          L"htm",
+                                          &index,
+                                          &param->saved_main_file_path))
+        return false;
+    } else {
+      param->saved_main_file_path = suggest_name;
+    }
   } else {
-    if (!win_util::SaveFileAs(container_hwnd, suggest_name,
-                              &param->saved_main_file_path))
-      return false;
+    if (g_should_prompt_for_filename) {
+      if (!win_util::SaveFileAs(container_hwnd, suggest_name,
+                                &param->saved_main_file_path))
+        return false;
+    } else {
+      param->saved_main_file_path = suggest_name;
+    }
     // Set save-as type to only-HTML if the contents of current tab can not be
     // saved as complete-HTML.
     index = 1;
@@ -952,7 +972,7 @@ bool SavePackage::GetSaveInfo(const std::wstring& suggest_name,
 
   DCHECK(download_manager);
   // Ensure the filename is safe.
-  FilePath path;
+  FilePath path(param->saved_main_file_path);
   download_manager->GenerateSafeFilename(param->current_tab_mime_type, &path);
   param->saved_main_file_path = path.ToWStringHack();
 
diff --git a/chrome/browser/download/save_package.h b/chrome/browser/download/save_package.h
index 6b53bb8..3363df5 100644
--- a/chrome/browser/download/save_package.h
+++ b/chrome/browser/download/save_package.h
@@ -134,6 +134,10 @@ class SavePackage : public base::RefCountedThreadSafe<SavePackage>,
 
   // Statics -------------------------------------------------------------------
 
+  // Used to disable prompting the user for a directory/filename of the saved
+  // web page.  This is available for testing.
+  static void SetShouldPromptUser(bool should_prompt);
+
   // Helper function for preparing suggested name for the SaveAs Dialog. The
   // suggested name is composed of the default save path and the web document's
   // title.
diff --git a/chrome/browser/download/save_page_uitest.cc b/chrome/browser/download/save_page_uitest.cc
index 0c75c21..be422f8 100644
--- a/chrome/browser/download/save_page_uitest.cc
+++ b/chrome/browser/download/save_page_uitest.cc
@@ -5,9 +5,11 @@
 #include "base/file_util.h"
 #include "base/path_service.h"
 #include "base/string_util.h"
+#include "chrome/app/chrome_dll_resource.h"
 #include "chrome/browser/automation/url_request_mock_http_job.h"
 #include "chrome/browser/download/save_package.h"
 #include "chrome/common/chrome_paths.h"
+#include "chrome/test/automation/browser_proxy.h"
 #include "chrome/test/automation/tab_proxy.h"
 #include "chrome/test/ui/ui_test.h"
 #include "net/url_request/url_request_unittest.h"
@@ -53,9 +55,18 @@ class SavePageTest : public UITest {
     UITest::SetUp();
     EXPECT_TRUE(file_util::CreateNewTempDirectory(L"", &save_dir_));
     save_dir_ += FilePath::kSeparators[0];
+
+    download_dir_ = GetDownloadDirectory();
+    download_dir_ += FilePath::kSeparators[0];
+  }
+
+  virtual void TearDown() {
+    UITest::TearDown();
+    DieFileDie(save_dir_, true);
   }
 
   std::wstring save_dir_;
+  std::wstring download_dir_;
 };
 
 TEST_F(SavePageTest, SaveHTMLOnly) {
@@ -110,3 +121,48 @@ TEST_F(SavePageTest, NoSave) {
   EXPECT_FALSE(WaitForDownloadShelfVisible(tab.get()));
 }
 
+TEST_F(SavePageTest, FilenameFromPageTitle) {
+  std::wstring file_name = L"b.htm";
+  std::wstring full_file_name = download_dir_ +
+      L"Test page for saving page feature.htm";
+  std::wstring dir = download_dir_ +
+      L"Test page for saving page feature_files";
+
+  GURL url = URLRequestMockHTTPJob::GetMockUrl(kTestDir + L"/" + file_name);
+  scoped_ptr<TabProxy> tab(GetActiveTab());
+  ASSERT_TRUE(tab->NavigateToURL(url));
+  WaitUntilTabCount(1);
+
+  scoped_ptr<BrowserProxy> browser(automation()->GetBrowserWindow(0));
+  automation()->SavePackageShouldPromptUser(false);
+  EXPECT_TRUE(browser->RunCommand(IDC_SAVE_PAGE));
+  EXPECT_TRUE(WaitForDownloadShelfVisible(tab.get()));
+  automation()->SavePackageShouldPromptUser(true);
+
+  CheckFile(dir + L"\\1.png", L"1.png", true);
+  CheckFile(dir + L"\\1.css", L"1.css", true);
+  CheckFile(full_file_name, file_name, false);
+  EXPECT_TRUE(DieFileDie(full_file_name, false));
+  EXPECT_TRUE(DieFileDie(dir, true));
+}
+
+TEST_F(SavePageTest, CleanFilenameFromPageTitle) {
+  std::wstring file_name = L"c.htm";
+  std::wstring full_file_name = download_dir_ + L"test.htm";
+  std::wstring dir = download_dir_ + L"test_files";
+
+  GURL url = URLRequestMockHTTPJob::GetMockUrl(kTestDir + L"/" + file_name);
+  scoped_ptr<TabProxy> tab(GetActiveTab());
+  ASSERT_TRUE(tab->NavigateToURL(url));
+  WaitUntilTabCount(1);
+
+  scoped_ptr<BrowserProxy> browser(automation()->GetBrowserWindow(0));
+  automation()->SavePackageShouldPromptUser(false);
+  EXPECT_TRUE(browser->RunCommand(IDC_SAVE_PAGE));
+  EXPECT_TRUE(WaitForDownloadShelfVisible(tab.get()));
+  automation()->SavePackageShouldPromptUser(true);
+
+  CheckFile(full_file_name, file_name, false);
+  EXPECT_TRUE(DieFileDie(full_file_name, false));
+  EXPECT_TRUE(DieFileDie(dir, true));
+}
diff --git a/chrome/test/automation/automation_messages_internal.h b/chrome/test/automation/automation_messages_internal.h
index 47bb11a..6ed478c 100644
--- a/chrome/test/automation/automation_messages_internal.h
+++ b/chrome/test/automation/automation_messages_internal.h
@@ -861,4 +861,9 @@ IPC_BEGIN_MESSAGES(Automation, 0)
   IPC_MESSAGE_ROUTED1(AutomationMsg_OverrideEncodingResponse,
                       bool /* success */)
 
+  // Used to disable the dialog box that prompts the user for a path when
+  // saving a web page.
+  IPC_MESSAGE_ROUTED1(AutomationMsg_SavePackageShouldPromptUser,
+                      bool /* false if we want to not show the dialog */)
+
 IPC_END_MESSAGES(Automation)
diff --git a/chrome/test/automation/automation_proxy.cc b/chrome/test/automation/automation_proxy.cc
index 87ed3680..59303b6 100644
--- a/chrome/test/automation/automation_proxy.cc
+++ b/chrome/test/automation/automation_proxy.cc
@@ -241,6 +241,10 @@ void AutomationProxy::SignalNewTabUITab(int load_time) {
   ::SetEvent(new_tab_ui_load_complete_);
 }
 
+bool AutomationProxy::SavePackageShouldPromptUser(bool should_prompt) {
+  return Send(new AutomationMsg_SavePackageShouldPromptUser(0, should_prompt));
+}
+
 bool AutomationProxy::GetBrowserWindowCount(int* num_windows) {
   if (!num_windows) {
     NOTREACHED();
diff --git a/chrome/test/automation/automation_proxy.h b/chrome/test/automation/automation_proxy.h
index f068a0d..6632604 100644
--- a/chrome/test/automation/automation_proxy.h
+++ b/chrome/test/automation/automation_proxy.h
@@ -174,6 +174,11 @@ class AutomationProxy : public IPC::Channel::Listener,
   // load_time is how long, in ms, the tab contents took to load.
   void SignalNewTabUITab(int load_time);
 
+  // Set whether or not running the save page as... command show prompt the
+  // user for a download path.  Returns true if the message is successfully
+  // sent.
+  bool SavePackageShouldPromptUser(bool should_prompt);
+
   // Returns the ID of the automation IPC channel, so that it can be
   // passed to the app as a launch parameter.
   const std::wstring& channel_id() const { return channel_id_; }
diff --git a/chrome/test/data/save_page/c.htm b/chrome/test/data/save_page/c.htm
new file mode 100644
index 0000000..a7080046
--- /dev/null
+++ b/chrome/test/data/save_page/c.htm
@@ -0,0 +1,2 @@
+<title>test.exe</title>
+<p>Make sure the saved page is named test.htm.</p>
\ No newline at end of file