[Extensions Bindings] Move setHandleRequestWithPromise to util

[Extensions Bindings] Move setHandleRequestWithPromise to util

The JS bindings provides a setHandleRequestWithUtil() method in addition
to setHandleRequest(). This is currently used only in one location, and
is already implemented in terms of setHandleRequest (which has a native
equivalent). Move this to the common util file to avoid dependencies on
binding.js.

BUG=653596
TEST=browser_tests --gtest_filter=PDFExtensionTest.BlockDirectAccess
                   --native-crx-bindings=1

Review-Url: https://codereview.chromium.org/2952893002
Cr-Commit-Position: refs/heads/master@{#483430}
Committed: https://chromium.googlesource.com/chromium/src/+/e5e0e9ee896c6e6ce80549d6ef2fcbd96589532f

[Devlin, 2017-06-21 22:51:16]

The CQ bit was checked by rdevlin.cronin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-21 22:51:45]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Devlin, 2017-06-21 23:12:53]

Description was changed from

==========
handle request
BUG=
==========

to

==========
[Extensions Bindings] Move setHandleRequestWithPromise to util

The JS bindings provides a setHandleRequestWithUtil() method in addition
to setHandleRequest(). This is currently used only in one location, and
is already implemented in terms of setHandleRequest (which has a native
equivalent). Move this to the common util file to avoid dependencies on
binding.js.

BUG=653596
TEST=browser_tests --gtest_filter=PDFExtensionTest.BlockDirectAccess
                   --native-crx-bindings=1
==========

[commit-bot: I haz the power, 2017-06-21 23:44:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-21 23:44:45]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Devlin, 2017-06-22 15:24:46]

The CQ bit was checked by rdevlin.cronin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-22 15:24:55]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Devlin, 2017-06-22 16:10:00]

rdevlin.cronin@chromium.org changed reviewers:
+ lazyboy@chromium.org

[Devlin, 2017-06-22 16:10:00]

lazyboy@, mind taking a look?

jbroman@, FYI

[commit-bot: I haz the power, 2017-06-22 16:33:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-22 16:33:45]

Dry run: This issue passed the CQ dry run.

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

https://codereview.chromium.org/2952893002/diff/40001/extensions/renderer/res...
File extensions/renderer/resources/utils.js (right):

https://codereview.chromium.org/2952893002/diff/40001/extensions/renderer/res...
extensions/renderer/resources/utils.js:256: var keepAlivePromise =
requireAsync('keep_alive').then(function(module) {
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:20]

https://codereview.chromium.org/2952893002/diff/40001/extensions/renderer/res...
File extensions/renderer/resources/utils.js (right):

https://codereview.chromium.org/2952893002/diff/40001/extensions/renderer/res...
extensions/renderer/resources/utils.js:256: var keepAlivePromise =
requireAsync('keep_alive').then(function(module) {
On 2017/06/22 17:31:43, jbroman wrote:
> 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. :(

... 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.

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

lgtm

https://codereview.chromium.org/2952893002/diff/40001/extensions/renderer/res...
File extensions/renderer/resources/utils.js (right):

https://codereview.chromium.org/2952893002/diff/40001/extensions/renderer/res...
extensions/renderer/resources/utils.js:248: binding, methodName, apiName,
customizedFunction) {
nit: apiName before methodName (feels odd otherwise)

[Devlin, 2017-06-29 16:18:20]

The CQ bit was checked by rdevlin.cronin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-29 16:18:33]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-29 17:42:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-29 17:42:04]

Dry run: This issue passed the CQ dry run.

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

https://codereview.chromium.org/2952893002/diff/40001/extensions/renderer/res...
File extensions/renderer/resources/utils.js (right):

https://codereview.chromium.org/2952893002/diff/40001/extensions/renderer/res...
extensions/renderer/resources/utils.js:248: binding, methodName, apiName,
customizedFunction) {
On 2017/06/29 00:05:00, lazyboy wrote:
> nit: apiName before methodName (feels odd otherwise)

Fair point; done.

[Devlin, 2017-06-29 18:21:34]

The CQ bit was checked by rdevlin.cronin@chromium.org

[Devlin, 2017-06-29 18:21:35]

The patchset sent to the CQ was uploaded after l-g-t-m from lazyboy@chromium.org
Link to the patchset: https://codereview.chromium.org/2952893002/#ps60001
(title: "lazyboy's")

[commit-bot: I haz the power, 2017-06-29 18:21:57]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-29 18:25:54]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1498760494519090,
"parent_rev": "9a43a37ef8c0021b6c44876c6b4c9332eb8c2cdc", "commit_rev":
"e5e0e9ee896c6e6ce80549d6ef2fcbd96589532f"}

[commit-bot: I haz the power, 2017-06-29 18:26:07]

Description was changed from

==========
[Extensions Bindings] Move setHandleRequestWithPromise to util

The JS bindings provides a setHandleRequestWithUtil() method in addition
to setHandleRequest(). This is currently used only in one location, and
is already implemented in terms of setHandleRequest (which has a native
equivalent). Move this to the common util file to avoid dependencies on
binding.js.

BUG=653596
TEST=browser_tests --gtest_filter=PDFExtensionTest.BlockDirectAccess
                   --native-crx-bindings=1
==========

to

==========
[Extensions Bindings] Move setHandleRequestWithPromise to util

The JS bindings provides a setHandleRequestWithUtil() method in addition
to setHandleRequest(). This is currently used only in one location, and
is already implemented in terms of setHandleRequest (which has a native
equivalent). Move this to the common util file to avoid dependencies on
binding.js.

BUG=653596
TEST=browser_tests --gtest_filter=PDFExtensionTest.BlockDirectAccess
                   --native-crx-bindings=1

Review-Url: https://codereview.chromium.org/2952893002
Cr-Commit-Position: refs/heads/master@{#483430}
Committed:
https://chromium.googlesource.com/chromium/src/+/e5e0e9ee896c6e6ce80549d6ef2f...
==========

[commit-bot: I haz the power, 2017-06-29 18:26:09]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/e5e0e9ee896c6e6ce80549d6ef2f...