Browser and unit test cases for patch submitted for Issue 23584 under

http://codereview.chromium.org/660264.

BUG=23584
TEST=covered by unit tests and browser tests

Original patch by Chamal De Silva <chamal.desilva@gmail.com> at
http://codereview.chromium.org/669262/show

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

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

4 files changed, 110 insertions, 29 deletions

diff --git a/chrome/browser/download/save_package.cc b/chrome/browser/download/save_package.cc
index 87b11a9..a549e9c 100644
--- a/chrome/browser/download/save_package.cc
+++ b/chrome/browser/download/save_package.cc
@@ -167,7 +167,7 @@ SavePackage::SavePackage(TabContents* web_content,
       tab_id_(web_content->GetRenderProcessHost()->id()),
       ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) {
   DCHECK(web_content);
-  const GURL& current_page_url = tab_contents_->GetURL();
+  const GURL& current_page_url = GetUrlToBeSaved();
   DCHECK(current_page_url.is_valid());
   page_url_ = current_page_url;
   DCHECK(save_type_ == SAVE_AS_ONLY_HTML ||
@@ -192,14 +192,7 @@ SavePackage::SavePackage(TabContents* tab_contents)
       tab_id_(tab_contents->GetRenderProcessHost()->id()),
       ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) {
 
-  // Instead of using tab_contents_.GetURL here, we use url()
-  // (which is the "real" url of the page)
-  // from the NavigationEntry because its reflects their origin
-  // rather than the displayed one (returned by GetURL) which may be
-  // different (like having "view-source:" on the front).
-  NavigationEntry* active_entry =
-          tab_contents_->controller().GetActiveEntry();
-  const GURL& current_page_url = active_entry->url();
+  const GURL& current_page_url = GetUrlToBeSaved();
   DCHECK(current_page_url.is_valid());
   page_url_ = current_page_url;
   InternalInit();
@@ -208,10 +201,11 @@ SavePackage::SavePackage(TabContents* tab_contents)
 // This is for testing use. Set |finished_| as true because we don't want
 // method Cancel to be be called in destructor in test mode.
 // We also don't call InternalInit().
-SavePackage::SavePackage(const FilePath& file_full_path,
+SavePackage::SavePackage(TabContents* tab_contents,
+                         const FilePath& file_full_path,
                          const FilePath& directory_full_path)
     : file_manager_(NULL),
-      tab_contents_(NULL),
+      tab_contents_(tab_contents),
       download_(NULL),
       saved_main_file_path_(file_full_path),
       saved_main_directory_path_(directory_full_path),
@@ -267,6 +261,18 @@ SavePackage::~SavePackage() {
     select_file_dialog_->ListenerDestroyed();
 }
 
+// Retrieves the URL to be saved from tab_contents_ variable.
+GURL SavePackage::GetUrlToBeSaved() {
+  // Instead of using tab_contents_.GetURL here, we use url()
+  // (which is the "real" url of the page)
+  // from the NavigationEntry because it reflects its' origin
+  // rather than the displayed one (returned by GetURL) which may be
+  // different (like having "view-source:" on the front).
+  NavigationEntry* active_entry =
+           tab_contents_->controller().GetActiveEntry();
+  return active_entry->url();
+}
+
 // Cancel all in progress request, might be called by user or internal error.
 void SavePackage::Cancel(bool user_action) {
   if (!canceled()) {
diff --git a/chrome/browser/download/save_package.h b/chrome/browser/download/save_package.h
index c41014b..67db2bd 100644
--- a/chrome/browser/download/save_package.h
+++ b/chrome/browser/download/save_package.h
@@ -198,7 +198,8 @@ class SavePackage : public base::RefCountedThreadSafe<SavePackage>,
   friend class base::RefCountedThreadSafe<SavePackage>;
 
   // For testing only.
-  SavePackage(const FilePath& file_full_path,
+  SavePackage(TabContents* tab_contents,
+              const FilePath& file_full_path,
               const FilePath& directory_full_path);
 
   ~SavePackage();
@@ -227,6 +228,10 @@ class SavePackage : public base::RefCountedThreadSafe<SavePackage>,
   SaveItem* LookupItemInProcessBySaveId(int32 save_id);
   void PutInProgressItemToSavedMap(SaveItem* save_item);
 
+  // Retrieves the URL to be saved from tab_contents_ variable.
+  GURL GetUrlToBeSaved();
+
+
   typedef base::hash_map<std::string, SaveItem*> SaveUrlItemMap;
   // in_progress_items_ is map of all saving job in in-progress state.
   SaveUrlItemMap in_progress_items_;
diff --git a/chrome/browser/download/save_package_unittest.cc b/chrome/browser/download/save_package_unittest.cc
index c082797..551a7d9 100644
--- a/chrome/browser/download/save_package_unittest.cc
+++ b/chrome/browser/download/save_package_unittest.cc
@@ -8,6 +8,9 @@
 #include "base/path_service.h"
 #include "base/string_util.h"
 #include "chrome/browser/download/save_package.h"
+#include "chrome/browser/net/url_request_mock_http_job.h"
+#include "chrome/browser/renderer_host/test/test_render_view_host.h"
+#include "googleurl/src/gurl.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 #define FPL FILE_PATH_LITERAL
@@ -53,25 +56,9 @@ bool HasOrdinalNumber(const FilePath::StringType& filename) {
 
 }  // namespace
 
-class SavePackageTest : public testing::Test {
+class SavePackageTest : public RenderViewHostTestHarness {
  public:
   SavePackageTest() {
-    FilePath test_dir;
-    PathService::Get(base::DIR_TEMP, &test_dir);
-
-    save_package_success_ = new SavePackage(
-        test_dir.AppendASCII("testfile" HTML_EXTENSION),
-        test_dir.AppendASCII("testfile_files"));
-
-    // We need to construct a path that is *almost* kMaxFilePathLength long
-    long_file_name.resize(kMaxFilePathLength + long_file_name.length());
-    while (long_file_name.length() < kMaxFilePathLength)
-      long_file_name += long_file_name;
-    long_file_name.resize(kMaxFilePathLength - 9 - test_dir.value().length());
-
-    save_package_fail_ = new SavePackage(
-        test_dir.AppendASCII(long_file_name + HTML_EXTENSION),
-        test_dir.AppendASCII(long_file_name + "_files"));
   }
 
   bool GetGeneratedFilename(bool need_success_generate_filename,
@@ -104,6 +91,34 @@ class SavePackageTest : public testing::Test {
                                                   contents_mime_type);
   }
 
+  GURL GetUrlToBeSaved() {
+    return save_package_success_->GetUrlToBeSaved();
+  }
+
+ protected:
+  virtual void SetUp() {
+    RenderViewHostTestHarness::SetUp();
+
+    // Do the initialization in SetUp so contents() is initialized by
+    // RenderViewHostTestHarness::SetUp.
+    FilePath test_dir;
+    PathService::Get(base::DIR_TEMP, &test_dir);
+
+    save_package_success_ = new SavePackage(contents(),
+        test_dir.AppendASCII("testfile" HTML_EXTENSION),
+        test_dir.AppendASCII("testfile_files"));
+
+    // We need to construct a path that is *almost* kMaxFilePathLength long
+    long_file_name.resize(kMaxFilePathLength + long_file_name.length());
+    while (long_file_name.length() < kMaxFilePathLength)
+      long_file_name += long_file_name;
+    long_file_name.resize(kMaxFilePathLength - 9 - test_dir.value().length());
+
+    save_package_fail_ = new SavePackage(contents(),
+        test_dir.AppendASCII(long_file_name + HTML_EXTENSION),
+        test_dir.AppendASCII(long_file_name + "_files"));
+  }
+
  private:
   // SavePackage for successfully generating file name.
   scoped_refptr<SavePackage> save_package_success_;
@@ -297,3 +312,29 @@ TEST_F(SavePackageTest, TestSuggestedSaveNames) {
   }
 }
 
+static const FilePath::CharType* kTestDir = FILE_PATH_LITERAL("save_page");
+
+// GetUrlToBeSaved method should return correct url to be saved.
+TEST_F(SavePackageTest, TestGetUrlToBeSaved) {
+  FilePath file_name(FILE_PATH_LITERAL("a.htm"));
+  GURL url = URLRequestMockHTTPJob::GetMockUrl(
+                 FilePath(kTestDir).Append(file_name));
+  NavigateAndCommit(url);
+  EXPECT_EQ(url, GetUrlToBeSaved());
+}
+
+// GetUrlToBeSaved method sould return actual url to be saved,
+// instead of the displayed url used to view source of a page.
+// Ex:GetUrlToBeSaved method should return http://www.google.com
+// when user types view-source:http://www.google.com
+TEST_F(SavePackageTest, TestGetUrlToBeSavedViewSource) {
+  FilePath file_name(FILE_PATH_LITERAL("a.htm"));
+  GURL view_source_url = URLRequestMockHTTPJob::GetMockViewSourceUrl(
+                             FilePath(kTestDir).Append(file_name));
+  GURL actual_url = URLRequestMockHTTPJob::GetMockUrl(
+                        FilePath(kTestDir).Append(file_name));
+  NavigateAndCommit(view_source_url);
+  EXPECT_EQ(actual_url, GetUrlToBeSaved());
+  EXPECT_EQ(view_source_url, contents()->GetURL());
+}
+
diff --git a/chrome/browser/download/save_page_browsertest.cc b/chrome/browser/download/save_page_browsertest.cc
index 799c6ac..99648b4 100644
--- a/chrome/browser/download/save_page_browsertest.cc
+++ b/chrome/browser/download/save_page_browsertest.cc
@@ -76,6 +76,35 @@ IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveHTMLOnly) {
       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,
+                                    SavePackage::SAVE_AS_ONLY_HTML));
+
+  EXPECT_EQ(actual_page_url, WaitForSavePackageToFinish());
+
+  if (browser()->SupportsWindowFeature(Browser::FEATURE_DOWNLOADSHELF))
+    EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
+
+  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(