[Extensions Bindings] Accept null callbacks while ignoring schema

[Extensions Bindings] Accept null callbacks while ignoring schema

For legacy reasons, we have the method ConvertArgumentsIgnorningSchema,
which still needs to take into account the callback method as the final
parameter. Accept a null callback for these cases, and unittests for the
same. This fixes some of the context menus browser tests with native
bindings.

Also update the context menus handler bindings.

BUG=629431
Review-Url: https://codereview.chromium.org/2915813003
Cr-Commit-Position: refs/heads/master@{#478398}
Committed: https://chromium.googlesource.com/chromium/src/+/e118f4d92ff6564c75fea95c07864a483cc42f29

[Devlin, 2017-05-31 19:50:42]

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

[commit-bot: I haz the power, 2017-05-31 19:51: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-05-31 21:23:11]

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

[commit-bot: I haz the power, 2017-05-31 21:23:11]

Dry run: This issue passed the CQ dry run.

[Devlin, 2017-06-03 01:17:50]

Description was changed from

==========
fixcontext

BUG=
==========

to

==========
[Extensions Bindings] Accept null callbacks while ignoring schema

For legacy reasons, we have the method ConvertArgumentsIgnorningSchema,
which still needs to take into account the callback method as the final
parameter. Accept a null callback for these cases, and unittests for the
same. This fixes some of the context menus browser tests with native
bindings.

Also update the context menus handler bindings.

BUG=629431
==========

[Devlin, 2017-06-03 01:18:06]

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

[Devlin, 2017-06-03 01:18:07]

And a short one for y'all. :)

[jbroman, 2017-06-05 18:16:32]

lgtm

https://codereview.chromium.org/2915813003/diff/1/extensions/renderer/api_sig...
File extensions/renderer/api_signature.cc (right):

https://codereview.chromium.org/2915813003/diff/1/extensions/renderer/api_sig...
extensions/renderer/api_signature.cc:322: DCHECK(value->IsFunction() ||
value->IsUndefined() || value->IsNull());
This DCHECK implies this value can only come from Chrome code, right? (it's not
provided by the extension author?)

https://codereview.chromium.org/2915813003/diff/1/extensions/renderer/api_sig...
File extensions/renderer/api_signature_unittest.cc (right):

https://codereview.chromium.org/2915813003/diff/1/extensions/renderer/api_sig...
extensions/renderer/api_signature_unittest.cc:406: EXPECT_EQ("[{\"not\":\"a
string\"}]", ValueToString(*parsed));
or R"([{"not":"a string"}])", if you prefer

[lazyboy, 2017-06-05 18:26:16]

lgtm

[Devlin, 2017-06-09 18:27:25]

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

[commit-bot: I haz the power, 2017-06-09 18:27:48]

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-09 19:24:30]

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

[commit-bot: I haz the power, 2017-06-09 19:24:31]

Dry run: This issue passed the CQ dry run.

[Devlin, 2017-06-09 20:26:31]

https://codereview.chromium.org/2915813003/diff/1/extensions/renderer/api_sig...
File extensions/renderer/api_signature.cc (right):

https://codereview.chromium.org/2915813003/diff/1/extensions/renderer/api_sig...
extensions/renderer/api_signature.cc:322: DCHECK(value->IsFunction() ||
value->IsUndefined() || value->IsNull());
On 2017/06/05 18:16:32, jbroman wrote:
> This DCHECK implies this value can only come from Chrome code, right? (it's
not
> provided by the extension author?)

More-or-less. :/  In theory, we should only get here after the extension
bindings have validated the signature, so we should know that it's one of these.
 In practice, there's flaws with this that are described above in the TODO. 
I've added a brief comment here explaining that.

https://codereview.chromium.org/2915813003/diff/1/extensions/renderer/api_sig...
File extensions/renderer/api_signature_unittest.cc (right):

https://codereview.chromium.org/2915813003/diff/1/extensions/renderer/api_sig...
extensions/renderer/api_signature_unittest.cc:406: EXPECT_EQ("[{\"not\":\"a
string\"}]", ValueToString(*parsed));
On 2017/06/05 18:16:32, jbroman wrote:
> or R"([{"not":"a string"}])", if you prefer

Sure, done.

[Devlin, 2017-06-09 20:26:34]

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

[Devlin, 2017-06-09 20:26:35]

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

[commit-bot: I haz the power, 2017-06-09 20:26:54]

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-09 20:32:23]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1497039994919030,
"parent_rev": "809e133889f27f882d9f4ad284a1916bcdf01d2d", "commit_rev":
"e118f4d92ff6564c75fea95c07864a483cc42f29"}

[commit-bot: I haz the power, 2017-06-09 20:32:37]

Description was changed from

==========
[Extensions Bindings] Accept null callbacks while ignoring schema

For legacy reasons, we have the method ConvertArgumentsIgnorningSchema,
which still needs to take into account the callback method as the final
parameter. Accept a null callback for these cases, and unittests for the
same. This fixes some of the context menus browser tests with native
bindings.

Also update the context menus handler bindings.

BUG=629431
==========

to

==========
[Extensions Bindings] Accept null callbacks while ignoring schema

For legacy reasons, we have the method ConvertArgumentsIgnorningSchema,
which still needs to take into account the callback method as the final
parameter. Accept a null callback for these cases, and unittests for the
same. This fixes some of the context menus browser tests with native
bindings.

Also update the context menus handler bindings.

BUG=629431

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

[commit-bot: I haz the power, 2017-06-09 20:32:39]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/e118f4d92ff6564c75fea95c0786...