When the download folder does not exist, change the download folder to a user's "Downloads"

chrome/browser/ui/download/download_tab_helper.h

Index: chrome/browser/ui/download/download_tab_helper.h
diff --git a/chrome/browser/ui/download/download_tab_helper.h b/chrome/browser/ui/download/download_tab_helper.h
index ac559136eb7b74d2056acbd2bdd50d8c95cb87db..7f3f7e0c86e00852510dac9219a66d78dead8109 100644
--- a/chrome/browser/ui/download/download_tab_helper.h
+++ b/chrome/browser/ui/download/download_tab_helper.h
@@ -37,6 +37,14 @@ class DownloadTabHelper : public TabContentsObserver {
   bool SavePage(const FilePath& main_file, const FilePath& dir_path,
                 SavePackage::SavePackageType save_type);
 
+  // Save page. |website_save_dir| is the default folder for saving HTML.
+  // |download_save_dir| is the default folder for saving downloaded files.
+  // |save_type| is HTML only or complete web page.
+  // Returns the title of saved page.
+  string16 SavePageToProperDirectory(const FilePath& website_save_dir,
+                                      const FilePath& download_save_dir,
+                                      SavePackage::SavePackageType save_type);

[Randy Smith (Not in Mondays), 2011/05/31 23:03:10]

In general, I feel better about methods that are only for use by tests being
private, with an explicit FRIEND_TEST_ALL_PREFIXES() to allow access by that one
test.

[haraken1, 2011/06/02 09:13:22]

If we simply make these methods private and add FRIEND_TEST_ALL_PREFIXES(), then
we cannot compile save_page_browsertest.cc. because SavePageTest.* belongs to
empty namespace: 

In save_page_browsertest.cc:

namespace {
  IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveToFolderForHTMLPage) {
   ...;
  }
  IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveToFolderForDownloadedFiles) {
   ...;
  }

}

[haraken1, 2011/06/02 09:13:22]

If we simply make these methods private and add FRIEND_TEST_ALL_PREFIXES(), then
we cannot compile save_page_browsertest.cc because SavePageTest.* belongs to an
empty namespace. 

save_page_browsertest.cc:
namespace {
  IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveToFolderForHTMLPage) {
   ...;
  }
  IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveToFolderForDownloadedFiles) {
   ...;
  }
  IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveToUsersDownloadsFolder) {
   ...;
  }
}

Thus, we cannot specify the above tests simply by writing
FRIEND_TEST_ALL_PREFIXES() as follows. 

download_tab_helper.h:
class DownloadTabHelper : public TabContentsObserver {
  private:
   FRIEND_TEST_ALL_PREFIXES(SavePageBrowserTest, SaveToFolderForHTMLPage);
   FRIEND_TEST_ALL_PREFIXES(SavePageBrowserTest,
SaveToFolderForDownloadedFiles);
   FRIEND_TEST_ALL_PREFIXES(SavePageBrowserTest, SaveToUsersDownloadsFolder);
  ...;
}

The first possible approach is to leave these methods public as I write. The
second approach is to eliminate the empty namespace (namespace { ... }) from
save_page_browsertest.cc. The third approach is to name the namespace of
save_page_browsertest.cc (like namespace savepage { ... }) and use the namespace
to specify the tests (like
FRIEND_TEST_ALL_PREFIXES(savepage::SavePageBrowserTest,
SaveToFolderForHTMLPage)). 

Which one would be best?

[Randy Smith (Not in Mondays), 2011/06/02 19:13:57]

What I've always done is move the tests (but not any helper code) out of the
empty namespace.  If there's no helper code, that may mean eliminating the
namespace completely, but more often it means ending the scope early.  See
download_browsertest.cc for an example.

[haraken1, 2011/06/03 06:50:26]

Done.

+
   // Returns the SavePackage which manages the page saving job. May be NULL.
   SavePackage* save_package() const { return save_package_.get(); }