Bindings: Make item_value return an Optional<JS::Value>

This removes some ambiguity about what the return value should be if
the index is out of range.

Previously, we would sometimes return a JS null, and other times a JS
undefined.

It will also let us fold together the checks for whether an index is a
supported property index, followed by getting the value just afterwards.
This commit is contained in:
Shannon Booth 2024-07-25 18:15:51 +12:00 committed by Andreas Kling
parent 9b59dc5e8b
commit c5c1a8fcc7
Notes: github-actions[bot] 2024-07-26 12:27:19 +00:00
31 changed files with 53 additions and 47 deletions

View file

@ -96,7 +96,7 @@ JS::ThrowCompletionOr<Optional<JS::PropertyDescriptor>> PlatformObject::legacy_p
// 2. Let value be an uninitialized variable.
// 3. If operation was defined without an identifier, then set value to the result of performing the steps listed in the interface description to determine the value of an indexed property with index as the index.
// 4. Otherwise, operation was defined with an identifier. Set value to the result of performing the method steps of operation with O as this and « index » as the argument values.
auto value = item_value(index);
auto value = *item_value(index);
// 5. Let desc be a newly created Property Descriptor with no fields.
JS::PropertyDescriptor descriptor;
@ -481,9 +481,9 @@ WebIDL::ExceptionOr<PlatformObject::DidDeletionFail> PlatformObject::delete_valu
VERIFY_NOT_REACHED();
}
JS::Value PlatformObject::item_value(size_t) const
Optional<JS::Value> PlatformObject::item_value(size_t) const
{
return JS::js_undefined();
return {};
}
JS::Value PlatformObject::named_item_value(FlyString const&) const

View file

@ -72,7 +72,7 @@ protected:
};
JS::ThrowCompletionOr<Optional<JS::PropertyDescriptor>> legacy_platform_object_get_own_property(JS::PropertyKey const&, IgnoreNamedProps ignore_named_props) const;
virtual JS::Value item_value(size_t index) const;
virtual Optional<JS::Value> item_value(size_t index) const;
virtual JS::Value named_item_value(FlyString const& name) const;
virtual Vector<FlyString> supported_property_names() const;
virtual bool is_supported_property_name(FlyString const&) const;

View file

@ -227,9 +227,11 @@ bool CSSRuleList::evaluate_media_queries(HTML::Window const& window)
return any_media_queries_changed_match_state;
}
JS::Value CSSRuleList::item_value(size_t index) const
Optional<JS::Value> CSSRuleList::item_value(size_t index) const
{
return item(index);
if (auto value = item(index))
return value;
return {};
}
}

View file

@ -52,7 +52,7 @@ public:
auto end() { return m_rules.end(); }
virtual bool is_supported_property_index(u32 index) const override;
virtual JS::Value item_value(size_t index) const override;
virtual Optional<JS::Value> item_value(size_t index) const override;
WebIDL::ExceptionOr<void> remove_a_css_rule(u32 index);
WebIDL::ExceptionOr<unsigned> insert_a_css_rule(Variant<StringView, CSSRule*>, u32 index);

View file

@ -116,10 +116,10 @@ bool MediaList::matches() const
return false;
}
JS::Value MediaList::item_value(size_t index) const
Optional<JS::Value> MediaList::item_value(size_t index) const
{
if (index >= m_media.size())
return JS::js_undefined();
return {};
return JS::PrimitiveString::create(vm(), m_media[index]->to_string());
}

View file

@ -32,7 +32,7 @@ public:
void delete_medium(StringView);
virtual bool is_supported_property_index(u32 index) const override;
virtual JS::Value item_value(size_t index) const override;
virtual Optional<JS::Value> item_value(size_t index) const override;
bool evaluate(HTML::Window const&);
bool matches() const;

View file

@ -164,10 +164,10 @@ bool StyleSheetList::is_supported_property_index(u32 index) const
return index < m_sheets.size();
}
JS::Value StyleSheetList::item_value(size_t index) const
Optional<JS::Value> StyleSheetList::item_value(size_t index) const
{
if (index >= m_sheets.size())
return JS::js_undefined();
return {};
return m_sheets[index].ptr();
}

View file

@ -36,7 +36,7 @@ public:
size_t length() const { return m_sheets.size(); }
virtual bool is_supported_property_index(u32 index) const override;
virtual JS::Value item_value(size_t index) const override;
virtual Optional<JS::Value> item_value(size_t index) const override;
DOM::Document& document() { return m_document; }
DOM::Document const& document() const { return m_document; }

View file

@ -302,11 +302,11 @@ void DOMTokenList::run_update_steps()
MUST(associated_element->set_attribute(m_associated_attribute, serialize_ordered_set()));
}
JS::Value DOMTokenList::item_value(size_t index) const
Optional<JS::Value> DOMTokenList::item_value(size_t index) const
{
auto string = item(index);
if (!string.has_value())
return JS::js_undefined();
return {};
return JS::PrimitiveString::create(vm(), string.release_value());
}

View file

@ -31,7 +31,7 @@ public:
void associated_attribute_changed(StringView value);
virtual bool is_supported_property_index(u32 index) const override;
virtual JS::Value item_value(size_t index) const override;
virtual Optional<JS::Value> item_value(size_t index) const override;
size_t length() const { return m_token_set.size(); }
Optional<String> item(size_t index) const;

View file

@ -169,11 +169,11 @@ bool HTMLCollection::is_supported_property_index(u32 index) const
return index < length();
}
JS::Value HTMLCollection::item_value(size_t index) const
Optional<JS::Value> HTMLCollection::item_value(size_t index) const
{
auto* element = item(index);
if (!element)
return JS::js_undefined();
return {};
return element;
}

View file

@ -40,7 +40,7 @@ public:
JS::MarkedVector<JS::NonnullGCPtr<Element>> collect_matching_elements() const;
virtual JS::Value item_value(size_t index) const override;
virtual Optional<JS::Value> item_value(size_t index) const override;
virtual JS::Value named_item_value(FlyString const& name) const override;
virtual Vector<FlyString> supported_property_names() const override;
virtual bool is_supported_property_name(FlyString const&) const override;

View file

@ -326,11 +326,11 @@ Attr const* NamedNodeMap::remove_attribute_ns(Optional<FlyString> const& namespa
return attribute;
}
JS::Value NamedNodeMap::item_value(size_t index) const
Optional<JS::Value> NamedNodeMap::item_value(size_t index) const
{
auto const* node = item(index);
if (!node)
return JS::js_undefined();
return {};
return node;
}

View file

@ -26,7 +26,7 @@ public:
virtual bool is_supported_property_index(u32 index) const override;
virtual Vector<FlyString> supported_property_names() const override;
virtual JS::Value item_value(size_t index) const override;
virtual Optional<JS::Value> item_value(size_t index) const override;
virtual JS::Value named_item_value(FlyString const& name) const override;
size_t length() const { return m_attributes.size(); }

View file

@ -25,11 +25,11 @@ void NodeList::initialize(JS::Realm& realm)
WEB_SET_PROTOTYPE_FOR_INTERFACE(NodeList);
}
JS::Value NodeList::item_value(size_t index) const
Optional<JS::Value> NodeList::item_value(size_t index) const
{
auto* node = item(index);
if (!node)
return JS::js_undefined();
return {};
return const_cast<Node*>(node);
}

View file

@ -21,7 +21,7 @@ public:
virtual u32 length() const = 0;
virtual Node const* item(u32 index) const = 0;
virtual JS::Value item_value(size_t index) const override;
virtual Optional<JS::Value> item_value(size_t index) const override;
virtual bool is_supported_property_index(u32) const override;
protected:

View file

@ -56,10 +56,10 @@ bool FileList::is_supported_property_index(u32 index) const
return index < m_files.size();
}
JS::Value FileList::item_value(size_t index) const
Optional<JS::Value> FileList::item_value(size_t index) const
{
if (index >= m_files.size())
return JS::js_undefined();
return {};
return m_files[index].ptr();
}

View file

@ -43,7 +43,7 @@ public:
}
virtual bool is_supported_property_index(u32 index) const override;
virtual JS::Value item_value(size_t index) const override;
virtual Optional<JS::Value> item_value(size_t index) const override;
virtual StringView interface_name() const override { return "FileList"sv; }
virtual WebIDL::ExceptionOr<void> serialization_steps(HTML::SerializationRecord& serialized, bool for_storage, HTML::SerializationMemory&) override;

View file

@ -66,10 +66,10 @@ bool DOMRectList::is_supported_property_index(u32 index) const
return index < m_rects.size();
}
JS::Value DOMRectList::item_value(size_t index) const
Optional<JS::Value> DOMRectList::item_value(size_t index) const
{
if (index >= m_rects.size())
return JS::js_undefined();
return {};
return m_rects[index].ptr();
}

View file

@ -27,7 +27,7 @@ public:
DOMRect const* item(u32 index) const;
virtual bool is_supported_property_index(u32) const override;
virtual JS::Value item_value(size_t index) const override;
virtual Optional<JS::Value> item_value(size_t index) const override;
private:
DOMRectList(JS::Realm&, Vector<JS::NonnullGCPtr<DOMRect>>);

View file

@ -219,9 +219,11 @@ bool HTMLAllCollection::is_supported_property_index(u32 index) const
return index < collect_matching_elements().size();
}
JS::Value HTMLAllCollection::item_value(size_t index) const
Optional<JS::Value> HTMLAllCollection::item_value(size_t index) const
{
return get_the_all_indexed_element(index);
if (auto value = get_the_all_indexed_element(index))
return value;
return {};
}
JS::Value HTMLAllCollection::named_item_value(FlyString const& name) const

View file

@ -34,7 +34,7 @@ public:
JS::MarkedVector<JS::NonnullGCPtr<DOM::Element>> collect_matching_elements() const;
virtual JS::Value item_value(size_t index) const override;
virtual Optional<JS::Value> item_value(size_t index) const override;
virtual JS::Value named_item_value(FlyString const& name) const override;
virtual Vector<FlyString> supported_property_names() const override;
virtual bool is_supported_property_index(u32) const override;

View file

@ -935,11 +935,13 @@ bool HTMLFormElement::is_supported_property_index(u32 index) const
}
// https://html.spec.whatwg.org/multipage/forms.html#dom-form-item
JS::Value HTMLFormElement::item_value(size_t index) const
Optional<JS::Value> HTMLFormElement::item_value(size_t index) const
{
// To determine the value of an indexed property for a form element, the user agent must return the value returned by
// the item method on the elements collection, when invoked with the given index as its argument.
return elements()->item(index);
if (auto value = elements()->item(index))
return value;
return {};
}
// https://html.spec.whatwg.org/multipage/forms.html#the-form-element:supported-property-names

View file

@ -106,7 +106,7 @@ private:
virtual void visit_edges(Cell::Visitor&) override;
// ^PlatformObject
virtual JS::Value item_value(size_t index) const override;
virtual Optional<JS::Value> item_value(size_t index) const override;
virtual JS::Value named_item_value(FlyString const& name) const override;
virtual Vector<FlyString> supported_property_names() const override;
virtual bool is_supported_property_index(u32) const override;

View file

@ -97,11 +97,11 @@ JS::GCPtr<MimeType> MimeTypeArray::named_item(FlyString const& name) const
return nullptr;
}
JS::Value MimeTypeArray::item_value(size_t index) const
Optional<JS::Value> MimeTypeArray::item_value(size_t index) const
{
auto return_value = item(index);
if (!return_value)
return JS::js_null();
return {};
return return_value.ptr();
}

View file

@ -29,7 +29,7 @@ private:
// ^Bindings::PlatformObject
virtual Vector<FlyString> supported_property_names() const override;
virtual JS::Value item_value(size_t index) const override;
virtual Optional<JS::Value> item_value(size_t index) const override;
virtual JS::Value named_item_value(FlyString const& name) const override;
virtual bool is_supported_property_index(u32) const override;
};

View file

@ -120,11 +120,11 @@ JS::GCPtr<MimeType> Plugin::named_item(FlyString const& name) const
return nullptr;
}
JS::Value Plugin::item_value(size_t index) const
Optional<JS::Value> Plugin::item_value(size_t index) const
{
auto return_value = item(index);
if (!return_value)
return JS::js_null();
return {};
return return_value.ptr();
}

View file

@ -35,7 +35,7 @@ private:
// ^Bindings::PlatformObject
virtual Vector<FlyString> supported_property_names() const override;
virtual JS::Value item_value(size_t index) const override;
virtual Optional<JS::Value> item_value(size_t index) const override;
virtual JS::Value named_item_value(FlyString const& name) const override;
virtual bool is_supported_property_index(u32) const override;
};

View file

@ -106,11 +106,11 @@ JS::GCPtr<Plugin> PluginArray::named_item(FlyString const& name) const
return nullptr;
}
JS::Value PluginArray::item_value(size_t index) const
Optional<JS::Value> PluginArray::item_value(size_t index) const
{
auto return_value = item(index);
if (!return_value)
return JS::js_null();
return {};
return return_value.ptr();
}

View file

@ -30,7 +30,7 @@ private:
// ^Bindings::PlatformObject
virtual Vector<FlyString> supported_property_names() const override;
virtual JS::Value item_value(size_t index) const override;
virtual Optional<JS::Value> item_value(size_t index) const override;
virtual JS::Value named_item_value(FlyString const& name) const override;
virtual bool is_supported_property_index(u32) const override;
};

View file

@ -114,7 +114,7 @@ JS_DEFINE_NATIVE_FUNCTION(ConsoleGlobalEnvironmentExtensions::$$_function)
auto array = TRY(JS::Array::create(*vm.current_realm(), node_list->length()));
for (auto i = 0u; i < node_list->length(); ++i) {
TRY(array->create_data_property_or_throw(i, node_list->item_value(i)));
TRY(array->create_data_property_or_throw(i, *node_list->item_value(i)));
}
return array;