Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2244123005:
[api] Add PropertyDescriptor and DefineProperty(). (Closed)
Description[api] Add PropertyDescriptor and DefineProperty().
BUG=
Committed: https://crrev.com/8acb7ab9f152f6e0b3e96ce91a6c48df74446c4e
Cr-Commit-Position: refs/heads/master@{#39093}
Patch Set 1 #Patch Set 2 : Use undefined for empty handles #Patch Set 3 : Add test that shows need for has_attribute() functions #
Total comments: 10
Patch Set 4 : Address review comments. #Patch Set 5 : Use reference instead of pointer #Patch Set 6 : Rename test variables #
Total comments: 9
Patch Set 7 : Address review comments. #Patch Set 8 : Do not allow empty handle for value. #Patch Set 9 : Do not allow empty handle for value. #Patch Set 10 : Move copy constructor to public: section. #Patch Set 11 : Reword comment. #
Total comments: 6
Patch Set 12 : Defer all functions in v8::Descriptor to internal descriptor. #Patch Set 13 : Fix formatting. #Patch Set 14 : Move destructor. #
Total comments: 4
Patch Set 15 : Address review comments. #Patch Set 16 : Fix documentation. #Patch Set 17 : Remove enum State, more restrictive API. #Patch Set 18 : Improve documentation for descriptor. #Patch Set 19 : Improve documentation for descriptor. #Patch Set 20 : Minor fixes. #
Total comments: 4
Patch Set 21 : Do not copy property descriptor to internal descriptor. #Patch Set 22 : Descriptor in defineProperty() is not const. #Patch Set 23 : Fix typo. #
Dependent Patchsets: Messages
Total messages: 79 (59 generated)
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)
franzih@chromium.org changed reviewers: + verwaest@chromium.org
Hi Toon, Here's v8::DefineProperty() that uses a PropertyDescriptor. Can you have a look, please. Thanks, Franzi
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== [api] Add PropertyDescriptor and DefineProperty(). BUG= ========== to ========== [api] Add PropertyDescriptor and DefineProperty(). The v8::PropertyDescriptor has for every field x a has_x() function. We need has_x(), when we re-define a property. In a re-definition, there is a difference between !has_x() and the default value for x. BUG= ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)
https://codereview.chromium.org/2244123005/diff/40001/include/v8.h File include/v8.h (right): https://codereview.chromium.org/2244123005/diff/40001/include/v8.h#newcode2699 include/v8.h:2699: // The defineProperty function is used to add an own property and/or or. You can't add 'add' and 'update' a property :) https://codereview.chromium.org/2244123005/diff/40001/include/v8.h#newcode2704 include/v8.h:2704: // In general, CreateDataProperty will be faster, however, does not allow is faster https://codereview.chromium.org/2244123005/diff/40001/include/v8.h#newcode2710 include/v8.h:2710: const PropertyDescriptor* descriptor); Perhaps this should be a reference? We'll probably want to give out a reference when passing a PropertyDescriptor along with an interceptor call in the defineProperty-interceptor case. https://codereview.chromium.org/2244123005/diff/40001/include/v8.h#newcode3484 include/v8.h:3484: PropertyDescriptor(Local<Function> get, bool has_get = false, If we can get by without has_get etc, I'd prefer that. It seems like the only thing it's used for is that defineProperty with non-configurable properties doesn't throw an exception if we're redefining accessors to the same value. If you do have a set:undefined, that should be a conflict with a present setter, whereas absent set doesn't conflict. I'm not sure this is a relevant cornercase... Did I miss something? https://codereview.chromium.org/2244123005/diff/40001/src/api.cc File src/api.cc (right): https://codereview.chromium.org/2244123005/diff/40001/src/api.cc#newcode3701 src/api.cc:3701: if (self->IsAccessCheckNeeded() && I'm pretty sure that access is properly checked in DefineOwnProperty. I don't think we need it above either.
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
franzih@chromium.org changed reviewers: + jochen@chromium.org
Thanks Toon, I addressed your comments. Jochen, can you have a look before we land this? Thanks, Franzi https://codereview.chromium.org/2244123005/diff/40001/include/v8.h File include/v8.h (right): https://codereview.chromium.org/2244123005/diff/40001/include/v8.h#newcode2699 include/v8.h:2699: // The defineProperty function is used to add an own property and/or On 2016/08/18 07:25:39, Toon Verwaest wrote: > or. You can't add 'add' and 'update' a property :) Done. https://codereview.chromium.org/2244123005/diff/40001/include/v8.h#newcode2704 include/v8.h:2704: // In general, CreateDataProperty will be faster, however, does not allow On 2016/08/18 07:25:39, Toon Verwaest wrote: > is faster Done. https://codereview.chromium.org/2244123005/diff/40001/include/v8.h#newcode2710 include/v8.h:2710: const PropertyDescriptor* descriptor); On 2016/08/18 07:25:39, Toon Verwaest wrote: > Perhaps this should be a reference? We'll probably want to give out a reference > when passing a PropertyDescriptor along with an interceptor call in the > defineProperty-interceptor case. Done. https://codereview.chromium.org/2244123005/diff/40001/include/v8.h#newcode3484 include/v8.h:3484: PropertyDescriptor(Local<Function> get, bool has_get = false, On 2016/08/18 07:25:39, Toon Verwaest wrote: > If we can get by without has_get etc, I'd prefer that. It seems like the only > thing it's used for is that defineProperty with non-configurable properties > doesn't throw an exception if we're redefining accessors to the same value. If > you do have a set:undefined, that should be a conflict with a present setter, > whereas absent set doesn't conflict. I'm not sure this is a relevant > cornercase... Did I miss something? We need has_x() for all fields. We need to distinguish between undefined and not present, when we re-define an existing property: var o = {}; Object.defineProperty(o, "a", {set: function() {}, configurable: true}); Object.defineProperty(o, "a", {configurable: false}); // OK Object.defineProperty(o, "a", {}); // OK Object.defineProperty(o, "a", {set: undefined}); // TypeError o = {} Object.defineProperty(o, "a", {value: 42}); Object.defineProperty(o, "a", {}); // OK Object.defineProperty(o, "a", {value: undefined}); // TypeError I'll add a test case to show the difference for undefined and !has_get(). https://codereview.chromium.org/2244123005/diff/40001/src/api.cc File src/api.cc (right): https://codereview.chromium.org/2244123005/diff/40001/src/api.cc#newcode3701 src/api.cc:3701: if (self->IsAccessCheckNeeded() && On 2016/08/18 07:25:39, Toon Verwaest wrote: > I'm pretty sure that access is properly checked in DefineOwnProperty. I don't > think we need it above either. You're right. Deleted in in v8::Object::DefineOwnProperty() as well.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)
lgtm
https://codereview.chromium.org/2244123005/diff/100001/include/v8.h File include/v8.h (right): https://codereview.chromium.org/2244123005/diff/100001/include/v8.h#newcode3464 include/v8.h:3464: * Empty handles for value, get, or set should be treated as undefined. so has_value = true and value.is_empty() is valid? why? https://codereview.chromium.org/2244123005/diff/100001/include/v8.h#newcode3481 include/v8.h:3481: PropertyDescriptor(Local<Value> value, bool has_value, it's a bit odd that has_get below has a default value, but has_value here doesn't. In general, I think we should avoid using default arguments unless it really helps alot (at least that's what the styleguide says anyways). What about having three constructors that take nothing (generic), a value (data) or a getter & setter (accessor) and then e.g. set_enumerable() that implicitly will has_enumerable_ to true? https://codereview.chromium.org/2244123005/diff/100001/include/v8.h#newcode3495 include/v8.h:3495: PropertyDescriptor(Local<Function> get, bool has_get = false, should we have a version that also takes a FunctionTemplate for getter / setter? https://codereview.chromium.org/2244123005/diff/100001/include/v8.h#newcode3539 include/v8.h:3539: }; this struct must not be copied / assigned to. just delete the default copy and assignment operators.
Hi Jochen, I addressed your comments. Please take another look. Thanks, Franzi https://codereview.chromium.org/2244123005/diff/100001/include/v8.h File include/v8.h (right): https://codereview.chromium.org/2244123005/diff/100001/include/v8.h#newcode3464 include/v8.h:3464: * Empty handles for value, get, or set should be treated as undefined. On 2016/08/23 12:38:41, jochen wrote: > so has_value = true and value.is_empty() is valid? why? Because desc = {value: undefined} is different form desc = {}. I changed the comment. Does that make more sense? https://codereview.chromium.org/2244123005/diff/100001/include/v8.h#newcode3481 include/v8.h:3481: PropertyDescriptor(Local<Value> value, bool has_value, On 2016/08/23 12:38:41, jochen wrote: > it's a bit odd that has_get below has a default value, but has_value here > doesn't. > > In general, I think we should avoid using default arguments unless it really > helps alot (at least that's what the styleguide says anyways). > > What about having three constructors that take nothing (generic), a value (data) > or a getter & setter (accessor) and then e.g. set_enumerable() that implicitly > will has_enumerable_ to true? OK, I'll make the enumerable and configurable properties mutable and remove the default arguments. Indeed, the long parameter lists with true/false are hard to read. https://codereview.chromium.org/2244123005/diff/100001/include/v8.h#newcode3495 include/v8.h:3495: PropertyDescriptor(Local<Function> get, bool has_get = false, On 2016/08/23 12:38:41, jochen wrote: > should we have a version that also takes a FunctionTemplate for getter / setter? My use case is going to be to convert an internal PropertyDescriptor to a v8::PropertyDescriptor. I don't need a version for FunctionTemplates. Is it OK if we leave it out until we need it? https://codereview.chromium.org/2244123005/diff/100001/include/v8.h#newcode3539 include/v8.h:3539: }; On 2016/08/23 12:38:41, jochen wrote: > this struct must not be copied / assigned to. just delete the default copy and > assignment operators. Done.
https://codereview.chromium.org/2244123005/diff/100001/include/v8.h File include/v8.h (right): https://codereview.chromium.org/2244123005/diff/100001/include/v8.h#newcode3464 include/v8.h:3464: * Empty handles for value, get, or set should be treated as undefined. On 2016/08/23 16:08:23, Franzi wrote: > On 2016/08/23 12:38:41, jochen wrote: > > so has_value = true and value.is_empty() is valid? why? > > Because desc = {value: undefined} is different form desc = {}. I changed the > comment. Does that make more sense? Empty handle would indicate that though. Unlike for accessors, this isn't necessary for data properties I think.
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2016/08/23 18:01:43, Toon Verwaest wrote: > https://codereview.chromium.org/2244123005/diff/100001/include/v8.h > File include/v8.h (right): > > https://codereview.chromium.org/2244123005/diff/100001/include/v8.h#newcode3464 > include/v8.h:3464: * Empty handles for value, get, or set should be treated as > undefined. > On 2016/08/23 16:08:23, Franzi wrote: > > On 2016/08/23 12:38:41, jochen wrote: > > > so has_value = true and value.is_empty() is valid? why? > > > > Because desc = {value: undefined} is different form desc = {}. I changed the > > comment. Does that make more sense? > > Empty handle would indicate that though. Unlike for accessors, this isn't > necessary for data properties I think. I see what you mean. Fixed it.
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)
jkummerow@chromium.org changed reviewers: + jkummerow@chromium.org
DBC. https://codereview.chromium.org/2244123005/diff/200001/include/v8.h File include/v8.h (right): https://codereview.chromium.org/2244123005/diff/200001/include/v8.h#newcode3461 include/v8.h:3461: * Empty handles for `get` or `set` are interpreted as undefined. Is there a particular reason for this, which causes the need to have explicit has_get_ etc fields? The internal version uses: bool has_get() const { return !get_.is_null(); } so I'm wondering what the advantage is of doing it differently in the public version. (I'm aware that "not present" and "undefined" must be distinguished; the question is why "empty handle" is interpreted as "present but undefined" rather than "not present".) Also, why is this special for get/set but not value? AFAICS, non-writable data properties behave much like accessor properties when it comes to redefinitions. https://codereview.chromium.org/2244123005/diff/200001/include/v8.h#newcode3498 include/v8.h:3498: void set_enumerable(bool enumeralbe) { nit: typo https://codereview.chromium.org/2244123005/diff/200001/src/api.cc File src/api.cc (right): https://codereview.chromium.org/2244123005/diff/200001/src/api.cc#newcode3691 src/api.cc:3691: auto self = Utils::OpenHandle(this); nit: please don't use auto, make the type explicit. Again twice below. https://codereview.chromium.org/2244123005/diff/200001/test/cctest/test-api.cc File test/cctest/test-api.cc (right): https://codereview.chromium.org/2244123005/diff/200001/test/cctest/test-api.c... test/cctest/test-api.cc:16254: // successful because re-define with same value nit: here and other comments: capitalization, grammar, punctuation. https://codereview.chromium.org/2244123005/diff/200001/test/cctest/test-api.c... test/cctest/test-api.cc:23631: CHECK(!desc.enumerable()); This seems rather pointless. Given that !desc.has_enumerable(), why would anyone care what desc.enumerable() returns? If someone changed internal implementation details so this returns true, why would that be bad enough to warrant a test failure? The same comment also applies to all other "CHECK(desc.x()...)" where "CHECK(!desc.has_x())".
for the record, I proposed to move the internal PropertyDescriptor to the API instead of reimplementing it slightly differently
Description was changed from ========== [api] Add PropertyDescriptor and DefineProperty(). The v8::PropertyDescriptor has for every field x a has_x() function. We need has_x(), when we re-define a property. In a re-definition, there is a difference between !has_x() and the default value for x. BUG= ========== to ========== [api] Add PropertyDescriptor and DefineProperty(). BUG= ==========
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2016/08/26 15:01:17, jochen wrote: > for the record, I proposed to move the internal PropertyDescriptor to the API > instead of reimplementing it slightly differently Hi Jochen, I moved the internal PropertyDescriptor to the API, but I didn't like it: we lose the clean interface. I think embedders either have a valid descriptor, or they are in JS and could call Object.defineProperty() anyways. I don't think it's necessary to have the complex, spec compliant descriptor in the API. Also, Jakob pointed out, that Local<> is an extra indirection compared to Handle<> (never mind the code getting super ugly with constant casting between v8 and internal for value, get, and set). Jakob suggested to keep an internal PropertyDescriptor in the API PropertyDesriptor and no other data fields. So all functions defer to the internal object. I quite like it because now the interface is clean, and we're not reimplementing the logic. What do you think? Thanks, Franzi
sgtm
Looks good, minor comments. https://codereview.chromium.org/2244123005/diff/260001/include/v8.h File include/v8.h (right): https://codereview.chromium.org/2244123005/diff/260001/include/v8.h#newcode3668 include/v8.h:3668: State enumerable() const; I'd be fine with a "bool enumerable() / bool has_enumerable()" pair (more verbose but also more consistent with e.g. value/has_value), but this is okay too. https://codereview.chromium.org/2244123005/diff/260001/src/api.cc File src/api.cc (right): https://codereview.chromium.org/2244123005/diff/260001/src/api.cc#newcode3888 src/api.cc:3888: auto self = Utils::OpenHandle(this); nit: no "auto" please
lgtm
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...) v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/11768) v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/11707) v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/7836)
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Hi Jakob, I made some final changes. Do you want to have another look? Thanks, Franzi https://codereview.chromium.org/2244123005/diff/200001/include/v8.h File include/v8.h (right): https://codereview.chromium.org/2244123005/diff/200001/include/v8.h#newcode3498 include/v8.h:3498: void set_enumerable(bool enumeralbe) { On 2016/08/26 13:28:30, Jakob wrote: > nit: typo Done. https://codereview.chromium.org/2244123005/diff/260001/include/v8.h File include/v8.h (right): https://codereview.chromium.org/2244123005/diff/260001/include/v8.h#newcode3668 include/v8.h:3668: State enumerable() const; On 2016/08/29 12:32:23, Jakob wrote: > I'd be fine with a "bool enumerable() / bool has_enumerable()" pair (more > verbose but also more consistent with e.g. value/has_value), but this is okay > too. As discussed, I'm using bool instead of the enum State. I added another constructor so we can distinguish between present and absent for writable. https://codereview.chromium.org/2244123005/diff/260001/src/api.cc File src/api.cc (right): https://codereview.chromium.org/2244123005/diff/260001/src/api.cc#newcode3888 src/api.cc:3888: auto self = Utils::OpenHandle(this); On 2016/08/29 12:32:24, Jakob wrote: > nit: no "auto" please Done.
lgtm https://codereview.chromium.org/2244123005/diff/380001/src/api.cc File src/api.cc (right): https://codereview.chromium.org/2244123005/diff/380001/src/api.cc#newcode3897 src/api.cc:3897: desc.set_value(Utils::OpenHandle(*descriptor.value(), true)); This works, but it's just about the least efficient way to copy descriptor.private_->desc to desc. Can't we access the former directly, without copying? https://codereview.chromium.org/2244123005/diff/380001/test/cctest/test-api.cc File test/cctest/test-api.cc (right): https://codereview.chromium.org/2244123005/diff/380001/test/cctest/test-api.c... test/cctest/test-api.cc:23611: TEST(PropertyDescriptor) { I still think that unit tests for getters and setters are so trivial as to be pointless; but I'm okay with keeping them if you like them.
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by franzih@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2244123005/diff/380001/src/api.cc File src/api.cc (right): https://codereview.chromium.org/2244123005/diff/380001/src/api.cc#newcode3897 src/api.cc:3897: desc.set_value(Utils::OpenHandle(*descriptor.value(), true)); On 2016/08/31 14:15:13, Jakob wrote: > This works, but it's just about the least efficient way to copy > descriptor.private_->desc to desc. Can't we access the former directly, without > copying? Ok, changed it. Since defineProperty() can change the internal property descriptor when redefining a property (it receives all previous properties) it is now changing the external descriptor. I changed the documentation to make that clear. https://codereview.chromium.org/2244123005/diff/380001/test/cctest/test-api.cc File test/cctest/test-api.cc (right): https://codereview.chromium.org/2244123005/diff/380001/test/cctest/test-api.c... test/cctest/test-api.cc:23611: TEST(PropertyDescriptor) { On 2016/08/31 14:15:13, Jakob wrote: > I still think that unit tests for getters and setters are so trivial as to be > pointless; but I'm okay with keeping them if you like them. I like them. They catch some set/get typos or autocompletion errors.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by franzih@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from verwaest@chromium.org, jochen@chromium.org, jkummerow@chromium.org Link to the patchset: https://codereview.chromium.org/2244123005/#ps440001 (title: "Fix typo.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [api] Add PropertyDescriptor and DefineProperty(). BUG= ========== to ========== [api] Add PropertyDescriptor and DefineProperty(). BUG= ==========
Message was sent while issue was closed.
Committed patchset #23 (id:440001)
Message was sent while issue was closed.
Description was changed from ========== [api] Add PropertyDescriptor and DefineProperty(). BUG= ========== to ========== [api] Add PropertyDescriptor and DefineProperty(). BUG= Committed: https://crrev.com/8acb7ab9f152f6e0b3e96ce91a6c48df74446c4e Cr-Commit-Position: refs/heads/master@{#39093} ==========
Message was sent while issue was closed.
Patchset 23 (id:??) landed as https://crrev.com/8acb7ab9f152f6e0b3e96ce91a6c48df74446c4e Cr-Commit-Position: refs/heads/master@{#39093} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
