bluetooth: Move connect/disconnect to mojo

content/browser/bluetooth/bluetooth_connected_devices_map.cc

Index: content/browser/bluetooth/bluetooth_connected_devices_map.cc
diff --git a/content/browser/bluetooth/bluetooth_connected_devices_map.cc b/content/browser/bluetooth/bluetooth_connected_devices_map.cc
new file mode 100644
index 0000000000000000000000000000000000000000..c76c006b62a59dcaa5af9b7b7952364230e216de
--- /dev/null
+++ b/content/browser/bluetooth/bluetooth_connected_devices_map.cc
@@ -0,0 +1,104 @@
+// Copyright 2016 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 "content/browser/bluetooth/bluetooth_connected_devices_map.h"
+
+#include "base/strings/string_util.h"
+#include "content/browser/web_contents/web_contents_impl.h"
+#include "content/public/browser/web_contents.h"
+#include "device/bluetooth/bluetooth_gatt_connection.h"
+
+namespace content {
+
+BluetoothConnectedDevicesMap::BluetoothConnectedDevicesMap(
+    WebContents* web_contents)
+    : web_contents_impl_(static_cast<WebContentsImpl*>(web_contents)) {}
+
+BluetoothConnectedDevicesMap::~BluetoothConnectedDevicesMap() {
+  for (size_t i = 0; i < device_id_to_connection_map_.size(); i++) {
+    DecrementDevicesConnectedCount();
+  }
+}
+
+bool BluetoothConnectedDevicesMap::IsConnectedToDeviceWithId(
+    const std::string& device_id) {
+  auto connection_iter = device_id_to_connection_map_.find(device_id);
+  if (connection_iter == device_id_to_connection_map_.end()) {
+    return false;
+  }
+  // Owners of BluetoothConnectedDevicesMap should notify it when a device
+  // disconnects but currently Android and Mac don't notify of disconnection,
+  // so the map could get into a state where it's holding a stale connection.
+  // For this reason we return the value of IsConnected for the connection.
+  // TODO(ortuno): Always return true once Android and Mac notify of
+  // disconnection.
+  // http://crbug.com/607273
+  return connection_iter->second->IsConnected();
+}
+
+void BluetoothConnectedDevicesMap::Insert(
+    const std::string& device_id,
+    std::unique_ptr<device::BluetoothGattConnection> connection) {
+  auto connection_iter = device_id_to_connection_map_.find(device_id);
+  if (connection_iter != device_id_to_connection_map_.end()) {
+    // Owners of BluetoothConnectedDevicesMap should notify it when a device
+    // disconnects but currently Android and Mac don't notify of disconnection,
+    // so the map could get into a state where it's holding a stale connection.
+    // For this reason we check if the current connection is active and if
+    // not we remove it.
+    // TODO(ortuno): Remove once Android and Mac notify of disconnection.
+    // http://crbug.com/607273
+    if (!connection_iter->second->IsConnected()) {
+      device_address_to_id_map_.erase(
+          connection_iter->second->GetDeviceAddress());
+      device_id_to_connection_map_.erase(connection_iter);
+      DecrementDevicesConnectedCount();
+    } else {
+      // A frame should never try to connect to an already connected device.

[Jeffrey Yasskin, 2016/05/03 01:15:57]

I'm not sure whether we should detect duplicate connection attempts on the
browser or renderer side. This comment implies that you're now convinced it
should happen on the renderer side. How come? It looks like notifications are
moving to the browser side.

[ortuno, 2016/05/03 16:11:53]

This comment shouldn't imply that. The case I had in mind was
WebBluetoothServiceImpl getting two successive connection requests. Once
http://crbug.com/583544 is fixed, WebBluetoothServiceImpl can check if there is
a pending connection and if so queue the second connection or reject with In
Progress. But for now WebBluetoothServiceImpl would just try to connect twice
and we would get two callbacks for the same connection.

I changed the comment to clarify and also added this comment to
WebBluetoothServiceImpl::RemoteServerConnect.

[Jeffrey Yasskin, 2016/05/03 16:31:32]

Looks good, thanks.

+      // Right now this happens because there is no way of knowing of pending
+      // connections.
+      // TODO(ortuno): CHECK that this never happens.
+      // http://crbug.com/583544
+      return;
+    }
+  }
+  device_address_to_id_map_[connection->GetDeviceAddress()] = device_id;
+  device_id_to_connection_map_[device_id] = std::move(connection);
+  IncrementDevicesConnectedCount();
+}
+
+void BluetoothConnectedDevicesMap::CloseConnectionToDeviceWithId(
+    const std::string& device_id) {
+  auto connection_iter = device_id_to_connection_map_.find(device_id);
+  if (connection_iter == device_id_to_connection_map_.end()) {
+    return;
+  }
+  DCHECK(device_address_to_id_map_.erase(

[Jeffrey Yasskin, 2016/05/03 01:15:57]

Oops, the contents of DCHECK won't run in release mode. To DCHECK things that
should run in released Chrome, we have to assign them to local variables
unconditionally and then DCHECK the variables.

[ortuno, 2016/05/03 16:11:53]

Ah, I thought we did the same in BluetoothAllowedDevicesMap but turns out we
used CHECK there. Changed to CHECK. Let me know if you prefer the variable
pattern.

+      connection_iter->second->GetDeviceAddress()));
+  DCHECK(device_id_to_connection_map_.erase(device_id));

[Jeffrey Yasskin, 2016/05/03 01:15:57]

May as well erase(connection_iter) here, to avoid a redundant lookup.

[ortuno, 2016/05/03 16:11:53]

I did it this way because otherwise I have to compare the result with end(). Do
you think the optimization is worth the extra code?

CHECK(device_id_to_connection_map_.erase(connection_iter) !=
device_id_to_connection_map_.end());

+  DecrementDevicesConnectedCount();
+}
+
+std::string BluetoothConnectedDevicesMap::CloseConnectionToDeviceWithAddress(
+    const std::string& device_address) {
+  auto device_address_iter = device_address_to_id_map_.find(device_address);
+  if (device_address_iter == device_address_to_id_map_.end()) {
+    return base::EmptyString();

[Jeffrey Yasskin, 2016/05/03 01:15:57]

The main reason to use EmptyString() is if you're returning a string&. When
you're returning by value, it's simpler to return std::string().

[ortuno, 2016/05/03 16:11:53]

Done.

+  }
+  std::string device_id = device_address_iter->second;
+  DCHECK(device_address_to_id_map_.erase(device_address));
+  DCHECK(device_id_to_connection_map_.erase(device_id));
+  DecrementDevicesConnectedCount();
+  return device_id;
+}
+
+void BluetoothConnectedDevicesMap::IncrementDevicesConnectedCount() {
+  web_contents_impl_->IncrementBluetoothConnectedDeviceCount();
+}
+
+void BluetoothConnectedDevicesMap::DecrementDevicesConnectedCount() {
+  web_contents_impl_->DecrementBluetoothConnectedDeviceCount();
+}
+
+}  // namespace content