New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 921633 link

Starred by 1 user
Status: Fixed
Owner:
█ raphael....@intel.com
OoO until Feb 4th
Closed: Jan 15
Cc:
lfg@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

[NamedConstructor] "prototype" property has wrong descriptor

Project Member Reported by raphael....@intel.com, Jan 14 
See https://github.com/heycam/webidl/issues/276#issuecomment-454027343 and https://github.com/web-platform-tests/wpt/pull/14841

From the former:
> In Chrome, Object.getOwnPropertyDescriptor(Image, "prototype").writable is true, while it should be false per spec. (WebKit and Gecko pass.)

The code adding the "prototype" property in interface.cc.tmpl is calling v8::Object::Set(), which does not allow one to specify a property's attributes.
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 15 

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

commit 01f90f504217e4c37b1b75aa2c8813ad2f99be8d
Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Date: Tue Jan 15 11:36:19 2019

Make named constructors' |prototype|s have the right property descriptor

As per https://heycam.github.io/webidl/#named-constructors, a named
constructors "prototype" property should NOT be writable, configurable or
enumerable, yet we were creating a property with the opposite
characteristics.

Call v8::Object::DefineOwnProperty() rather than v8::Object::Set() so we can
pass the property descriptor values we want.

More tests for named constructors are coming in
https://github.com/web-platform-tests/wpt/pull/14841

While here, do some minor cleanups suggested in the review:
* Rename |interfacePrototype| to |interface_prototype| to follow Chromium's
  coding style
* Replace an if check for |result| with a CHECK(), as we are already calling
  ToChecked() when setting it.

Bug:  921633 
Change-Id: Ie1f31c7eb456f45e113cff3048f43aea7e05e8ba
Reviewed-on: https://chromium-review.googlesource.com/c/1409552
Commit-Queue: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622814}
[modify] https://crrev.com/01f90f504217e4c37b1b75aa2c8813ad2f99be8d/third_party/blink/renderer/bindings/templates/interface.cc.tmpl
[modify] https://crrev.com/01f90f504217e4c37b1b75aa2c8813ad2f99be8d/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface_constructor.cc
[modify] https://crrev.com/01f90f504217e4c37b1b75aa2c8813ad2f99be8d/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface_event_target.cc
[modify] https://crrev.com/01f90f504217e4c37b1b75aa2c8813ad2f99be8d/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface_named_constructor.cc
[modify] https://crrev.com/01f90f504217e4c37b1b75aa2c8813ad2f99be8d/third_party/blink/renderer/bindings/tests/results/core/v8_test_interface_named_constructor_2.cc
[modify] https://crrev.com/01f90f504217e4c37b1b75aa2c8813ad2f99be8d/third_party/blink/web_tests/external/wpt/html/semantics/embedded-content/the-img-element/Image-constructor.html

Comment 2 by raphael....@intel.com, Jan 15

Labels: Hotlist-Interop M-73
Status: Fixed (was: Started)
►

Sign in to add a comment