Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(286)

Issue 2805123002: [Extensions Bindings] Allow schema violations through sendRequest (Closed)

Created:
3 years, 8 months ago by Devlin
Modified:
3 years, 8 months ago
Reviewers:
lazyboy, jbroman
CC:
chromium-apps-reviews_chromium.org, chromium-reviews, extensions-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[Extensions Bindings] Allow schema violations through sendRequest The context menus API is another that violates the schema is defines when calling sendRequest() through its custom bindings. Allow these as well, similar to how we allow them when an API updates its arguments post validation. The context menus API also has functions defined as properties on object parameters, and serializes these into dictionaries. Match the the serialization into a dictionary, but don't serialize properties of the function. Add an end-to-end test for the contextMenus API with native bindings. BUG=653596 Review-Url: https://codereview.chromium.org/2805123002 Cr-Commit-Position: refs/heads/master@{#463300} Committed: https://chromium.googlesource.com/chromium/src/+/1714411e513f2df50e5ce8ca057fe40ff3a1e449

Patch Set 1 #

Total comments: 2

Patch Set 2 : $Function.bind #

Patch Set 3 : . #

Patch Set 4 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+107 lines, -28 lines) Patch
M chrome/browser/extensions/native_bindings_apitest.cc View 1 2 3 chunks +48 lines, -0 lines 0 comments Download
M extensions/renderer/api_binding_js_util.cc View 1 chunk +9 lines, -4 lines 0 comments Download
M extensions/renderer/api_signature.cc View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M extensions/renderer/argument_spec.cc View 1 2 1 chunk +8 lines, -10 lines 0 comments Download
M extensions/renderer/resources/context_menus_custom_bindings.js View 2 chunks +3 lines, -2 lines 0 comments Download
M extensions/renderer/resources/context_menus_handlers.js View 1 7 chunks +38 lines, -12 lines 0 comments Download

Messages

Total messages: 23 (17 generated)
Devlin
Another couple of ugly cases :( Once we get all tests passing, we can audit ...
3 years, 8 months ago (2017-04-06 21:55:40 UTC) #2
jbroman
lgtm
3 years, 8 months ago (2017-04-07 18:20:52 UTC) #3
lazyboy
lgtm https://codereview.chromium.org/2805123002/diff/1/extensions/renderer/resources/context_menus_handlers.js File extensions/renderer/resources/context_menus_handlers.js (right): https://codereview.chromium.org/2805123002/diff/1/extensions/renderer/resources/context_menus_handlers.js#newcode14 extensions/renderer/resources/context_menus_handlers.js:14: $Function(bindingUtil.hasLastError, bindingUtil) : $Function.bind?
3 years, 8 months ago (2017-04-07 20:31:35 UTC) #4
Devlin
https://codereview.chromium.org/2805123002/diff/1/extensions/renderer/resources/context_menus_handlers.js File extensions/renderer/resources/context_menus_handlers.js (right): https://codereview.chromium.org/2805123002/diff/1/extensions/renderer/resources/context_menus_handlers.js#newcode14 extensions/renderer/resources/context_menus_handlers.js:14: $Function(bindingUtil.hasLastError, bindingUtil) : On 2017/04/07 20:31:34, lazyboy wrote: > ...
3 years, 8 months ago (2017-04-10 17:05:38 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2805123002/60001
3 years, 8 months ago (2017-04-10 17:06:08 UTC) #20
commit-bot: I haz the power
3 years, 8 months ago (2017-04-10 17:13:27 UTC) #23
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/1714411e513f2df50e5ce8ca057f...

Powered by Google App Engine
This is Rietveld 408576698