[chromedriver] setting browser default download directory

chrome/test/chromedriver/chrome_launcher.cc

Index: chrome/test/chromedriver/chrome_launcher.cc
diff --git a/chrome/test/chromedriver/chrome_launcher.cc b/chrome/test/chromedriver/chrome_launcher.cc
index 21595b0c8b3fab8f83af37918ef29e7f2e2eb8b5..a3b551e67d102170cc3d7264404720949d9d608a 100644
--- a/chrome/test/chromedriver/chrome_launcher.cc
+++ b/chrome/test/chromedriver/chrome_launcher.cc
@@ -128,24 +128,28 @@ Status PrepareCommandLine(int port,
     switches.RemoveSwitch(*iter);
   }
   switches.SetFromSwitches(capabilities.switches);
-
+  base::FilePath userDataDirPath;

[samuong, 2014/10/15 22:08:17]

For local variable names in C++, we use underscores rather than camelCase (see
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Variable_Names).
Please try to stay as consistent as you can with existing code.

   if (!switches.HasSwitch("user-data-dir")) {
-    command.AppendArg("data:,");
     if (!user_data_dir->CreateUniqueTempDir())
       return Status(kUnknownError, "cannot create temp dir for user data dir");
     switches.SetSwitch("user-data-dir", user_data_dir->path().value());
-    Status status = internal::PrepareUserDataDir(
-        user_data_dir->path(), capabilities.prefs.get(),
-        capabilities.local_state.get());
-    if (status.IsError())
-      return status;
+    userDataDirPath = user_data_dir->path();
+  } else {
+    userDataDirPath = base::FilePath(switches.GetSwitchValue("user-data-dir"));
   }
-
+  //

[samuong, 2014/10/15 22:08:17]

More style nitpicking: don't use empty comments like this, just use blank lines,
and stay consistent with existing code.

+  command.AppendArg("data:,");

[samuong, 2014/10/15 22:08:17]

Existing user data directories might have a home page set, and the test could
expect chrome to open this home page when it launches. So we should only add
"data:," to the command line if there is no user-data-dir specified.

+  Status status = internal::PrepareUserDataDir(
+                                  userDataDirPath  , capabilities.prefs.get(),
+                                  capabilities.local_state.get());
+  if (status.IsError())
+      return status;
+  //
   if (!extension_dir->CreateUniqueTempDir()) {
     return Status(kUnknownError,
                   "cannot create temp dir for unpacking extensions");
   }
-  Status status = internal::ProcessExtensions(capabilities.extensions,
+  status = internal::ProcessExtensions(capabilities.extensions,
                                               extension_dir->path(),
                                               true,
                                               &switches,
@@ -762,7 +766,43 @@ Status WritePrefsFile(
   }
   return Status(kOk);
 }
+//
+Status mergeExistPrefFile(const base::DictionaryValue* custom_prefs,

[samuong, 2014/10/15 22:08:17]

For function names in C++ like this, we use CamelCase and start the function
name with a capital letter (see
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Names).

+        const base::FilePath& path, base::DictionaryValue* mergePrefs) {
+
+    if (base::PathExists(path)) { // if exist, import it
+        std::string data_read;
+        base::ReadFileToString(path, &data_read);
+        //
+        int code;
+        std::string error_msg;
+        scoped_ptr<base::Value> template_value(
+                base::JSONReader::ReadAndReturnError(data_read, 0, &code,
+                        &error_msg));
+
+        base::DictionaryValue* prefs = NULL;
+        if (!template_value || !template_value->GetAsDictionary(&prefs)) {
+            return Status(kUnknownError,
+                    "cannot parse default Preference: " + error_msg);
+        }
+        mergePrefs->MergeDictionary(prefs);
+        // overwrite if any custom prefs
+        if (custom_prefs) {
+            for (base::DictionaryValue::Iterator it(*custom_prefs);
+                    !it.IsAtEnd(); it.Advance()) {
+                mergePrefs->Set(it.key(), it.value().DeepCopy());
+            }
+        }
+
+    } else { // nothing to  read - copy user define data directly
+        if (custom_prefs)
+            mergePrefs->MergeDictionary(custom_prefs);
+    }
+
+    return Status(kOk);
+}
 
+//
 Status PrepareUserDataDir(
     const base::FilePath& user_data_dir,
     const base::DictionaryValue* custom_prefs,
@@ -771,13 +811,21 @@ Status PrepareUserDataDir(
       user_data_dir.AppendASCII(chrome::kInitialProfile);
   if (!base::CreateDirectory(default_dir))
     return Status(kUnknownError, "cannot create default profile directory");
-
-  Status status =
+  //
+  base::DictionaryValue mergePrefs;
+  // merge to existed Preferences
+  Status status = mergeExistPrefFile(custom_prefs,
+          default_dir.Append(chrome::kPreferencesFilename),
+          &mergePrefs);
+  if (status.IsError())
+      return status;
+  // merge to template
+  status =
       WritePrefsFile(kPreferences,
-                     custom_prefs,
+                     &mergePrefs,
                      default_dir.Append(chrome::kPreferencesFilename));
   if (status.IsError())
-    return status;
+      return status;

[samuong, 2014/10/15 22:08:17]

C++ code should be indented with two spaces, not four (see
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Spaces_vs._Tabs).
Please make sure you stay consistent with the existing coding style.

 
   status = WritePrefsFile(kLocalState,
                           custom_local_state,