[Extensions Bindings] Move setHandleRequestWithPromise to util

extensions/renderer/resources/utils.js

Index: extensions/renderer/resources/utils.js
diff --git a/extensions/renderer/resources/utils.js b/extensions/renderer/resources/utils.js
index 2cefb15144a16c4c20ca5aeb22402bb460d66c32..6182300364ee40c890ad6828c1171c0c77b73a2d 100644
--- a/extensions/renderer/resources/utils.js
+++ b/extensions/renderer/resources/utils.js
@@ -7,6 +7,21 @@ var schemaRegistry = requireNative('schema_registry');
 var CHECK = requireNative('logging').CHECK;
 var DCHECK = requireNative('logging').DCHECK;
 var WARNING = requireNative('logging').WARNING;
+var logActivity = requireNative('activityLogger');
+var exceptionHandler = require('uncaught_exception_handler');
+
+var runCallbackWithLastError;
+if (bindingUtil) {
+  runCallbackWithLastError = function(name, message, stack, callback, args) {
+    bindingUtil.runCallbackWithLastError(message, function() {
+      $Function.apply(callback, null, args);
+    });
+  }
+} else {
+  var lastError = require('lastError');
+  if (lastError)  // lastError can be undefined in unittests.
+    runCallbackWithLastError = lastError.run;
+}
 
 /**
  * An object forEach. Calls |f| with each (key, value) pair of |obj|, using
@@ -229,6 +244,36 @@ function promise(func) {
   });
 }
 
+function handleRequestWithPromise(
+    binding, methodName, apiName, customizedFunction) {

[lazyboy, 2017/06/29 00:05:00]

nit: apiName before methodName (feels odd otherwise)

[Devlin, 2017/06/29 18:21:26]

Fair point; done.

+  var fullName = apiName + '.' + methodName;
+  var extensionId = requireNative('process').GetExtensionId();
+  binding.setHandleRequest(methodName, function() {
+    logActivity.LogAPICall(extensionId, fullName, $Array.slice(arguments));
+    var stack = exceptionHandler.getExtensionStackTrace();
+    var callback = arguments[arguments.length - 1];
+    var args = $Array.slice(arguments, 0, arguments.length - 1);
+    var keepAlivePromise = requireAsync('keep_alive').then(function(module) {

[jbroman, 2017/06/22 17:31:43]

FYI: since you're using $Array.slice, $Function.apply etc. here,
Promise.prototype.then is equally vulnerable. If you're concerned about
interception (and interception here would enable capturing the keep_alive module
object, which seems like it might be powerful), consider doing something similar
and exposing $Promise.then and $Promise.catch. (Not new to this CL, but noticed
while skimming.)

Applies here and elsewhere.

Yes, this makes the syntax...unpleasant. :(

[Devlin, 2017/06/22 18:43:19]

... Yuck :(  Good catch.

Ideally, I'd rather just remove this whole function, since it's really odd and
only used in one place, but replacing it is also probably non-trivial (and
should likely just be done as part of converting mimeHandlerPrivate bindings).

The good news here is that since this is only used by a private API (and is
restricted to blessed contexts), theoretically the only code running here is
trusted (but "theoretically" doesn't help me sleep better at night).

I'll add a comment here to indicate no new code should use this.

+      return module.createKeepAlive();
+    });
+    $Function.apply(customizedFunction, this, args).then(function(result) {
+      if (callback) {
+        exceptionHandler.safeCallbackApply(
+            fullName, {__proto__: null, stack: stack}, callback, [result]);
+      }
+    }).catch(function(error) {
+      if (callback) {
+        var message = exceptionHandler.safeErrorToString(error, true);
+        runCallbackWithLastError(fullName, message, stack, callback);
+      }
+    }).then(function() {
+      keepAlivePromise.then(function(keepAlive) {
+        keepAlive.close();
+      });
+    });
+  });
+};
+
 exports.$set('forEach', forEach);
 exports.$set('loadTypeSchema', loadTypeSchema);
 exports.$set('lookup', lookup);
@@ -236,3 +281,4 @@ exports.$set('defineProperty', defineProperty);
 exports.$set('expose', expose);
 exports.$set('deepCopy', deepCopy);
 exports.$set('promise', promise);
+exports.$set('handleRequestWithPromise', handleRequestWithPromise);