bluetooth: Implement acceptAllDevices

third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp

 // Copyright 2015 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 "modules/bluetooth/Bluetooth.h"
 
 #include "bindings/core/v8/CallbackPromiseAdapter.h"
 #include "bindings/core/v8/ScriptPromise.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "core/dom/DOMException.h"
@@ skipping 82 matching lines @@
           "'namePrefix', if present, must me non-empty.");
       return;
     }
     canonicalizedFilter.namePrefix = filter.namePrefix();
   }
 }
 
 static void convertRequestDeviceOptions(const RequestDeviceOptions& options,
                                         WebRequestDeviceOptions& result,
                                         ExceptionState& exceptionState) {
-  ASSERT(options.hasFilters());
-
-  if (options.filters().isEmpty()) {
+  if ((options.hasFilters() && options.acceptAllDevices()) ||

[scheib, 2016/11/22 21:22:44]

Do you find either of these easier to read?

  if (!(options.hasFilters() ^ options.acceptAllDevices()))

  if (options.hasFilters() == options.acceptAllDevices())

[ortuno, 2016/11/23 06:03:08]

Went with the first one. Done.

+      (!options.hasFilters() && !options.acceptAllDevices())) {
     exceptionState.throwTypeError(
-        "'filters' member must be non-empty to find any devices.");
+        "Either 'filters' should be present or 'acceptAllDevices' should be "
+        "true, but not both.");
+    return;
   }
 
-  Vector<WebBluetoothScanFilter> filters;
-  for (const BluetoothScanFilter& filter : options.filters()) {
-    WebBluetoothScanFilter canonicalizedFilter = WebBluetoothScanFilter();
+  result.acceptAllDevices = options.acceptAllDevices();
 
-    canonicalizeFilter(filter, canonicalizedFilter, exceptionState);
+  if (options.hasFilters()) {
+    if (options.filters().isEmpty()) {

[haraken, 2016/11/22 03:58:42]

Is it possible that isEmpty returns true while hasFilters returns true?

[ortuno, 2016/11/22 04:00:12]

Yup:

navigator.bluetooth.requestDevice({filters: []});

+      exceptionState.throwTypeError(
+          "'filters' member must be non-empty to find any devices.");
+      return;
+    }
 
-    if (exceptionState.hadException())
-      return;
+    Vector<WebBluetoothScanFilter> filters;
+    for (const BluetoothScanFilter& filter : options.filters()) {
+      WebBluetoothScanFilter canonicalizedFilter = WebBluetoothScanFilter();
 
-    filters.append(canonicalizedFilter);
+      canonicalizeFilter(filter, canonicalizedFilter, exceptionState);
+
+      if (exceptionState.hadException())
+        return;
+
+      filters.append(canonicalizedFilter);
+    }
+
+    result.filters.assign(filters);
+  } else {
+    result.hasFilters = false;
   }
 
-  result.filters.assign(filters);
-
   if (options.hasOptionalServices()) {
     Vector<WebString> optionalServices;
     for (const StringOrUnsignedLong& optionalService :
          options.optionalServices()) {
       const String& validatedOptionalService =
           BluetoothUUID::getService(optionalService, exceptionState);
       if (exceptionState.hadException())
         return;
       optionalServices.append(validatedOptionalService);
     }
@@ skipping 25 matching lines @@
         m_resolver->getExecutionContext()->activeDOMObjectsAreStopped())
       return;
     m_resolver->reject(BluetoothError::take(m_resolver, error));
   }
 
  private:
   Persistent<Bluetooth> m_bluetooth;
   Persistent<ScriptPromiseResolver> m_resolver;
 };
 
-// https://webbluetoothchrome.github.io/web-bluetooth/#dom-bluetooth-requestdevice
 ScriptPromise Bluetooth::requestDevice(ScriptState* scriptState,
                                        const RequestDeviceOptions& options,
                                        ExceptionState& exceptionState) {
   ExecutionContext* context = scriptState->getExecutionContext();
 
-  // 1. If the incumbent settings object is not a secure context, reject promise
-  //    with a SecurityError and abort these steps.
   String errorMessage;
   if (!context->isSecureContext(errorMessage)) {
     return ScriptPromise::rejectWithDOMException(
         scriptState, DOMException::create(SecurityError, errorMessage));
   }
 
-  // 2. If the algorithm is not allowed to show a popup, reject promise with a
-  //    SecurityError and abort these steps.
   if (!UserGestureIndicator::consumeUserGesture()) {
     return ScriptPromise::rejectWithDOMException(
         scriptState,
         DOMException::create(
             SecurityError,
             "Must be handling a user gesture to show a permission request."));
   }
 
   WebBluetooth* webbluetooth =
       BluetoothSupplement::fromScriptState(scriptState);
   if (!webbluetooth)
     return ScriptPromise::rejectWithDOMException(
         scriptState, DOMException::create(NotSupportedError));
 
-  // 3. In order to convert the arguments from service names and aliases to just
-  //    UUIDs, do the following substeps:
   WebRequestDeviceOptions webOptions;

[scheib, 2016/11/22 21:22:44]

Some of these comments helped explain why these steps were happening :|

[ortuno, 2016/11/23 06:03:08]

hmm I think the code is pretty clear already but either way I added the comments
back and removed the number since those are no longer correct.

   convertRequestDeviceOptions(options, webOptions, exceptionState);
   if (exceptionState.hadException())
     return exceptionState.reject(scriptState);
 
-  // Subsequent steps are handled in the browser process.
   ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
   ScriptPromise promise = resolver->promise();
   webbluetooth->requestDevice(webOptions,
                               new RequestDeviceCallback(this, resolver));
   return promise;
 }
 
 DEFINE_TRACE(Bluetooth) {
   visitor->trace(m_deviceInstanceMap);
 }
 
 BluetoothDevice* Bluetooth::getBluetoothDeviceRepresentingDevice(
     std::unique_ptr<WebBluetoothDeviceInit> deviceInit,
     ScriptPromiseResolver* resolver) {
   BluetoothDevice* device = m_deviceInstanceMap.get(deviceInit->id);
   if (!device) {
     String deviceId = deviceInit->id;
     device = BluetoothDevice::take(resolver, std::move(deviceInit));
 
     auto result = m_deviceInstanceMap.add(deviceId, device);
     DCHECK(result.isNewEntry);
   }
   return device;
 }
 
 }  // namespace blink