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

content/renderer/bluetooth/web_bluetooth_impl.cc

Index: content/renderer/bluetooth/web_bluetooth_impl.cc
diff --git a/content/renderer/bluetooth/web_bluetooth_impl.cc b/content/renderer/bluetooth/web_bluetooth_impl.cc
index f245362d2259568b00c0b5ff799a9a313eda4e2e..f87bb37ef2056358a19a0d04d34a6cdc9ca41dc3 100644
--- a/content/renderer/bluetooth/web_bluetooth_impl.cc
+++ b/content/renderer/bluetooth/web_bluetooth_impl.cc
@@ -25,6 +25,16 @@
 
 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) {}
 
@@ -64,12 +74,12 @@ void WebBluetoothImpl::disconnect(const blink::WebString& 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())),
@@ -80,11 +90,12 @@ void WebBluetoothImpl::getPrimaryServices(
 
 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(
@@ -176,7 +187,7 @@ void WebBluetoothImpl::OnRequestDeviceComplete(
         blink::WebString::fromUTF8(device->id),
         blink::WebString::fromUTF8(device->name), uuids)));
   } else {
-    callbacks->onError(error);
+    callbacks->onError(ToInt32(error));
   }
 }
 
@@ -199,7 +210,7 @@ void WebBluetoothImpl::OnConnectComplete(
   if (error == blink::mojom::WebBluetoothError::SUCCESS) {
     callbacks->onSuccess();
   } else {
-    callbacks->onError(error);
+    callbacks->onError(ToInt32(error));
   }
 }
 
@@ -221,7 +232,7 @@ void WebBluetoothImpl::OnGetPrimaryServicesComplete(
     }
     callbacks->onSuccess(promise_services);
   } else {
-    callbacks->onError(error);
+    callbacks->onError(ToInt32(error));
   }
 }
 
@@ -246,7 +257,7 @@ void WebBluetoothImpl::OnGetCharacteristicsComplete(
     }
     callbacks->onSuccess(promise_characteristics);
   } else {
-    callbacks->onError(error);
+    callbacks->onError(ToInt32(error));
   }
 }
 
@@ -257,7 +268,7 @@ void WebBluetoothImpl::OnReadValueComplete(
   if (error == blink::mojom::WebBluetoothError::SUCCESS) {
     callbacks->onSuccess(value.PassStorage());
   } else {
-    callbacks->onError(error);
+    callbacks->onError(ToInt32(error));
   }
 }
 
@@ -268,7 +279,7 @@ void WebBluetoothImpl::OnWriteValueComplete(
   if (error == blink::mojom::WebBluetoothError::SUCCESS) {
     callbacks->onSuccess(value);
   } else {
-    callbacks->onError(error);
+    callbacks->onError(ToInt32(error));
   }
 }
 
@@ -278,7 +289,7 @@ void WebBluetoothImpl::OnStartNotificationsComplete(
   if (error == blink::mojom::WebBluetoothError::SUCCESS) {
     callbacks->onSuccess();
   } else {
-    callbacks->onError(error);
+    callbacks->onError(ToInt32(error));
   }
 }