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

chrome/browser/ui/download/download_tab_helper.h

 // 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.
 
 #ifndef CHROME_BROWSER_UI_DOWNLOAD_DOWNLOAD_TAB_HELPER_H_
 #define CHROME_BROWSER_UI_DOWNLOAD_DOWNLOAD_TAB_HELPER_H_
 #pragma once
 
 #include "base/basictypes.h"
 #include "chrome/browser/download/save_package.h"
@@ skipping 19 matching lines @@
   // Prepare for saving the URL to disk.
   // URL may refer to the iframe on the page.
   void OnSaveURL(const GURL& url);
 
   // Save page with the main HTML file path, the directory for saving resources,
   // and the save type: HTML only or complete web page. Returns true if the
   // saving process has been initiated successfully.
   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(); }
 
   // Notifies the delegate that a download is about to be started.
   // This notification is fired before a local temporary file has been created.
   bool CanDownload(int request_id);
 
   // Notifies the delegate that a download started.
   void OnStartDownload(DownloadItem* download);
 
  private:
   // TabContentsObserver overrides.
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
   virtual void DidGetUserGesture() OVERRIDE;
 
   // SavePackage, lazily created.
   scoped_refptr<SavePackage> save_package_;
 
   // Owning TabContentsWrapper.
   TabContentsWrapper* tab_contents_wrapper_;
 
   // Delegate for notifying our owner (usually Browser) about stuff. Not owned
   // by us.
   DownloadTabHelperDelegate* delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadTabHelper);
 };
 
 #endif  // CHROME_BROWSER_UI_DOWNLOAD_DOWNLOAD_TAB_HELPER_H_