Submitting for developer0420@gmail.com

The LPITEMIDLIST returned from SHBrowseForFolder might not get released if
SHGetPathFromIDList fails

Added in the comment changes

Added GetDisplayNameOf for getting selection, using old way if 
STRRET Structure type does not equal STRRET_WSTR as I was
unable to get complete code coverage.

Added BrowseCallbackProc and listen to MSG so
that we can highlight existing value

Added in the comment changes ( changed to smart pointer for managing
COM interface pointers.)

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

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

2 files changed, 53 insertions, 7 deletions

diff --git a/chrome/browser/views/options/content_page_view.cc b/chrome/browser/views/options/content_page_view.cc
index 665b14b..acad68d 100644
--- a/chrome/browser/views/options/content_page_view.cc
+++ b/chrome/browser/views/options/content_page_view.cc
@@ -196,9 +196,12 @@ void ContentPageView::ButtonPressed(views::NativeButton* sender) {
     const std::wstring dialog_title =
        l10n_util::GetString(IDS_OPTIONS_DOWNLOADLOCATION_BROWSE_TITLE);
     select_file_dialog_->SelectFile(SelectFileDialog::SELECT_FOLDER,
-                                    dialog_title, std::wstring(),
+                                    dialog_title,
+                                    profile()->GetPrefs()->GetString(
+                                        prefs::kDownloadDefaultDirectory),
                                     std::wstring(), std::wstring(),
-                                    GetRootWindow(), NULL);
+                                    GetRootWindow(),
+                                    NULL);
   } else if (sender == download_ask_for_save_location_checkbox_) {
     bool enabled = download_ask_for_save_location_checkbox_->IsSelected();
     if (enabled) {
diff --git a/chrome/browser/views/shell_dialogs.cc b/chrome/browser/views/shell_dialogs.cc
index 31cbaa5b..a9e8c53 100644
--- a/chrome/browser/views/shell_dialogs.cc
+++ b/chrome/browser/views/shell_dialogs.cc
@@ -7,6 +7,7 @@
 #include <windows.h>
 #include <Commdlg.h>
 #include <shlobj.h>
+#include <atlbase.h>
 
 #include <algorithm>
 #include <map>
@@ -234,6 +235,11 @@ class SelectFileDialogImpl : public SelectFileDialog,
                          HWND owner,
                          std::wstring* path);
 
+  // The callback function for when the select folder dialog is opened.
+  static int CALLBACK BrowseCallbackProc(HWND window, UINT message,
+                                         LPARAM parameter,
+                                         LPARAM data);
+
   // The listener to be notified of selection completion.
   Listener* listener_;
 
@@ -317,6 +323,19 @@ void SelectFileDialogImpl::FileNotSelected(void* params, RunState run_state) {
   EndRun(run_state);
 }
 
+int CALLBACK SelectFileDialogImpl::BrowseCallbackProc(HWND window,
+                                                      UINT message,
+                                                      LPARAM parameter,
+                                                      LPARAM data)
+{
+  if (message == BFFM_INITIALIZED) {
+    // WParam is TRUE since passing a path.
+    // data lParam member of the BROWSEINFO structure.
+    SendMessage(window, BFFM_SETSELECTION, TRUE, (LPARAM)data);
+  }
+  return 0;
+}
+
 bool SelectFileDialogImpl::RunSelectFolderDialog(const std::wstring& title,
                                                  HWND owner,
                                                  std::wstring* path) {
@@ -324,26 +343,50 @@ bool SelectFileDialogImpl::RunSelectFolderDialog(const std::wstring& title,
 
   wchar_t dir_buffer[MAX_PATH + 1];
 
+  bool result = false;
   BROWSEINFO browse_info = {0};
   browse_info.hwndOwner = owner;
   browse_info.lpszTitle = title.c_str();
   browse_info.pszDisplayName = dir_buffer;
   browse_info.ulFlags = BIF_USENEWUI | BIF_RETURNONLYFSDIRS;
+  
+  if (path->length()) {
+    // Highlight the current value.
+    browse_info.lParam = (LPARAM)path->c_str();
+    browse_info.lpfn = &BrowseCallbackProc;
+  }
+  
   LPITEMIDLIST list = SHBrowseForFolder(&browse_info);
   DisableOwner(owner);
   if (list) {
-    wchar_t out_dir_buffer[MAX_PATH + 1];
-    if (SHGetPathFromIDList(list, out_dir_buffer)) {
-      *path = out_dir_buffer;
+    STRRET out_dir_buffer;
+    ZeroMemory(&out_dir_buffer, sizeof(out_dir_buffer));
+    out_dir_buffer.uType = STRRET_WSTR;
+    CComPtr<IShellFolder> shell_folder = NULL;
+    if (SHGetDesktopFolder (&shell_folder) == NOERROR) {
+      HRESULT hr = shell_folder->GetDisplayNameOf(list, SHGDN_FORPARSING,
+                                                 &out_dir_buffer);
+      if (SUCCEEDED(hr) && out_dir_buffer.uType == STRRET_WSTR) {
+        *path = out_dir_buffer.pOleStr;
+        CoTaskMemFree(out_dir_buffer.pOleStr);
+        result = true;
+      }
+      else {
+        // Use old way if we don't get what we want.
+        wchar_t old_out_dir_buffer[MAX_PATH + 1];
+        if (SHGetPathFromIDList(list, old_out_dir_buffer)) {
+          *path = old_out_dir_buffer;
+          result = true;
+        }
+      }
 
       // According to MSDN, win2000 will not resolve shortcuts, so we do it
       // ourself.
       file_util::ResolveShortcut(path);
-      return true;
     }
     CoTaskMemFree(list);
   }
-  return false;
+  return result;
 }
 
 bool SelectFileDialogImpl::RunOpenFileDialog(