New issue
Advanced search Search tips

Issue 810487 link

Starred by 3 users
Status: Started
Owner:
rdevlin....@chromium.org
Cc:
lazyboy@chromium.org
jbroman@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Extension Bindings: Eliminate JS schema/util files

Project Member Reported by rdevlin....@chromium.org, Feb 8 2018 
With native bindings, we shouldn't need the JS schema, utility, validation, etc files, but there are a few places that still use them.  We should clean them up for performance (native should be faster), size (kill the code!), and consistency (returning the same errors) reasons.

Heavily related to, but not blocking,  issue 653596 .  Filing a separate bug, since it's not really necessary for the core bindings work to be considered complete (and because that bug already has some 200+ commits).
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 13 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/057f7f704bcabd73956a9c4e9123a38504075b61

commit 057f7f704bcabd73956a9c4e9123a38504075b61
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Tue Feb 13 16:25:18 2018

[Extensions Bindings] Add a native ValidateType method

Add a utility method exposed to JS to allow bindings to validate a type
against an expected schema. This is used to e.g. match expected
instances used in the declarative APIs.

This eliminates one of the remaining reliances on the utils/schemaUtils
JS files for native bindings.

Bug: 810487

Change-Id: Id87d38f8196f56b285e9a69221a34f643957d237
Reviewed-on: https://chromium-review.googlesource.com/905422
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536374}
[modify] https://crrev.com/057f7f704bcabd73956a9c4e9123a38504075b61/extensions/renderer/bindings/api_binding_js_util.cc
[modify] https://crrev.com/057f7f704bcabd73956a9c4e9123a38504075b61/extensions/renderer/bindings/api_binding_js_util.h
[modify] https://crrev.com/057f7f704bcabd73956a9c4e9123a38504075b61/extensions/renderer/bindings/api_binding_js_util_unittest.cc
[modify] https://crrev.com/057f7f704bcabd73956a9c4e9123a38504075b61/extensions/renderer/resources/declarative_webrequest_custom_bindings.js
[modify] https://crrev.com/057f7f704bcabd73956a9c4e9123a38504075b61/extensions/renderer/resources/guest_view/web_view/web_view_request_custom_bindings.js
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 26 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9f8faccf900938f122dae884bac0cfe909d42953

commit 9f8faccf900938f122dae884bac0cfe909d42953
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Mon Mar 26 16:04:45 2018

[Extensions Bindings] Allow native custom signature parsing

A few APIs validate arguments against a signature specified in an event
call, rather than as an API method. Allow this validation to happen
natively when native bindings are enabled by adding a JS utility to add
a custom signature and validate against it.

Use this for certificateProvider, printerProvider, and webRequest custom
bindings instead of the JS validation.

Add unittests for the new JS utility method.

Bug: 810487

Change-Id: Ifbc8480ae15acbdaa9ecdffcfb7bfe159e201510
Reviewed-on: https://chromium-review.googlesource.com/914808
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545806}
[modify] https://crrev.com/9f8faccf900938f122dae884bac0cfe909d42953/chrome/renderer/resources/extensions/certificate_provider_custom_bindings.js
[modify] https://crrev.com/9f8faccf900938f122dae884bac0cfe909d42953/chrome/test/data/extensions/api_test/webrequest/test_api.js
[modify] https://crrev.com/9f8faccf900938f122dae884bac0cfe909d42953/extensions/renderer/bindings/api_binding_js_util.cc
[modify] https://crrev.com/9f8faccf900938f122dae884bac0cfe909d42953/extensions/renderer/bindings/api_binding_js_util.h
[modify] https://crrev.com/9f8faccf900938f122dae884bac0cfe909d42953/extensions/renderer/bindings/api_binding_js_util_unittest.cc
[modify] https://crrev.com/9f8faccf900938f122dae884bac0cfe909d42953/extensions/renderer/bindings/api_type_reference_map.cc
[modify] https://crrev.com/9f8faccf900938f122dae884bac0cfe909d42953/extensions/renderer/bindings/api_type_reference_map.h
[modify] https://crrev.com/9f8faccf900938f122dae884bac0cfe909d42953/extensions/renderer/resources/printer_provider_custom_bindings.js
[modify] https://crrev.com/9f8faccf900938f122dae884bac0cfe909d42953/extensions/renderer/resources/web_request_event.js
[modify] https://crrev.com/9f8faccf900938f122dae884bac0cfe909d42953/extensions/test/data/api_test/printer_provider/request_capability/test.js
[modify] https://crrev.com/9f8faccf900938f122dae884bac0cfe909d42953/extensions/test/data/api_test/printer_provider/request_print/test.js
[modify] https://crrev.com/9f8faccf900938f122dae884bac0cfe909d42953/extensions/test/data/api_test/printer_provider/request_printers/test.js
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 27 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0e268f2b001dc472d17b95f07624e3aecebb7be8

commit 0e268f2b001dc472d17b95f07624e3aecebb7be8
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Tue Mar 27 17:53:20 2018

[Extensions Bindings] Exclude schemaUtils, json_schema with native bindings

The 'schemaUtils' and 'json_schema' JS bindings files are no longer
needed with native bindings. Don't include them as resources in
the dispatcher's source map.

Bug: 810487

Change-Id: I7476069311b7ba7d0ffc0a2e37a01dcd43808c66
Reviewed-on: https://chromium-review.googlesource.com/981261
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546157}
[modify] https://crrev.com/0e268f2b001dc472d17b95f07624e3aecebb7be8/extensions/renderer/dispatcher.cc
►

Sign in to add a comment