Add Cancel test for SavePackage.

chrome/browser/download/save_page_browsertest.cc

Index: chrome/browser/download/save_page_browsertest.cc
===================================================================
--- chrome/browser/download/save_page_browsertest.cc	(revision 96208)
+++ chrome/browser/download/save_page_browsertest.cc	(working copy)
@@ -44,6 +44,25 @@
     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,
@@ -152,17 +171,10 @@
 };
 
 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());
@@ -173,7 +185,7 @@
   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));
 }
 
@@ -185,13 +197,10 @@
       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());
@@ -207,17 +216,10 @@
 }
 
 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());
@@ -256,10 +258,7 @@
   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());
@@ -281,17 +280,10 @@
 }
 
 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());
@@ -314,8 +306,25 @@
   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.