Add usb_detachable_whitelist to device policy proto

chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc

 // Copyright (c) 2012 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/chromeos/policy/device_policy_decoder_chromeos.h"
 
 #include <limits>
 #include <string>
 
 #include "base/callback.h"
 #include "base/json/json_reader.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/strings/stringprintf.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/policy/device_local_account.h"
 #include "chrome/browser/chromeos/policy/proto/chrome_device_policy.pb.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/update_engine_client.h"
 #include "chromeos/settings/cros_settings_names.h"
 #include "components/policy/core/browser/browser_policy_connector.h"
 #include "components/policy/core/common/external_data_fetcher.h"
 #include "components/policy/core/common/policy_map.h"
@@ skipping 791 matching lines @@
     const em::DisplayRotationDefaultProto& container(
         policy.display_rotation_default());
     policies->Set(
         key::kDisplayRotationDefault,
         POLICY_LEVEL_MANDATORY,
         POLICY_SCOPE_MACHINE,
         POLICY_SOURCE_CLOUD,
         DecodeIntegerValue(container.display_rotation_default()).release(),
         nullptr);
   }
+
+  if (policy.has_usb_detachable_whitelist()) {
+    const em::UsbDetachableWhitelistProto& container(
+        policy.usb_detachable_whitelist());
+    base::ListValue* whitelist = new base::ListValue();
+    RepeatedPtrField<em::UsbDeviceIdProto>::const_iterator entry;
+    for (entry = container.id().begin(); entry != container.id().end();
+         ++entry) {
+      uint16_t vid = entry->has_vendor_id() ? entry->vendor_id() : 0x0000;

[bartfab (slow), 2016/03/01 14:49:57]

Nit: const.

[vpalatin, 2016/03/01 17:49:01]

Done.

+      uint16_t pid = entry->has_product_id() ? entry->product_id() : 0x0000;

[bartfab (slow), 2016/03/01 14:49:57]

Nit: const.

[vpalatin, 2016/03/01 17:49:01]

Done.

+      whitelist->Append(
+          new base::StringValue(base::StringPrintf("%04x:%04x", vid, pid)));

[bartfab (slow), 2016/03/01 14:49:57]

Why not use a base::ListValue of two base::FundamentalValues here instead? It
would avoid the (potentially lossy, if the value is out of range) conversion
from int to 4-digit string.

[vpalatin, 2016/03/01 17:49:01]

That would look nicer but I really want to print this as a 'standard' VID:PID
pair as other Linux tool (e.g. lsusb),
if we get a set of integers in base 10, it's somewhat unusable without doing the
conversion manually.

[bartfab (slow), 2016/03/02 10:40:33]

I think there is a compromise that can make both of us happy. The issue I have
is that converting to |uint16_t| hides errors if the value is outside the
expected range. I suggest two changes:

1) Switch |vid| and |pid| from |uint16_t| to |int32_t|. If the values are in the
expected range, this will have no effect - StringPrintf() will still produce
4-digit output. But if the values are invalid (e.g. more than four digits or
negative), we will be able to see it in the StringPrintf output.

2) Rather than combining |vid| and |pid| into one string, StringPrintf() them
into separate strings. This way, each piece of information on the
chrome://policy page maps directly to a policy proto field. This will make the
information in chrome://policy slightly harder to consume (you have to
concatenate the |vid| and |pid| in your head) but considering they are printed
as hex digits with correct padding already, that should not be a big burden.

+    }
+    policies->Set(key::kUsbDetachableWhitelist, POLICY_LEVEL_MANDATORY,
+                  POLICY_SCOPE_MACHINE, POLICY_SOURCE_CLOUD, whitelist, NULL);

[bartfab (slow), 2016/03/01 14:49:57]

Nit: s/NULL/nullptr/

[vpalatin, 2016/03/01 17:49:01]

Done.

+  }
 }
 
 }  // namespace
 
 void DecodeDevicePolicy(const em::ChromeDeviceSettingsProto& policy,
                         PolicyMap* policies) {
   // Decode the various groups of policies.
   DecodeLoginPolicies(policy, policies);
   DecodeNetworkPolicies(policy, policies);
   DecodeReportingPolicies(policy, policies);
   DecodeAutoUpdatePolicies(policy, policies);
   DecodeAccessibilityPolicies(policy, policies);
   DecodeGenericPolicies(policy, policies);
 }
 
 }  // namespace policy