bluetooth: Avoid including non-blink mojo bindings in blink code

content/renderer/bluetooth/web_bluetooth_impl.cc

 // Copyright 2014 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/renderer/bluetooth/web_bluetooth_impl.h"
 
 #include <memory>
 #include <utility>
 #include <vector>
 
 #include "base/memory/ptr_util.h"
 #include "base/optional.h"
 #include "content/child/mojo/type_converters.h"
 #include "content/child/thread_safe_sender.h"
 #include "content/renderer/bluetooth/bluetooth_type_converters.h"
 #include "ipc/ipc_message.h"
 #include "mojo/public/cpp/bindings/array.h"
 #include "services/shell/public/cpp/interface_provider.h"
 #include "third_party/WebKit/public/platform/modules/bluetooth/WebBluetoothDevice.h"
 #include "third_party/WebKit/public/platform/modules/bluetooth/WebBluetoothDeviceInit.h"
 #include "third_party/WebKit/public/platform/modules/bluetooth/WebBluetoothRemoteGATTCharacteristic.h"
 #include "third_party/WebKit/public/platform/modules/bluetooth/WebBluetoothRemoteGATTCharacteristicInit.h"
 #include "third_party/WebKit/public/platform/modules/bluetooth/WebBluetoothRemoteGATTService.h"
 #include "third_party/WebKit/public/platform/modules/bluetooth/WebRequestDeviceOptions.h"
 
 namespace content {
 
+namespace {
+
+// Blink shouldn't use non-blink mojo bindings so we can't return a non-blink

[Jeffrey Yasskin, 2016/07/13 20:34:58]

Maybe "Blink can't use non-blink mojo enums like
blink::mojom::WebBluetoothError, so we pass it as an int32 across the boundary."

I'll also point out that it's confusing that a non-blink enum would be named
"blink::mojom::...".

[ortuno, 2016/07/13 22:47:28]

Done.
Yeah I known :/ We incorrectly added the mojom to blink so we are stuck with the
blink namespace.

+// enum. For that reason we cast to int32_t.
+int32_t ToInt32(blink::mojom::WebBluetoothError error) {
+  return static_cast<int32_t>(error);
+}
+
+}  // namespace
+
 WebBluetoothImpl::WebBluetoothImpl(shell::InterfaceProvider* remote_interfaces)
     : remote_interfaces_(remote_interfaces), binding_(this) {}
 
 WebBluetoothImpl::~WebBluetoothImpl() {
 }
 
 void WebBluetoothImpl::requestDevice(
     const blink::WebRequestDeviceOptions& options,
     blink::WebBluetoothRequestDeviceCallbacks* callbacks) {
   GetWebBluetoothService().RequestDevice(
@@ skipping 19 matching lines @@
 
 void WebBluetoothImpl::disconnect(const blink::WebString& device_id) {
   connected_devices_.erase(device_id.utf8());
 
   GetWebBluetoothService().RemoteServerDisconnect(
       mojo::String::From(device_id));
 }
 
 void WebBluetoothImpl::getPrimaryServices(
     const blink::WebString& device_id,
-
-    blink::mojom::WebBluetoothGATTQueryQuantity quantity,
+    int32_t quantity,
     const blink::WebString& services_uuid,
     blink::WebBluetoothGetPrimaryServicesCallbacks* callbacks) {
   GetWebBluetoothService().RemoteServerGetPrimaryServices(
-      mojo::String::From(device_id), quantity,
+      mojo::String::From(device_id),
+      static_cast<blink::mojom::WebBluetoothGATTQueryQuantity>(quantity),
       services_uuid.isEmpty()
           ? base::nullopt
           : base::make_optional(device::BluetoothUUID(services_uuid.utf8())),
       base::Bind(&WebBluetoothImpl::OnGetPrimaryServicesComplete,
                  base::Unretained(this), device_id,
                  base::Passed(base::WrapUnique(callbacks))));
 }
 
 void WebBluetoothImpl::getCharacteristics(
     const blink::WebString& service_instance_id,
-    blink::mojom::WebBluetoothGATTQueryQuantity quantity,
+    int32_t quantity,
     const blink::WebString& characteristics_uuid,
     blink::WebBluetoothGetCharacteristicsCallbacks* callbacks) {
   GetWebBluetoothService().RemoteServiceGetCharacteristics(
-      mojo::String::From(service_instance_id), quantity,
+      mojo::String::From(service_instance_id),
+      static_cast<blink::mojom::WebBluetoothGATTQueryQuantity>(quantity),

[Jeffrey Yasskin, 2016/07/13 20:34:58]

Could you
CHECK(IsKnownEnumValue(static_cast<blink::mojom::WebBluetoothGATTQueryQuantity>(quantity)))
before using it?

[ortuno, 2016/07/13 22:47:28]

I could but I'm not sure what would be the benefit. The browser side checks the
enum anyway.

[Jeffrey Yasskin, 2016/07/13 22:56:14]

It makes sure that we match enums across the boundary, since the type system
isn't doing it for us. A DCHECK is probably just as fine since these are scoped
enums with an underlying type.

[ortuno, 2016/07/13 23:12:39]

Done.

       characteristics_uuid.isEmpty()
           ? base::nullopt
           : base::make_optional(
                 device::BluetoothUUID(characteristics_uuid.utf8())),
       base::Bind(&WebBluetoothImpl::OnGetCharacteristicsComplete,
                  base::Unretained(this), service_instance_id,
                  base::Passed(base::WrapUnique(callbacks))));
 }
 
 void WebBluetoothImpl::readValue(
@@ skipping 71 matching lines @@
     blink::mojom::WebBluetoothDevicePtr device) {
   if (error == blink::mojom::WebBluetoothError::SUCCESS) {
     blink::WebVector<blink::WebString> uuids(device->uuids.size());
     for (size_t i = 0; i < device->uuids.size(); ++i)
       uuids[i] = blink::WebString::fromUTF8(device->uuids[i]);
 
     callbacks->onSuccess(base::WrapUnique(new blink::WebBluetoothDeviceInit(
         blink::WebString::fromUTF8(device->id),
         blink::WebString::fromUTF8(device->name), uuids)));
   } else {
-    callbacks->onError(error);
+    callbacks->onError(ToInt32(error));
   }
 }
 
 void WebBluetoothImpl::GattServerDisconnected(const mojo::String& device_id) {
   auto device_iter = connected_devices_.find(device_id);
   if (device_iter != connected_devices_.end()) {
     // Remove device from the map before calling dispatchGattServerDisconnected
     // to avoid removing a device the gattserverdisconnected event handler might
     // have re-connected.
     blink::WebBluetoothDevice* device = device_iter->second;
     connected_devices_.erase(device_iter);
     device->dispatchGattServerDisconnected();
   }
 }
 
 void WebBluetoothImpl::OnConnectComplete(
     std::unique_ptr<blink::WebBluetoothRemoteGATTServerConnectCallbacks>
         callbacks,
     blink::mojom::WebBluetoothError error) {
   if (error == blink::mojom::WebBluetoothError::SUCCESS) {
     callbacks->onSuccess();
   } else {
-    callbacks->onError(error);
+    callbacks->onError(ToInt32(error));
   }
 }
 
 void WebBluetoothImpl::OnGetPrimaryServicesComplete(
     const blink::WebString& device_id,
     std::unique_ptr<blink::WebBluetoothGetPrimaryServicesCallbacks> callbacks,
     blink::mojom::WebBluetoothError error,
     mojo::Array<blink::mojom::WebBluetoothRemoteGATTServicePtr> services) {
   if (error == blink::mojom::WebBluetoothError::SUCCESS) {
     // TODO(dcheng): This WebVector should use smart pointers.
     blink::WebVector<blink::WebBluetoothRemoteGATTService*> promise_services(
         services.size());
 
     for (size_t i = 0; i < services.size(); i++) {
       promise_services[i] = new blink::WebBluetoothRemoteGATTService(
           blink::WebString::fromUTF8(services[i]->instance_id),
           blink::WebString::fromUTF8(services[i]->uuid), true /* isPrimary */,
           device_id);
     }
     callbacks->onSuccess(promise_services);
   } else {
-    callbacks->onError(error);
+    callbacks->onError(ToInt32(error));
   }
 }
 
 void WebBluetoothImpl::OnGetCharacteristicsComplete(
     const blink::WebString& service_instance_id,
     std::unique_ptr<blink::WebBluetoothGetCharacteristicsCallbacks> callbacks,
     blink::mojom::WebBluetoothError error,
     mojo::Array<blink::mojom::WebBluetoothRemoteGATTCharacteristicPtr>
         characteristics) {
   if (error == blink::mojom::WebBluetoothError::SUCCESS) {
     // TODO(dcheng): This WebVector should use smart pointers.
     blink::WebVector<blink::WebBluetoothRemoteGATTCharacteristicInit*>
         promise_characteristics(characteristics.size());
 
     for (size_t i = 0; i < characteristics.size(); i++) {
       promise_characteristics[i] =
           new blink::WebBluetoothRemoteGATTCharacteristicInit(
               service_instance_id,
               blink::WebString::fromUTF8(characteristics[i]->instance_id),
               blink::WebString::fromUTF8(characteristics[i]->uuid),
               characteristics[i]->properties);
     }
     callbacks->onSuccess(promise_characteristics);
   } else {
-    callbacks->onError(error);
+    callbacks->onError(ToInt32(error));
   }
 }
 
 void WebBluetoothImpl::OnReadValueComplete(
     std::unique_ptr<blink::WebBluetoothReadValueCallbacks> callbacks,
     blink::mojom::WebBluetoothError error,
     mojo::Array<uint8_t> value) {
   if (error == blink::mojom::WebBluetoothError::SUCCESS) {
     callbacks->onSuccess(value.PassStorage());
   } else {
-    callbacks->onError(error);
+    callbacks->onError(ToInt32(error));
   }
 }
 
 void WebBluetoothImpl::OnWriteValueComplete(
     const blink::WebVector<uint8_t>& value,
     std::unique_ptr<blink::WebBluetoothWriteValueCallbacks> callbacks,
     blink::mojom::WebBluetoothError error) {
   if (error == blink::mojom::WebBluetoothError::SUCCESS) {
     callbacks->onSuccess(value);
   } else {
-    callbacks->onError(error);
+    callbacks->onError(ToInt32(error));
   }
 }
 
 void WebBluetoothImpl::OnStartNotificationsComplete(
     std::unique_ptr<blink::WebBluetoothNotificationsCallbacks> callbacks,
     blink::mojom::WebBluetoothError error) {
   if (error == blink::mojom::WebBluetoothError::SUCCESS) {
     callbacks->onSuccess();
   } else {
-    callbacks->onError(error);
+    callbacks->onError(ToInt32(error));
   }
 }
 
 void WebBluetoothImpl::OnStopNotificationsComplete(
     std::unique_ptr<blink::WebBluetoothNotificationsCallbacks> callbacks) {
   callbacks->onSuccess();
 }
 
 void WebBluetoothImpl::DispatchCharacteristicValueChanged(
     const std::string& characteristic_instance_id,
@@ skipping 10 matching lines @@
     // Create an associated interface ptr and pass it to the WebBluetoothService
     // so that it can send us events without us prompting.
     blink::mojom::WebBluetoothServiceClientAssociatedPtrInfo ptr_info;
     binding_.Bind(&ptr_info, web_bluetooth_service_.associated_group());
     web_bluetooth_service_->SetClient(std::move(ptr_info));
   }
   return *web_bluetooth_service_;
 }
 
 }  // namespace content