Add Cancel test for SavePackage.

chrome/browser/download/save_page_browsertest.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/path_service.h"
 #include "base/scoped_temp_dir.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/download/download_history.h"
@@ skipping 26 matching lines @@
 #endif
 
 class SavePageBrowserTest : public InProcessBrowserTest {
  protected:
   void SetUp() {
     ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_dir_));
     ASSERT_TRUE(save_dir_.CreateUniqueTempDir());
     InProcessBrowserTest::SetUp();
   }
 
+  void GetPaths(const std::string& prefix,
+                GURL* url,
+                FilePath* full_file_name,
+                FilePath* dir) {
+    *url = URLRequestMockHTTPJob::GetMockUrl(
+        FilePath(kTestDir).AppendASCII(prefix + ".htm"));
+    ui_test_utils::NavigateToURL(browser(), *url);

[Paweł Hajdan Jr., 2011/08/11 17:26:40]

GetPath doesn't really correspond to NavigateToURL for me... do you have better
ideas for name of this method?

[achuithb, 2011/08/11 23:21:59]

The problem is that the function does 2 separate things - create URL + navigate
to that url, and initialize the full_file_name and dir variables. The name
GetPaths recognizes the second item but not the navigation part. 

This is really a setup/initialization function before we call SavePage, so maybe

Init or SetupSavePage?

The other option I can think of is to have two separate functions Navigate and
GetPaths.

Do any of these suggestions appeal to you, or do you have any other suggestion?
I'm leaning towards splitting this function into Navigate + GetPaths.

[Paweł Hajdan Jr., 2011/08/12 16:48:57]

SGTM

+
+    *full_file_name = save_dir_.path().AppendASCII(prefix + ".htm");
+    *dir = save_dir_.path().AppendASCII(prefix + "_files");
+  }
+
+  TabContents* GetCurrentTab() const {
+    TabContents* current_tab = browser()->GetSelectedTabContents();
+    EXPECT_TRUE(current_tab);
+    return current_tab;
+  }
+
+
   GURL WaitForSavePackageToFinish() const {
     ui_test_utils::TestNotificationObserver observer;
     ui_test_utils::RegisterAndWait(&observer,
         content::NOTIFICATION_SAVE_PACKAGE_SUCCESSFULLY_FINISHED,
         NotificationService::AllSources());
     return Details<DownloadItem>(observer.details()).ptr()->original_url();
   }
 
 #if defined(OS_CHROMEOS)
   const ActiveDownloadsUI::DownloadList& GetDownloads() const {
@@ skipping 88 matching lines @@
   std::vector<DownloadHistoryInfo> history_entries_;
 
   // Path to directory containing test data.
   FilePath test_dir_;
 
   // Temporary directory we will save pages to.
   ScopedTempDir save_dir_;
 };
 
 IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveHTMLOnly) {
-  FilePath file_name(FILE_PATH_LITERAL("a.htm"));
-  GURL url = URLRequestMockHTTPJob::GetMockUrl(
-      FilePath(kTestDir).Append(file_name));
-  ui_test_utils::NavigateToURL(browser(), url);
-
-  TabContents* current_tab = browser()->GetSelectedTabContents();
-  ASSERT_TRUE(current_tab);
-
-  FilePath full_file_name = save_dir_.path().Append(file_name);
-  FilePath dir = save_dir_.path().AppendASCII("a_files");
-  ASSERT_TRUE(current_tab->SavePage(full_file_name, dir,
+  GURL url;
+  FilePath full_file_name, dir;
+  GetPaths("a", &url, &full_file_name, &dir);
+  ASSERT_TRUE(GetCurrentTab()->SavePage(full_file_name, dir,
                                     SavePackage::SAVE_AS_ONLY_HTML));

[Paweł Hajdan Jr., 2011/08/11 17:26:40]

nit: Isn't this line misaligned now?

[achuithb, 2011/08/11 23:21:59]

Done.

 
   EXPECT_EQ(url, WaitForSavePackageToFinish());
 
   CheckDownloadUI(full_file_name);
   CheckDownloadHistory(url, full_file_name, 1);  // a.htm is 1 file.
 
   EXPECT_TRUE(file_util::PathExists(full_file_name));
   EXPECT_FALSE(file_util::PathExists(dir));
   EXPECT_TRUE(file_util::ContentsEqual(
-      test_dir_.Append(FilePath(kTestDir)).Append(file_name),
+      test_dir_.Append(FilePath(kTestDir)).Append(FILE_PATH_LITERAL("a.htm")),
       full_file_name));
 }
 
 IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveViewSourceHTMLOnly) {
   FilePath file_name(FILE_PATH_LITERAL("a.htm"));
   GURL view_source_url = URLRequestMockHTTPJob::GetMockViewSourceUrl(
       FilePath(kTestDir).Append(file_name));
   GURL actual_page_url = URLRequestMockHTTPJob::GetMockUrl(
       FilePath(kTestDir).Append(file_name));
   ui_test_utils::NavigateToURL(browser(), view_source_url);
 
-  TabContents* current_tab = browser()->GetSelectedTabContents();
-  ASSERT_TRUE(current_tab);
-
   FilePath full_file_name = save_dir_.path().Append(file_name);
   FilePath dir = save_dir_.path().AppendASCII("a_files");
 
-  ASSERT_TRUE(current_tab->SavePage(full_file_name, dir,
+  ASSERT_TRUE(GetCurrentTab()->SavePage(full_file_name, dir,
                                     SavePackage::SAVE_AS_ONLY_HTML));
 
   EXPECT_EQ(actual_page_url, WaitForSavePackageToFinish());
 
   CheckDownloadUI(full_file_name);
   CheckDownloadHistory(actual_page_url, full_file_name, 1);  // a.htm is 1 file.
 
   EXPECT_TRUE(file_util::PathExists(full_file_name));
   EXPECT_FALSE(file_util::PathExists(dir));
   EXPECT_TRUE(file_util::ContentsEqual(
       test_dir_.Append(FilePath(kTestDir)).Append(file_name),
       full_file_name));
 }
 
 IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveCompleteHTML) {
-  FilePath file_name(FILE_PATH_LITERAL("b.htm"));
-  GURL url = URLRequestMockHTTPJob::GetMockUrl(
-      FilePath(kTestDir).Append(file_name));
-  ui_test_utils::NavigateToURL(browser(), url);
-
-  TabContents* current_tab = browser()->GetSelectedTabContents();
-  ASSERT_TRUE(current_tab);
-
-  FilePath full_file_name = save_dir_.path().Append(file_name);
-  FilePath dir = save_dir_.path().AppendASCII("b_files");
-  ASSERT_TRUE(current_tab->SavePage(full_file_name, dir,
+  GURL url;
+  FilePath full_file_name, dir;
+  GetPaths("b", &url, &full_file_name, &dir);
+  ASSERT_TRUE(GetCurrentTab()->SavePage(full_file_name, dir,
                                     SavePackage::SAVE_AS_COMPLETE_HTML));

[Paweł Hajdan Jr., 2011/08/11 17:26:40]

nit: Isn't this line misaligned now?

[achuithb, 2011/08/11 23:21:59]

Done.

 
   EXPECT_EQ(url, WaitForSavePackageToFinish());
 
   CheckDownloadUI(full_file_name);
   CheckDownloadHistory(url, full_file_name, 3);  // b.htm is 3 files.
 
   EXPECT_TRUE(file_util::PathExists(full_file_name));
   EXPECT_TRUE(file_util::PathExists(dir));
   EXPECT_TRUE(file_util::TextContentsEqual(
       test_dir_.Append(FilePath(kTestDir)).AppendASCII("b.saved1.htm"),
@@ skipping 17 matching lines @@
 
   GURL url = URLRequestMockHTTPJob::GetMockUrl(
       FilePath(kTestDir).Append(file_name));
   ui_test_utils::NavigateToURL(browser(), url);
 
   FilePath full_file_name = save_dir_.path().AppendASCII(
       std::string("Test page for saving page feature") + kAppendedExtension);
   FilePath dir = save_dir_.path().AppendASCII(
       "Test page for saving page feature_files");
 
-  TabContents* current_tab = browser()->GetSelectedTabContents();
-  ASSERT_TRUE(current_tab);
-
-  ASSERT_TRUE(current_tab->SavePage(full_file_name, dir,
+  ASSERT_TRUE(GetCurrentTab()->SavePage(full_file_name, dir,
                                     SavePackage::SAVE_AS_COMPLETE_HTML));

[Paweł Hajdan Jr., 2011/08/11 17:26:40]

nit: Isn't this line misaligned now?

[achuithb, 2011/08/11 23:21:59]

Done.

 
   EXPECT_EQ(url, WaitForSavePackageToFinish());
 
   CheckDownloadUI(full_file_name);
   CheckDownloadHistory(url, full_file_name, 3);  // b.htm is 3 files.
 
   EXPECT_TRUE(file_util::PathExists(full_file_name));
   EXPECT_TRUE(file_util::PathExists(dir));
   EXPECT_TRUE(file_util::TextContentsEqual(
       test_dir_.Append(FilePath(kTestDir)).AppendASCII("b.saved2.htm"),
       full_file_name));
   EXPECT_TRUE(file_util::ContentsEqual(
       test_dir_.Append(FilePath(kTestDir)).AppendASCII("1.png"),
       dir.AppendASCII("1.png")));
   EXPECT_TRUE(file_util::ContentsEqual(
       test_dir_.Append(FilePath(kTestDir)).AppendASCII("1.css"),
       dir.AppendASCII("1.css")));
 }
 
 IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, RemoveFromList) {
-  FilePath file_name(FILE_PATH_LITERAL("a.htm"));
-  GURL url = URLRequestMockHTTPJob::GetMockUrl(
-      FilePath(kTestDir).Append(file_name));
-  ui_test_utils::NavigateToURL(browser(), url);
-
-  TabContents* current_tab = browser()->GetSelectedTabContents();
-  ASSERT_TRUE(current_tab);
-
-  FilePath full_file_name = save_dir_.path().Append(file_name);
-  FilePath dir = save_dir_.path().AppendASCII("a_files");
-  ASSERT_TRUE(current_tab->SavePage(full_file_name, dir,
+  GURL url;
+  FilePath full_file_name, dir;
+  GetPaths("a", &url, &full_file_name, &dir);
+  ASSERT_TRUE(GetCurrentTab()->SavePage(full_file_name, dir,
                                     SavePackage::SAVE_AS_ONLY_HTML));

[Paweł Hajdan Jr., 2011/08/11 17:26:40]

nit: Isn't this line misaligned now?

[achuithb, 2011/08/11 23:21:59]

Done.

 
   EXPECT_EQ(url, WaitForSavePackageToFinish());
 
   CheckDownloadUI(full_file_name);
   CheckDownloadHistory(url, full_file_name, 1);  // a.htm is 1 file.
 
   EXPECT_EQ(GetDownloadManager()->RemoveAllDownloads(), 1);
 
 #if defined(OS_CHROMEOS)
   EXPECT_EQ(GetDownloads().size(), 0U);
 #endif
 
   // Should not be in history.
   QueryDownloadHistory();
   EXPECT_EQ(std::find_if(history_entries_.begin(), history_entries_.end(),
       DownloadHistoryInfoMatch(url, full_file_name, 1)),
       history_entries_.end());
 
   EXPECT_TRUE(file_util::PathExists(full_file_name));
   EXPECT_FALSE(file_util::PathExists(dir));
   EXPECT_TRUE(file_util::ContentsEqual(
-      test_dir_.Append(FilePath(kTestDir)).Append(file_name),
+      test_dir_.Append(FilePath(kTestDir)).Append(FILE_PATH_LITERAL("a.htm")),
       full_file_name));
 }
 
+IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, Cancel) {

[Paweł Hajdan Jr., 2011/08/11 17:26:40]

nit: Add comment what this is testing.

[achuithb, 2011/08/11 23:21:59]

Done.

+  GURL url;
+  FilePath full_file_name, dir;
+  GetPaths("a", &url, &full_file_name, &dir);
+  scoped_refptr<SavePackage> save_package(new SavePackage(GetCurrentTab(),
+      SavePackage::SAVE_AS_ONLY_HTML, full_file_name, dir));
 }
+
+IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, ExplicitCancel) {
+  GURL url;
+  FilePath full_file_name, dir;
+  GetPaths("a", &url, &full_file_name, &dir);
+  scoped_refptr<SavePackage> save_package(new SavePackage(GetCurrentTab(),
+      SavePackage::SAVE_AS_ONLY_HTML, full_file_name, dir));
+  save_package->Cancel(true);
+}
+
+} // namespace

[Paweł Hajdan Jr., 2011/08/11 17:26:40]

nit: Two spaces between } and //

[achuithb, 2011/08/11 23:21:59]

Done.