Set drive as the default download folder

chrome/browser/download/download_dir_policy_handler.cc

Index: chrome/browser/download/download_dir_policy_handler.cc
diff --git a/chrome/browser/download/download_dir_policy_handler.cc b/chrome/browser/download/download_dir_policy_handler.cc
index 3f79ff513a4022250245425044f9ed9a89c45faa..0422e6a3589a664f8e079250bf3dd2bd1b7ffb1f 100644
--- a/chrome/browser/download/download_dir_policy_handler.cc
+++ b/chrome/browser/download/download_dir_policy_handler.cc
@@ -11,25 +11,78 @@
 #include "chrome/browser/download/download_prefs.h"
 #include "chrome/browser/policy/policy_path_parser.h"
 #include "chrome/common/pref_names.h"
+#include "components/policy/core/browser/configuration_policy_handler_parameters.h"
+#include "components/policy/core/browser/policy_error_map.h"
 #include "components/policy/core/common/policy_map.h"
+#include "components/policy/core/common/policy_types.h"
+#include "grit/component_strings.h"
 #include "policy/policy_constants.h"
 
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/chromeos/drive/file_system_util.h"
+#endif
+
+namespace {
+#if defined(OS_CHROMEOS)
+const char* kDriveNamePolicyVariableName = "${google_drive}";
+
+// The relative path of the drive download folder with respect to the path

[bartfab (slow), 2014/03/14 13:43:45]

Is this really the path to the *download* folder? Is it not the path to the root
of your drive?

+// returned by GetDriveMountPointPathUsingUserHash.
+const base::FilePath::CharType* kPathRelativeToDriveRoot =
+    FILE_PATH_LITERAL("/root/");
+#endif
+}  // namespace
+
 DownloadDirPolicyHandler::DownloadDirPolicyHandler()
     : TypeCheckingPolicyHandler(policy::key::kDownloadDirectory,
                                 base::Value::TYPE_STRING) {}
 
 DownloadDirPolicyHandler::~DownloadDirPolicyHandler() {}
 
-void DownloadDirPolicyHandler::ApplyPolicySettings(
+bool DownloadDirPolicyHandler::CheckPolicySettings(
+    const policy::PolicyMap& policies,
+    policy::PolicyErrorMap* errors) {
+  const base::Value* value = NULL;
+  if (!CheckAndGetValue(policies, errors, &value))
+    return false;
+
+#if defined(OS_CHROMEOS)
+  // On chromeos, setting Download directory is supported for the current user,

[bartfab (slow), 2014/03/14 13:43:45]

Nit 1: s/chromeos/Chrome OS/
Nit 2: s/Download/the download/
3: You are right that this cannot be set for the whole device. But that won't
happen on Chrome OS anyway because you cannot set the download directory through
device policy. What the check here really prevents is someone setting the
download directory via *platform policy*, which happens to be picked up by
chromeos=1 builds on Linux.

+  // but not supported for the whole device.
+  if (value &&
+      policies.Get(policy_name())->scope != policy::POLICY_SCOPE_USER) {
+    errors->AddError(policy_name(), IDS_POLICY_SCOPE_ERROR);
+    return false;
+  }
+#endif
+
+  return true;
+}
+
+void DownloadDirPolicyHandler::ApplyPolicySettingsWithParameters(
     const policy::PolicyMap& policies,
+    const policy::PolicyHandlerParameters& parameters,
     PrefValueMap* prefs) {
   const base::Value* value = policies.GetValue(policy_name());
   base::FilePath::StringType string_value;
   if (!value || !value->GetAsString(&string_value))
     return;
 
-  base::FilePath::StringType expanded_value =
-      policy::path_parser::ExpandPathVariables(string_value);
+  base::FilePath::StringType expanded_value;
+#if defined(OS_CHROMEOS)

[bartfab (slow), 2014/03/14 13:43:45]

Nit: File a bug and add a TODO to fold this Chrome OS-specific code into
policy::path_parser.

+  size_t position = string_value.find(kDriveNamePolicyVariableName);
+  if (position != std::string::npos) {

[bartfab (slow), 2014/03/14 13:43:45]

Nit: s/std::string/base::FilePath::StringType/

+    base::FilePath::StringType drive_mount;

[bartfab (slow), 2014/03/14 13:43:45]

Nit: What you store in this variable is not the path to the mounting point but
the path to the root directory of the mounted drive. How about
s/drive_mount/google_drive_root/?

+    if (!parameters.user_id_hash.empty())

[bartfab (slow), 2014/03/14 13:43:45]

Nit: Multi-line conditionals require curly braces.

+      drive_mount = drive::util::GetDriveMountPointPathUsingUserHash(
+                        parameters.user_id_hash).value() +
+                    kPathRelativeToDriveRoot;
+    expanded_value = string_value.replace(
+        position, strlen(kDriveNamePolicyVariableName), drive_mount);

[bartfab (slow), 2014/03/14 13:43:45]

Once you go for base::FilePath::CharType (which can be an 8-bit or 16-bit
character, depending on platform), you cannot use strlen(), which handles 8-bit
characters only.

+  }
+#else
+  expanded_value = policy::path_parser::ExpandPathVariables(string_value);
+#endif
   // Make sure the path isn't empty, since that will point to an undefined
   // location; the default location is used instead in that case.
   // This is checked after path expansion because a non-empty policy value can
@@ -38,6 +91,8 @@ void DownloadDirPolicyHandler::ApplyPolicySettings(
     expanded_value = DownloadPrefs::GetDefaultDownloadDirectory().value();
   prefs->SetValue(prefs::kDownloadDefaultDirectory,
                   base::Value::CreateStringValue(expanded_value));
+#if !defined(OS_CHROMEOS)

[bartfab (slow), 2014/03/14 13:43:45]

The policy is documented as:

"If this policy is left not set the default download directory will be used and
the user will be able to change it."

With this #ifdef, that is not true on Chrome OS.

   prefs->SetValue(prefs::kPromptForDownload,
                   base::Value::CreateBooleanValue(false));
+#endif
 }