Bluetooth API: improve discovery

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

Index: chrome/renderer/resources/extensions/experimental.bluetooth_custom_bindings.js
diff --git a/chrome/renderer/resources/extensions/experimental.bluetooth_custom_bindings.js b/chrome/renderer/resources/extensions/experimental.bluetooth_custom_bindings.js
index 4763dd6381bc667186709dda19f31a076c1f3ed1..0291aa613bc9e8fd2fb8ca6fe61bd78056ba74e0 100644
--- a/chrome/renderer/resources/extensions/experimental.bluetooth_custom_bindings.js
+++ b/chrome/renderer/resources/extensions/experimental.bluetooth_custom_bindings.js
@@ -14,26 +14,42 @@ 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);
+      });
 });