Set drive as the default download folder

chrome/browser/download/download_dir_policy_handler.cc

 // Copyright 2013 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.
 
 #include "chrome/browser/download/download_dir_policy_handler.h"
 
 #include "base/files/file_path.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/prefs/pref_value_map.h"
 #include "base/values.h"
 #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
   // lead to an empty path value after expansion (e.g. "\"\"").
   if (expanded_value.empty())
     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
 }