Skip to content

Commit 2ba8761

Browse files
author
bors-servo
authored
Auto merge of #21869 - jdm:window-getter, r=<try>
Window getter This is a rebase of #19904 with an attempt at porting Gecko's named property getter support to Servo on top of it. It's very rough but I want to see what effect it has on tests so far.
2 parents caa4d19 + d61e128 commit 2ba8761

File tree

11 files changed

+212
-77
lines changed

11 files changed

+212
-77
lines changed

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎components/script/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ metrics = {path = "../metrics"}
6868
mitochondria = "1.1.2"
6969
mime = "0.2.1"
7070
mime_guess = "1.8.0"
71+
mozjs_sys = "*"
7172
mozjs = "0.9.0"
7273
msg = {path = "../msg"}
7374
net_traits = {path = "../net_traits"}

‎components/script/dom/bindings/codegen/CodegenRust.py‎

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2920,6 +2920,14 @@ def definition_body(self):
29202920
%s;
29212921
assert!(!prototype_proto.is_null());""" % getPrototypeProto)]
29222922

2923+
if self.descriptor.hasNamedPropertiesObject():
2924+
assert self.descriptor.isGlobal()
2925+
assert not self.haveUnscopables
2926+
code.append(CGGeneric("""\
2927+
rooted!(in(cx) let mut prototype_proto_proto = prototype_proto.get());
2928+
dom::types::%s::create_named_properties_object(cx, prototype_proto_proto.handle(), prototype_proto.handle_mut());
2929+
assert!(!prototype_proto.is_null());""" % name))
2930+
29232931
properties = {
29242932
"id": name,
29252933
"unscopables": "unscopable_names" if self.haveUnscopables else "&[]"
@@ -2943,8 +2951,12 @@ def definition_body(self):
29432951
else:
29442952
proto_properties = properties
29452953

2954+
29462955
code.append(CGGeneric("""
29472956
rooted!(in(cx) let mut prototype = ptr::null_mut::<JSObject>());
2957+
"""))
2958+
2959+
code.append(CGGeneric("""
29482960
create_interface_prototype_object(cx,
29492961
prototype_proto.handle().into(),
29502962
&PrototypeClass,
@@ -2953,6 +2965,9 @@ def definition_body(self):
29532965
%(consts)s,
29542966
%(unscopables)s,
29552967
prototype.handle_mut().into());
2968+
""" % proto_properties))
2969+
2970+
code.append(CGGeneric("""
29562971
assert!(!prototype.is_null());
29572972
assert!((*cache)[PrototypeList::ID::%(id)s as usize].is_null());
29582973
(*cache)[PrototypeList::ID::%(id)s as usize] = prototype.get();
@@ -4998,9 +5013,7 @@ def getBody(self):
49985013
attrs = "JSPROP_ENUMERATE"
49995014
if self.descriptor.operations['IndexedSetter'] is None:
50005015
attrs += " | JSPROP_READONLY"
5001-
# FIXME(#11868) Should assign to desc.value, desc.get() is a copy.
5002-
fillDescriptor = ("desc.get().value = result_root.get();\n"
5003-
"fill_property_descriptor(MutableHandle::from_raw(desc), proxy.get(), (%s) as u32);\n"
5016+
fillDescriptor = ("fill_property_descriptor(MutableHandle::from_raw(desc), proxy.get(), result_root.get(), (%s) as u32);\n"
50045017
"return true;" % attrs)
50055018
templateValues = {
50065019
'jsvalRef': 'result_root.handle_mut()',
@@ -5013,8 +5026,7 @@ def getBody(self):
50135026
CGIndenter(CGProxyIndexedGetter(self.descriptor, templateValues)).define() + "\n" +
50145027
"}\n")
50155028

5016-
namedGetter = self.descriptor.operations['NamedGetter']
5017-
if namedGetter:
5029+
if self.descriptor.operations['NamedGetter']:
50185030
attrs = []
50195031
if not self.descriptor.interface.getExtendedAttribute("LegacyUnenumerableNamedProperties"):
50205032
attrs.append("JSPROP_ENUMERATE")
@@ -5024,9 +5036,7 @@ def getBody(self):
50245036
attrs = " | ".join(attrs)
50255037
else:
50265038
attrs = "0"
5027-
# FIXME(#11868) Should assign to desc.value, desc.get() is a copy.
5028-
fillDescriptor = ("desc.get().value = result_root.get();\n"
5029-
"fill_property_descriptor(MutableHandle::from_raw(desc), proxy.get(), (%s) as u32);\n"
5039+
fillDescriptor = ("fill_property_descriptor(MutableHandle::from_raw(desc), proxy.get(), result_root.get(), (%s) as u32);\n"
50305040
"return true;" % attrs)
50315041
templateValues = {
50325042
'jsvalRef': 'result_root.handle_mut()',
@@ -5267,11 +5277,10 @@ def getBody(self):
52675277
else:
52685278
indexed = ""
52695279

5270-
namedGetter = self.descriptor.operations['NamedGetter']
52715280
condition = "RUST_JSID_IS_STRING(id) || RUST_JSID_IS_INT(id)"
52725281
if indexedGetter:
52735282
condition = "index.is_none() && (%s)" % condition
5274-
if namedGetter:
5283+
if self.descriptor.operations['NamedGetter']:
52755284
named = """\
52765285
if %s {
52775286
let mut has_on_proto = false;
@@ -5354,8 +5363,7 @@ def getBody(self):
53545363
else:
53555364
getIndexedOrExpando = getFromExpando + "\n"
53565365

5357-
namedGetter = self.descriptor.operations['NamedGetter']
5358-
if namedGetter:
5366+
if self.descriptor.operations['NamedGetter']:
53595367
condition = "RUST_JSID_IS_STRING(id) || RUST_JSID_IS_INT(id)"
53605368
# From step 1:
53615369
# If O supports indexed properties and P is an array index, then:
@@ -5684,6 +5692,7 @@ def contains_unsafe_arg(arguments):
56845692
return reduce((lambda x, y: x or y[1] == '*mut JSContext'), arguments, False)
56855693

56865694
methods = []
5695+
56875696
for name, arguments, rettype in members():
56885697
arguments = list(arguments)
56895698
methods.append(CGGeneric("%sfn %s(&self%s) -> %s;\n" % (
@@ -5997,6 +6006,7 @@ def reexportedName(name):
59976006
cgThings = []
59986007

59996008
unscopableNames = []
6009+
60006010
for m in descriptor.interface.members:
60016011
if (m.isMethod() and
60026012
(not m.isIdentifierLess() or m == descriptor.operations["Stringifier"])):
@@ -6034,7 +6044,7 @@ def reexportedName(name):
60346044
elif m.getExtendedAttribute("Replaceable"):
60356045
cgThings.append(CGSpecializedReplaceableSetter(descriptor, m))
60366046

6037-
if (not m.isStatic() and not descriptor.interface.isCallback()):
6047+
if not m.isStatic() and not descriptor.interface.isCallback():
60386048
cgThings.append(CGMemberJITInfo(descriptor, m))
60396049

60406050
if descriptor.concrete:

‎components/script/dom/bindings/codegen/Configuration.py‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,8 +267,10 @@ def addOperation(operation, m):
267267
continue
268268

269269
def addIndexedOrNamedOperation(operation, m):
270-
self.proxy = True
270+
if not self.isGlobal():
271+
self.proxy = True
271272
if m.isIndexed():
273+
assert not self.isGlobal()
272274
operation = 'Indexed' + operation
273275
else:
274276
assert m.isNamed()
@@ -353,6 +355,15 @@ def binaryNameFor(self, name):
353355
def internalNameFor(self, name):
354356
return self._internalNames.get(name, name)
355357

358+
def hasNamedPropertiesObject(self):
359+
if self.interface.isExternal():
360+
return False
361+
362+
return self.isGlobal() and self.supportsNamedProperties()
363+
364+
def supportsNamedProperties(self):
365+
return self.operations['NamedGetter'] is not None
366+
356367
def getExtendedAttributes(self, member, getter=False, setter=False):
357368
def maybeAppendInfallibleToAttrs(attrs, throws):
358369
if throws is None:

‎components/script/dom/bindings/proxyhandler.rs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
use dom::bindings::conversions::is_dom_proxy;
1010
use dom::bindings::utils::delete_property_by_id;
11+
use js_sys::jsgc::{IntoHandle, IntoMutableHandle};
1112
use js::glue::{GetProxyHandler, GetProxyHandlerFamily};
1213
use js::glue::{GetProxyPrivate, SetProxyPrivate};
1314
use js::glue::InvokeGetOwnPropertyDescriptor;
@@ -20,9 +21,11 @@ use js::jsapi::HandleId as RawHandleId;
2021
use js::jsapi::HandleObject as RawHandleObject;
2122
use js::jsapi::JS_DefinePropertyById;
2223
use js::jsapi::JS_GetPropertyDescriptorById;
24+
use js::jsapi::JS_HasPropertyById;
2325
use js::jsapi::MutableHandle as RawMutableHandle;
2426
use js::jsapi::MutableHandleObject as RawMutableHandleObject;
2527
use js::jsapi::ObjectOpResult;
28+
use js::jsval::JSVal;
2629
use js::jsval::ObjectValue;
2730
use js::jsval::UndefinedValue;
2831
use js::rust::{Handle, HandleObject, MutableHandle, MutableHandleObject};
@@ -198,10 +201,32 @@ pub unsafe fn ensure_expando_object(
198201
pub fn fill_property_descriptor(
199202
mut desc: MutableHandle<PropertyDescriptor>,
200203
obj: *mut JSObject,
204+
v: JSVal,
201205
attrs: u32,
202206
) {
203207
desc.obj = obj;
208+
desc.value = v;
204209
desc.attrs = attrs;
205210
desc.getter = None;
206211
desc.setter = None;
207212
}
213+
214+
pub unsafe fn has_property_on_prototype(
215+
cx: *mut JSContext,
216+
proxy: HandleObject,
217+
id: RawHandleId,
218+
) -> Result<bool, ()> {
219+
rooted!(in(cx) let mut proto = ptr::null_mut());
220+
if !GetObjectProto(cx, proxy.into_handle(), proto.handle_mut().into_handle_mut()) {
221+
return Err(());
222+
}
223+
if proto.is_null() {
224+
return Ok(false);
225+
}
226+
let mut has = false;
227+
if JS_HasPropertyById(cx, proto.handle().into_handle(), id, &mut has) {
228+
Ok(has)
229+
} else {
230+
Err(())
231+
}
232+
}

‎components/script/dom/document.rs‎

Lines changed: 68 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,56 @@ pub struct Document {
396396
responsive_images: DomRefCell<Vec<Dom<HTMLImageElement>>>,
397397
}
398398

399+
// https://html.spec.whatwg.org/multipage/#dom-document-nameditem-filter
400+
fn filter_by_name(name: &Atom, element: &Element) -> bool {
401+
// TODO: Handle <embed>, <iframe> and <object>.
402+
if element.is::<HTMLFormElement>() || element.is::<HTMLImageElement>() {
403+
if let Some(attr) = element.get_attribute(&ns!(), &local_name!("name")) {
404+
if attr.value().as_atom() == name {
405+
return true;
406+
}
407+
}
408+
}
409+
410+
if element.is::<HTMLImageElement>() &&
411+
element.get_id().as_ref().map_or(false, |id| id == name)
412+
{
413+
return true;
414+
}
415+
416+
false
417+
}
418+
419+
/// The filter for the named getter HTMLCollection that must be returned from
420+
/// the document.
421+
#[derive(JSTraceable, MallocSizeOf)]
422+
struct NamedGetterFilter {
423+
name: Atom,
424+
}
425+
426+
impl CollectionFilter for NamedGetterFilter {
427+
fn filter(&self, elem: &Element, _root: &Node) -> bool {
428+
filter_by_name(&self.name, elem)
429+
}
430+
}
431+
432+
/// The filter for the named getter HTMLCollection that must be returned from
433+
/// the window (not the document).
434+
#[derive(JSTraceable, MallocSizeOf)]
435+
pub struct WindowNamedGetterFilter {
436+
pub name: Atom,
437+
}
438+
439+
impl CollectionFilter for WindowNamedGetterFilter {
440+
fn filter(&self, element: &Element, _: &Node) -> bool {
441+
if filter_by_name(&self.name, element) {
442+
return true;
443+
}
444+
445+
element.get_id().as_ref().map_or(false, |id| id == &self.name)
446+
}
447+
}
448+
399449
#[derive(JSTraceable, MallocSizeOf)]
400450
struct ImagesFilter;
401451
impl CollectionFilter for ImagesFilter {
@@ -4011,73 +4061,27 @@ impl DocumentMethods for Document {
40114061
_cx: *mut JSContext,
40124062
name: DOMString,
40134063
) -> Option<NonNull<JSObject>> {
4014-
#[derive(JSTraceable, MallocSizeOf)]
4015-
struct NamedElementFilter {
4016-
name: Atom,
4017-
}
4018-
impl CollectionFilter for NamedElementFilter {
4019-
fn filter(&self, elem: &Element, _root: &Node) -> bool {
4020-
filter_by_name(&self.name, elem.upcast())
4021-
}
4022-
}
4023-
// https://html.spec.whatwg.org/multipage/#dom-document-nameditem-filter
4024-
fn filter_by_name(name: &Atom, node: &Node) -> bool {
4025-
let html_elem_type = match node.type_id() {
4026-
NodeTypeId::Element(ElementTypeId::HTMLElement(type_)) => type_,
4027-
_ => return false,
4028-
};
4029-
let elem = match node.downcast::<Element>() {
4030-
Some(elem) => elem,
4031-
None => return false,
4032-
};
4033-
match html_elem_type {
4034-
HTMLElementTypeId::HTMLFormElement => {
4035-
match elem.get_attribute(&ns!(), &local_name!("name")) {
4036-
Some(ref attr) => attr.value().as_atom() == name,
4037-
None => false,
4038-
}
4039-
},
4040-
HTMLElementTypeId::HTMLImageElement => {
4041-
match elem.get_attribute(&ns!(), &local_name!("name")) {
4042-
Some(ref attr) => {
4043-
if attr.value().as_atom() == name {
4044-
true
4045-
} else {
4046-
match elem.get_attribute(&ns!(), &local_name!("id")) {
4047-
Some(ref attr) => attr.value().as_atom() == name,
4048-
None => false,
4049-
}
4050-
}
4051-
},
4052-
None => false,
4053-
}
4054-
},
4055-
// TODO: Handle <embed>, <iframe> and <object>.
4056-
_ => false,
4057-
}
4058-
}
40594064
let name = Atom::from(name);
4065+
let filter = NamedGetterFilter { name };
4066+
40604067
let root = self.upcast::<Node>();
40614068
{
40624069
// Step 1.
40634070
let mut elements = root
40644071
.traverse_preorder()
4065-
.filter(|node| filter_by_name(&name, &node))
4066-
.peekable();
4067-
if let Some(first) = elements.next() {
4068-
if elements.peek().is_none() {
4069-
// TODO: Step 2.
4070-
// Step 3.
4071-
return Some(NonNull::new_unchecked(
4072-
first.reflector().get_jsobject().get(),
4073-
));
4074-
}
4075-
} else {
4076-
return None;
4072+
.filter_map(DomRoot::downcast::<Element>)
4073+
.filter(|element| filter.filter(&element, &root));
4074+
4075+
let first_element = elements.next()?;
4076+
if elements.next().is_none() {
4077+
// TODO: Step 2.
4078+
// Step 3.
4079+
return Some(NonNull::new_unchecked(
4080+
first_element.reflector().get_jsobject().get()
4081+
));
40774082
}
40784083
}
40794084
// Step 4.
4080-
let filter = NamedElementFilter { name: name };
40814085
let collection = HTMLCollection::create(self.window(), root, Box::new(filter));
40824086
Some(NonNull::new_unchecked(
40834087
collection.reflector().get_jsobject().get(),
@@ -4087,6 +4091,12 @@ impl DocumentMethods for Document {
40874091
// https://html.spec.whatwg.org/multipage/#dom-tree-accessors:supported-property-names
40884092
fn SupportedPropertyNames(&self) -> Vec<DOMString> {
40894093
// FIXME: unimplemented (https://github.com/servo/servo/issues/7273)
4094+
//
4095+
// We really need a HashMap from name to element, much like `id_map` for
4096+
// this.
4097+
//
4098+
// Chances are that if you implement this, you can also implement most
4099+
// of Window::SupportedPropertyNames.
40904100
vec![]
40914101
}
40924102

‎components/script/dom/element.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,10 @@ impl Element {
321321
*self.is.borrow_mut() = Some(is);
322322
}
323323

324+
pub fn get_id(&self) -> Ref<Option<Atom>> {
325+
self.id_attribute.borrow()
326+
}
327+
324328
pub fn get_is(&self) -> Option<LocalName> {
325329
self.is.borrow().clone()
326330
}

‎components/script/dom/webidls/Window.webidl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
//getter WindowProxy (unsigned long index);
4646

4747
// https://github.com/servo/servo/issues/14453
48-
// getter object (DOMString name);
48+
getter object (DOMString name);
4949

5050
// the user agent
5151
readonly attribute Navigator navigator;

0 commit comments

Comments
 (0)