Add usb_detachable_whitelist to device policy proto

chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc

Index: chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc
diff --git a/chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc b/chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc
index 2579fe748b014c7560fe8d9a68fa4891cbedb68f..31efd2679e973c7a80149e81cc2a14a8751b34f6 100644
--- a/chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc
+++ b/chrome/browser/chromeos/policy/device_policy_decoder_chromeos.cc
@@ -11,6 +11,7 @@
 #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"
@@ -822,6 +823,22 @@ void DecodeGenericPolicies(const em::ChromeDeviceSettingsProto& policy,
         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