Replace "bluetooth" permission with manifest property.

chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.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/common/extensions/api/bluetooth/bluetooth_manifest_permission.h"
+
+#include "base/memory/scoped_ptr.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/values.h"
+#include "chrome/common/extensions/api/bluetooth/bluetooth_manifest_data.h"
+#include "chrome/common/extensions/api/manifest_types.h"
+#include "chrome/common/extensions/extension_messages.h"
+#include "device/bluetooth/bluetooth_utils.h"
+#include "extensions/common/error_utils.h"
+#include "extensions/common/manifest_constants.h"
+#include "grit/generated_resources.h"
+#include "ipc/ipc_message.h"
+#include "ui/base/l10n/l10n_util.h"
+
+namespace extensions {
+
+namespace bluetooth_errors {
+const char kErrorInvalidProfileUuid[] = "Invalid uuid '*'";

[not at google - send to devlin, 2014/01/23 20:29:12]

UUID

[rpaquay, 2014/01/24 16:58:23]

Done.

+}
+
+namespace errors = bluetooth_errors;
+
+namespace  {
+
+bool ParseUuid(BluetoothManifestPermission* permission,
+               const std::string& profile_uuid,
+               base::string16* error) {
+  std::string canonical_uuid =
+      device::bluetooth_utils::CanonicalUuid(profile_uuid);
+  if (canonical_uuid.empty()) {
+    *error = ErrorUtils::FormatErrorMessageUTF16(
+        errors::kErrorInvalidProfileUuid, profile_uuid);
+    return false;
+  }
+  permission->AddPermission(profile_uuid);
+  return true;
+}
+
+bool ParseUuidArray(BluetoothManifestPermission* permission,
+                    const scoped_ptr<std::vector<std::string> >& profiles,
+                    base::string16* error) {
+  for (std::vector<std::string>::const_iterator it =
+      profiles->begin(); it != profiles->end(); ++it) {
+    if (!ParseUuid(permission, *it, error)) {
+      return false;
+    }
+  }
+  return true;
+}
+
+void SetProfileUuids(std::vector<std::string>& uuids,
+                     BluetoothManifestPermission* permission) {
+  for (BluetoothManifestPermission::BluetoothProfileUuidSet::const_iterator it =
+       permission->profile_uuids().begin();
+       it != permission->profile_uuids().end() ; ++it) {
+    uuids.push_back(*it);
+  }
+}
+
+}  // namespace
+
+BluetoothManifestPermission::BluetoothManifestPermission() {}
+
+BluetoothManifestPermission::~BluetoothManifestPermission() {}
+
+// static
+scoped_ptr<BluetoothManifestPermission> BluetoothManifestPermission::FromValue(
+    const base::Value& value,
+    base::string16* error) {
+  scoped_ptr<api::manifest_types::Bluetooth> bluetooth =
+      api::manifest_types::Bluetooth::FromValue(value, error);
+  if (!bluetooth)
+    return scoped_ptr<BluetoothManifestPermission>();
+
+  scoped_ptr<BluetoothManifestPermission> result(
+      new BluetoothManifestPermission());
+  if (bluetooth->profiles) {
+    if (!ParseUuidArray(result.get(),
+                        bluetooth->profiles,
+                        error)) {
+      return scoped_ptr<BluetoothManifestPermission>();
+    }
+  }
+  return result.Pass();
+}
+
+bool BluetoothManifestPermission::CheckRequest(
+    const Extension* extension,
+    const BluetoothPermissionRequest& request) const {
+
+  std::string canonical_param_uuid = device::bluetooth_utils::CanonicalUuid(
+      request.profile_uuid);
+  for (BluetoothProfileUuidSet::const_iterator it = profile_uuids_.begin();
+       it != profile_uuids_.end(); ++it) {
+

[not at google - send to devlin, 2014/01/23 20:29:12]

no blank line

[rpaquay, 2014/01/24 16:58:23]

Done.

+    std::string canonical_uuid = device::bluetooth_utils::CanonicalUuid(*it);
+    if (canonical_uuid == canonical_param_uuid)
+      return true;
+  }
+  return false;
+}
+
+std::string BluetoothManifestPermission::name() const {
+  return manifest_keys::kBluetooth;
+}
+
+std::string BluetoothManifestPermission::id() const {
+  return name();
+}
+
+bool BluetoothManifestPermission::HasMessages() const {
+  return true;
+}
+
+PermissionMessages BluetoothManifestPermission::GetMessages() const {
+  DCHECK(HasMessages());
+  PermissionMessages result;
+
+  result.push_back(PermissionMessage(
+        PermissionMessage::kBluetooth,
+        l10n_util::GetStringUTF16(
+            IDS_EXTENSION_PROMPT_WARNING_BLUETOOTH)));

[not at google - send to devlin, 2014/01/24 21:59:38]

Ok if not for scanning what is this even for..? If it's just for the ability to
request the bluetooth devices that are already listed... it seems totally
redundant.

+
+  if (!profile_uuids_.empty()) {
+    result.push_back(PermissionMessage(
+          PermissionMessage::kBluetoothDevices,

[meacer, 2014/01/24 22:22:20]

Corresponding permission in USB API is singular: kUsbDevice. You might want to
match that.

+          l10n_util::GetStringUTF16(
+              IDS_EXTENSION_PROMPT_WARNING_BLUETOOTH_DEVICES)));
+  }

[not at google - send to devlin, 2014/01/23 20:29:12]

We should run this by security. Showing two warnings here is a bit unnecessary
given how similar they are:

IDS_EXTENSION_PROMPT_WARNING_BLUETOOTH:
Access information about Bluetooth devices paired with your system.

IDS_EXTENSION_PROMPT_WARNING_BLUETOOTH_DEVICES:
Access Bluetooth devices paired with your system.

I think I understand the difference when reading it, but in most cases it seems
like permission spam.

Is there a more fine-grained permission scheme we can use? Like, "bluetooth"
should have a "uuid" field and a "scan": true field as well? Then the former
would be "Discover new Bluetooth devices". Then again... that's still much these
same.

Or... have 3 difference messages.
1. Scan for bluetooth devices
2. Scan and pair with bluetooth devices.
3. Pair with bluetooth devices.

If those are the differences.

Oh and another question - is it possible to actually be more accurate than
"access bluetooth devices"? We basically know what the devices are ahead of time
because they're whitelisted.

+meacer

[meacer, 2014/01/23 22:25:14]

Let's be consistent with the USB API. It has a generic warning ("Access USB
devices") and a device specific ("Access USB device <Device Name>") warning with
the device name. The usage in chrome-app-samples is that the generic warning is
requested at install time and the device warning is requested during runtime.

That said, I think install time warnings can be merged as Ben suggests (for both
Bluetooth and USB). If both bluetooth and bluetoothDevice permissions is
requested, can we show something like "Access USB Devices (<device name>)"?

[rpaquay, 2014/01/24 00:05:39]

+keybuk: Scott, a couple of questions:
1) Do you think it would be reasonable to add a "device name" in the bluetooth
permission warning dialog? It is not clear to this can be done, as the uuid
prodived in the manifest file correspond to a profile the app registers.
2) Do you think it would make send to add additional permissions to bluetooth,
such as "scan for devices" (or something else). The reason we are asking is
because when we change "bluetooth" from a permission to a manifest entry, the
text in manifest.json looks weird by default:
instead of
  "permissions": ["bluetooth"]
we now have
  "bluetooth": {}
It would be nicer to have something like
    "bluetooth": {"scan": true}
and it would look like this when adding profiles uuid:
    "bluetooth": {"scan": true, "profiles": ["1234", "456"]}

[not at google - send to devlin, 2014/01/24 21:59:38]

Mustafa, what you say makes sense (install vs runtime permissions), in which
case they should be treated separately, but that's not what the code looks like
here, it looks like they're all install time?

[meacer, 2014/01/24 22:22:20]

The app can still choose to request bluetoothDevices at runtime though, right?
It's just not mandatory to do so. For USB, you had to request usbDevice during
runtime (via chrome.permissions.request), but apparently you can now request it
during install time too. So my suggestions is this:

bluetooth: Should be install time only.
bluetoothDevice: Install time or runtime, whichever the app chooses.

If both are requested at install time, they should be collapsed into a single
warning: "Access bluetooth devices including <device name>".
But now I'm not sure if it's possible to request bluetoothDevice at runtime,
since it won't be a permission but a top level key?

+
+  return result;
+}
+
+bool BluetoothManifestPermission::FromValue(const base::Value* value) {
+  if (!value)
+    return false;
+  base::string16 error;
+  scoped_ptr<BluetoothManifestPermission> manifest_permission(
+      BluetoothManifestPermission::FromValue(*value, &error));

[not at google - send to devlin, 2014/01/23 20:29:12]

what about the error here? Not sure what is supposd to happen to it, maybe pass
to the caller?

[rpaquay, 2014/01/24 16:58:23]

AFAIK, ultimately this method is called only when deserializing permissions form
the json format we use in extension_prefs, i.e. the persistent list of
permissions granted for a given extension (see
ExtensionPrefs::ReadPrefAsPermissionSet). So this is *not* related to giving
precise and meaningful error to the user when parsing manifest.json.

+
+  if (!manifest_permission)
+    return false;
+
+  profile_uuids_ = manifest_permission->profile_uuids_;
+  return true;
+}
+
+scoped_ptr<base::Value> BluetoothManifestPermission::ToValue() const {
+  api::manifest_types::Bluetooth bluetooth;
+  bluetooth.profiles.reset(new std::vector<std::string>(profile_uuids_));
+  return scoped_ptr<base::Value>(bluetooth.ToValue().release()).Pass();

[not at google - send to devlin, 2014/01/23 20:29:12]

use bluetooth.ToValue().PassAs<base::Value>() here?

(btw you don't need the Pass() on the end there)

[rpaquay, 2014/01/24 16:58:23]

Done.

[not at google - send to devlin, 2014/01/24 21:59:38]

(Not confident in my C++ here but...) could you replace the std::copy with

bluetooth.profiles.reset(new std::vector<std::string>(
    profile_uuids_.begin(), profile_uuids_.end()));

?

[rpaquay, 2014/01/25 00:48:50]

Yep, done.

+}
+
+ManifestPermission* BluetoothManifestPermission::Clone() const {
+  scoped_ptr<BluetoothManifestPermission> result(
+      new BluetoothManifestPermission());
+  result->profile_uuids_ = profile_uuids_;
+  return result.release();
+}
+
+ManifestPermission* BluetoothManifestPermission::Diff(
+    const ManifestPermission* rhs) const {
+  const BluetoothManifestPermission* other =
+      static_cast<const BluetoothManifestPermission*>(rhs);
+
+  scoped_ptr<BluetoothManifestPermission> result(
+      new BluetoothManifestPermission());
+  std::set_difference(

[not at google - send to devlin, 2014/01/23 20:29:12]

Can you use
https://code.google.com/p/chromium/codesearch#chromium/src/base/stl_util.h&q=...
here?

[rpaquay, 2014/01/24 16:58:23]

Done.

+      profile_uuids_.begin(), profile_uuids_.end(),
+      other->profile_uuids_.begin(), other->profile_uuids_.end(),
+      std::inserter<BluetoothProfileUuidSet>(
+          result->profile_uuids_, result->profile_uuids_.begin()));
+  return result.release();
+}
+
+ManifestPermission* BluetoothManifestPermission::Union(
+    const ManifestPermission* rhs) const {
+  const BluetoothManifestPermission* other =
+      static_cast<const BluetoothManifestPermission*>(rhs);
+
+  scoped_ptr<BluetoothManifestPermission> result(
+      new BluetoothManifestPermission());
+  std::set_union(

[not at google - send to devlin, 2014/01/23 20:29:12]

(no equivalent for union but maybe there should be, same for all of these other
set operations)

[rpaquay, 2014/01/24 16:58:23]

Done.

+      profile_uuids_.begin(), profile_uuids_.end(),
+      other->profile_uuids_.begin(), other->profile_uuids_.end(),
+      std::inserter<BluetoothProfileUuidSet>(
+          result->profile_uuids_, result->profile_uuids_.begin()));
+  return result.release();
+}
+
+ManifestPermission* BluetoothManifestPermission::Intersect(
+    const ManifestPermission* rhs) const {
+  const BluetoothManifestPermission* other =
+      static_cast<const BluetoothManifestPermission*>(rhs);
+
+  scoped_ptr<BluetoothManifestPermission> result(
+      new BluetoothManifestPermission());
+  std::set_intersection(
+      profile_uuids_.begin(), profile_uuids_.end(),
+      other->profile_uuids_.begin(), other->profile_uuids_.end(),
+      std::inserter<BluetoothProfileUuidSet>(
+          result->profile_uuids_, result->profile_uuids_.begin()));
+  return result.release();
+}
+
+bool BluetoothManifestPermission::Contains(const ManifestPermission* rhs)
+    const {
+  const BluetoothManifestPermission* other =
+      static_cast<const BluetoothManifestPermission*>(rhs);
+
+  return std::includes(
+      profile_uuids_.begin(), profile_uuids_.end(),
+      other->profile_uuids_.begin(), other->profile_uuids_.end());
+}
+
+bool BluetoothManifestPermission::Equal(const ManifestPermission* rhs) const {
+  const BluetoothManifestPermission* other =
+      static_cast<const BluetoothManifestPermission*>(rhs);
+
+  return (profile_uuids_ == other->profile_uuids_);
+}
+
+void BluetoothManifestPermission::Write(IPC::Message* m) const {
+  IPC::WriteParam(m, profile_uuids_);
+}
+
+bool BluetoothManifestPermission::Read(const IPC::Message* m,
+                                     PickleIterator* iter) {
+  return IPC::ReadParam(m, iter, &profile_uuids_);
+}
+
+void BluetoothManifestPermission::Log(std::string* log) const {
+  IPC::LogParam(profile_uuids_, log);
+}
+
+void BluetoothManifestPermission::AddPermission(
+    const std::string& profile_uuid) {
+  profile_uuids_.push_back(profile_uuid);
+}
+
+}  // namespace extensions