[chrome.displaySource] Session notification improvements

extensions/renderer/resources/display_source_custom_bindings.js

Index: extensions/renderer/resources/display_source_custom_bindings.js
diff --git a/extensions/renderer/resources/display_source_custom_bindings.js b/extensions/renderer/resources/display_source_custom_bindings.js
index 13e81670a05ec1f7e25ee0a7618bb06b95069f41..d67e4fa8e3c75d865b3519f268f0afba7e9d35a9 100644
--- a/extensions/renderer/resources/display_source_custom_bindings.js
+++ b/extensions/renderer/resources/display_source_custom_bindings.js
@@ -11,31 +11,50 @@ var natives = requireNative('display_source');
 
 binding.registerCustomHook(function(bindingsAPI, extensionId) {
   var apiFunctions = bindingsAPI.apiFunctions;
-  apiFunctions.setHandleRequest('startSession',
-      function(sessionInfo, callback) {
+  apiFunctions.setHandleRequest(
+      'startSession', function(sessionInfo, callback) {
+        function callbackWrapper(message) {
+          if (callback === undefined)
+            return;
+
+          try {
+            if (message !== null)
+              lastError.set(
+                  'displaySource.startSession', message, null, chrome);
+            callback();
+          } finally {
+            lastError.clear(chrome);
+          }
+        }
+
         try {
-          natives.StartSession(sessionInfo);
+          natives.StartSession(sessionInfo, callbackWrapper);

[asargent_no_longer_on_chrome, 2016/02/09 00:07:56]

Instead of having the callback wrapper handling split between JS and C++, where
the C++ code doesn't really do anything complicated, did you consider just doing
it entirely in JS? We essentially do that for most API requests that don't have
custom bindings - see the following functions in send_request.js :

-prepareRequest
-sendRequest
-handleResponse

Those just keep an array of information, including a reference to the callback
for later use, indexed by an integer request id that the C++ code assigns. 

[Mikhail, 2016/02/09 14:26:42]

I should have a look at this, thanks!

[Mikhail, 2016/02/10 15:58:51]

Done.

         } catch (e) {
-          lastError.set('displaySource.startSession', e.message, null, chrome);
-        } finally {
-           if (callback !== undefined)
-             callback();
-           lastError.clear(chrome);
+          callbackWrapper(e.message);
         }
-  });
-  apiFunctions.setHandleRequest('terminateSession',
-      function(sink_id, callback) {
+      });
+  apiFunctions.setHandleRequest(
+      'terminateSession', function(sink_id, callback) {
+        function callbackWrapper(message) {
+          if (callback === undefined)
+            return;
+
+          try {
+            if (message !== null)
+              lastError.set(
+                  'displaySource.terminateSession', message, null, chrome);
+            callback();
+          } finally {
+            lastError.clear(chrome);
+          }
+        }
+
         try {
-          natives.TerminateSession(sink_id);
+          natives.TerminateSession(sink_id, callbackWrapper);
         } catch (e) {
-          lastError.set(
-              'displaySource.terminateSession', e.message, null, chrome);
-        } finally {
-           if (callback !== undefined)
-             callback();
-           lastError.clear(chrome);
+          callbackWrapper(e.message);
         }
-  });
+      });
 });
 
 exports.$set('binding', binding.generate());