Bluetooth API: improve discovery

chrome/renderer/resources/extensions/experimental.bluetooth_custom_bindings.js

 // Copyright (c) 2012 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.
 
 // Custom bindings for the Bluetooth API.
 
 var chromeHidden = requireNative('chrome_hidden').GetChromeHidden();
 var sendRequest = require('sendRequest').sendRequest;
 
 // Use custom bindings to create an undocumented event listener that will
 // receive events about device discovery and call the event listener that was
 // provided with the request to begin discovery.
 chromeHidden.registerCustomHook('experimental.bluetooth', function(api) {
   var apiFunctions = api.apiFunctions;
 
   chromeHidden.bluetooth = {};
-  chromeHidden.bluetooth.handler = null;
+  chromeHidden.bluetooth.deviceDiscoveredHandler = null;
   chromeHidden.bluetooth.onDeviceDiscovered =
       new chrome.Event("experimental.bluetooth.onDeviceDiscovered");
 
-  function deviceDiscoveredListener(device) {
-    if (chromeHidden.bluetooth.handler != null)
-      chromeHidden.bluetooth.handler(device);
-  }
-  chromeHidden.bluetooth.onDeviceDiscovered.addListener(
-      deviceDiscoveredListener);
+  var clearDeviceDiscoveredHandler = function() {

[asargent_no_longer_on_chrome, 2012/07/24 20:29:45]

style nit: no need to assign this to a var - instead just write it like this:

function clearDeviceDiscoveredHandler() {
  ...
}

[bryeung, 2012/08/03 20:08:37]

Done.

+    chromeHidden.bluetooth.onDeviceDiscovered.removeListener(
+        chromeHidden.bluetooth.deviceDiscoveredHandler);
+    chromeHidden.bluetooth.deviceDiscoveredHandler = null;
+  };
 
-  apiFunctions.setHandleRequest('startDiscovery', function() {
-    var args = arguments;
-    if (args.length > 0 && args[0] && args[0].deviceCallback) {
-      chromeHidden.bluetooth.handler = args[0].deviceCallback;
-    }
-    sendRequest(this.name, args, this.definition.parameters);
-  });
-  apiFunctions.setHandleRequest('stopDiscovery', function() {
-    chromeHidden.bluetooth.handler = null;
-    sendRequest(this.name, arguments, this.definition.parameters);
-  });
+  apiFunctions.setHandleRequest('startDiscovery',
+      function() {

[asargent_no_longer_on_chrome, 2012/07/24 20:29:45]

nit: my personal preference is to leave the opening declaration of the function
on the line above this as it was before, so that the body of the function
doesn't have to indented so much.

[bryeung, 2012/08/03 20:08:37]

I'd prefer to keep all of the functions indented similarly here, and the one
below doesn't fit on the first line due to arguments.  Are you okay with leaving
this as-is?

[asargent_no_longer_on_chrome, 2012/08/03 23:35:23]

I think it's fine to leave it as you have it. Vi vs. emacs. Coke vs. Pepsi.
Canada vs. US.

+        var args = arguments;
+        if (args.length > 0 && args[0] && args[0].deviceCallback) {
+          chromeHidden.bluetooth.deviceDiscoveredHandler =
+              args[0].deviceCallback;
+          chromeHidden.bluetooth.onDeviceDiscovered.addListener(
+              chromeHidden.bluetooth.deviceDiscoveredHandler);
+          sendRequest(this.name,
+                      args,
+                      this.definition.parameters,
+                      {customCallback:this.customCallback});
+        } else {
+          throw new Error("deviceCallback is required in the options object");

[asargent_no_longer_on_chrome, 2012/07/24 20:29:45]

nit: the canonical way to report errors is to set lastError and call the result
callback. So at the top of the file you should add:

var lastError = require('last_error');

And then instead of throwing an Error on this line, you should instead do
something like this:

  
  if (typeof(args[args.length-1]) == "function") {
    var callback = args[args.length-1];
    lastError.set("deviceCallback is required in the options object");
    callback();
    return;
  }

[bryeung, 2012/08/03 20:08:37]

Done.

+        }
+      });
+  apiFunctions.setCustomCallback('startDiscovery',
+      function(name, request, response) {
+        if (chrome.runtime.lastError) {
+          clearDeviceDiscoveredHandler();
+          return;
+        }
+      });
+  apiFunctions.setHandleRequest('stopDiscovery',
+      function() {
+        clearDeviceDiscoveredHandler();
+        sendRequest(this.name, arguments, this.definition.parameters);
+      });
 });