bindings: Instantiate named properties and indexed properties with the same prototype |
|||||
Issue descriptionIn V8, NamedPropertyHandlerConfiguration and IndexedPropertyHandlerConfiguration both have two different constructors each. One of them takes getter, setter, query, deleter and enumerator callbacks, while the other takes getter/setter/descriptor/deleter/enumerator and definer callbacks instead. A query callback is used to determine if a given property is present and, if it is, return an integer corresponding to v8::PropertyAttribute. Descriptor and definer callbacks were added later; the former returns a v8::Value created out of a v8::PropertyDescriptor object and is used in calls to e.g. Object.getOwnPropertyDescriptor(), and the latter is used to intercept calls when a new property is defined in an object. In the bindings layer, we add both indexed and named property handlers depending on what a given IDL interface supports. We used to use the same constructor prototype for both NamedPropertyHandlerConfiguration and IndexedPropertyHandlerConfiguration, but with https://codereview.chromium.org/2832923003 (and bug 695385 ), IndexedPropertyHandlerConfiguration started being instantiated with the descriptor+definer prototype instead. We should probably do the same to NamedPropertyHandlerConfiguration. Doing things symmetrically is helpful for a handful of reasons: - The code is easier to understand. - The definer/descriptor signature matches the slots for legacy platform objects in WebIDL spec more closely (https://heycam.github.io/webidl/#es-legacy-platform-objects). - Even if an interface only supports named properties, we still install a handler for indexed properties due to the way V8 works: a property like 0 or "0" will always go to an indexed property handler. When an interface only supports named properties, we make the indexed property callbacks just fall back gracefully to the named property ones, and doing so is more difficult when each HandlerConfiguration sets different callback types (with different signatures). See https://chromium-review.googlesource.com/c/chromium/src/+/660497, for example.
,
Mar 30 2018
,
Mar 30 2018
,
Mar 30 2018
I'm marking this is as blocking bug 618305 because we need descriptor callbacks to properly implement all the intricacies of Window/WindowProxy/Location property handling. Namely, https://html.spec.whatwg.org/multipage/browsers.html#crossoriginproperties-(-o-) shows that some properties need to be access property descriptor rather than data ones. That is not possible with query callbacks, which only allow one to return an int manipulating v8::PropertyAttribute (and it's actually the reason why descriptor callbacks were added to V8 in the first place). (Location/Window will likely need a custom property descriptor but that's orthogonal to the issue at hand)
,
Apr 3 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5b84e88a070e40a1a74576a6cf41109d29a8035a commit 5b84e88a070e40a1a74576a6cf41109d29a8035a Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com> Date: Tue Apr 03 13:14:36 2018 Remove custom bindings for CSSStyleDeclaration. Attempt to remove these custom bindings again after 2015's https://codereview.chromium.org/1119653002 and others. At the time the CLs were reverted because of performance regressions; I seem to be unable to trigger the perf trybots myself, so we will need to see if there is anything to fix after landing. There was nothing in the custom bindings that really required custom code, so the existing implementations of the named setter/getter/enumerator/query were moved almost as-is to CSSStyleDeclaration.cpp. The local supporting functions were almost untouched as well, except for the fact that they were moved into an anonymous namespace instead of being made static, and they all take AtomicString's in their arguments rather than String's. Removing these custom bindings helps get rid of named property queries (this was the only custom named query implementation in the tree), which we need to do in order to be able to instantiate indexed and named property handlers the same way and simplify our bindings code. Bug: 345519, 764633 Change-Id: I76171bb4c2ae23fc672748dc11b26094f16e573e Reviewed-on: https://chromium-review.googlesource.com/984232 Reviewed-by: Yuki Shiino <yukishiino@chromium.org> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Commit-Queue: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com> Cr-Commit-Position: refs/heads/master@{#547683} [modify] https://crrev.com/5b84e88a070e40a1a74576a6cf41109d29a8035a/third_party/WebKit/Source/bindings/bindings.gni [modify] https://crrev.com/5b84e88a070e40a1a74576a6cf41109d29a8035a/third_party/WebKit/Source/core/css/BUILD.gn [rename] https://crrev.com/5b84e88a070e40a1a74576a6cf41109d29a8035a/third_party/WebKit/Source/core/css/CSSStyleDeclaration.cpp [modify] https://crrev.com/5b84e88a070e40a1a74576a6cf41109d29a8035a/third_party/WebKit/Source/core/css/CSSStyleDeclaration.h [modify] https://crrev.com/5b84e88a070e40a1a74576a6cf41109d29a8035a/third_party/WebKit/Source/core/css/CSSStyleDeclaration.idl
,
Apr 11 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a21878dbbdb4f3ebe979d63b690f5a499bbd02fd commit a21878dbbdb4f3ebe979d63b690f5a499bbd02fd Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com> Date: Wed Apr 11 22:34:10 2018 CSSStyleDeclaration: Remove [RaisesException] from setter Previously, the custom bindings code for CSSStyleDeclaration only create an ExceptionState locally to pass to SetPropertyInternal(). Go back to this behavior and drop [RaisesException] from the setter due to performance issues: [RaisesException] causes an ExceptionState to be generated automatically in namedPropertySetter(), and an expensive CString needs to be created to set the ExceptionState's |property_name| argument. By creating an ExceptionState ourselves in AnonymousNamedSetter(), we can avoid creating a CString altogether and use a custom value for |property_name|. This brings some performance tests back to the values they used to report when we had custom bindings for CSSStyleDeclaration, namely: - blink_perf.css' CSSPropertySetterGetter and CSSPropertyUpdateValue - blink_perf.layout's SimpleTextPathLineLayout Bug: 764633, 829408 Change-Id: I99db6021d4483c5743daa452f9cb532eacda79a7 Reviewed-on: https://chromium-review.googlesource.com/1002881 Commit-Queue: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Yuki Shiino <yukishiino@chromium.org> Cr-Commit-Position: refs/heads/master@{#549955} [modify] https://crrev.com/a21878dbbdb4f3ebe979d63b690f5a499bbd02fd/third_party/blink/renderer/core/css/css_style_declaration.cc [modify] https://crrev.com/a21878dbbdb4f3ebe979d63b690f5a499bbd02fd/third_party/blink/renderer/core/css/css_style_declaration.h [modify] https://crrev.com/a21878dbbdb4f3ebe979d63b690f5a499bbd02fd/third_party/blink/renderer/core/css/css_style_declaration.idl
,
Apr 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fc2b8eaf154bbb50ae28adf35b456f763d65b91c commit fc2b8eaf154bbb50ae28adf35b456f763d65b91c Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com> Date: Thu Apr 12 07:54:29 2018 CSSStyleDeclaration: Add [TreatNullAs=NullString] to the setter Follow-up to 5b84e88a0 ("Remove custom bindings for CSSStyleDeclaration"). The custom bindings code used to convert the property value to a string via V8StringResource<kTreatNullAsNullString> even though that does not match the setter's signature in the IDL file. Now that the bindings are being generated automatically, the IDL's signature was being respected and causing property values to be converted via V8StringResource<kTreatNullAndUndefinedAsNullString> Update the signature in the IDL so that we maintain compatibility with the older code. Bug: 764633 Change-Id: Ief868d4d3a0655f56e844d3b05cfbd771b2cf8da Reviewed-on: https://chromium-review.googlesource.com/1002878 Commit-Queue: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com> Reviewed-by: Kentaro Hara <haraken@chromium.org> Cr-Commit-Position: refs/heads/master@{#550088} [modify] https://crrev.com/fc2b8eaf154bbb50ae28adf35b456f763d65b91c/third_party/blink/renderer/core/css/css_style_declaration.idl
,
Jun 15 2018
,
Nov 30
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by bugdroid1@chromium.org
, Sep 13 2017