Bug 230031

Created attachment 437577 [details] Patch

Created attachment 437794 [details] Patch

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

Created attachment 437813 [details] Patch

Created attachment 437815 [details] Patch

Comment on attachment 437815 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437815&action=review > Source/WebCore/css/typedom/CSSNumericValue.cpp:50 > + auto numericValue = CSSNumericValue::rectifyNumberish(WTFMove(value)); Either use this or remove it. > Source/WebCore/css/typedom/CSSNumericValue.cpp:58 > + UNUSED_PARAM(values); You can also just omit "values" above. sub(Vector<CSSNumberish>&&) > Source/WebCore/css/typedom/CSSNumericValue.cpp:75 > +} inconsistent spacing. > Source/WebCore/css/typedom/CSSNumericValue.cpp:91 > +// if (WTF::holds_alternative<RefPtr<CSSNumericValue>>(numberish)) remove > Source/WebCore/css/typedom/CSSNumericValue.cpp:108 > + // FIXME: add impl. This is probably also good to have above. > Source/WebCore/css/typedom/CSSNumericValue.h:47 > + Ref<CSSNumericValue> add(Vector<CSSNumberish>&& values); "values" is probably unnecessary here. It doesn't add any meaning. > Source/WebCore/css/typedom/numeric/CSSMathMax.cpp:50 > +const CSSNumericArray& CSSMathMax::values() const after values() and in header. > Source/WebCore/css/typedom/numeric/CSSMathOperator.h:32 > +enum class CSSMathOperator: uint8_t { nit: space before colon > Source/WebCore/css/typedom/numeric/CSSMathSum.cpp:50 > +const CSSNumericArray& CSSMathSum::values() const after values() > Source/WebCore/css/typedom/numeric/CSSMathValue.h:36 > +enum class CSSMathValueType: uint8_t { space before colon. > Source/WebCore/css/typedom/numeric/CSSNumericBaseType.cpp:28 > +#if ENABLE(CSS_TYPED_OM) You should probably just omit this file, unless you have plans for it. If you do need it, you need to include config.h and CSSNumericBaseType.h outside the ENABLE macro. > Source/WebCore/css/typedom/numeric/CSSNumericBaseType.h:32 > +enum class CSSNumericBaseType { : uint8_t > Source/WebCore/css/typedom/numeric/CSSNumericType.cpp:31 > +#if ENABLE(CSS_TYPED_OM) > + > +#include "config.h" > +#include "CSSNumericType.h" include outside the ENABLE macros.

Created attachment 437920 [details] Patch

Comment on attachment 437920 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437920&action=review > Source/WebCore/css/typedom/CSSNumericValue.cpp:140 > + return Exception { TypeError, "Not implemented Error" }; NotSupportedError > Source/WebCore/css/typedom/numeric/CSSMathInvert.idl:2 > +* Copyright (C) 2018 Apple Inc. All rights reserved. 2021 > Source/WebCore/css/typedom/numeric/CSSMathMax.idl:2 > +* Copyright (C) 2018 Apple Inc. All rights reserved. 2021 Several more times below. Check all the files. > Source/WebCore/css/typedom/numeric/CSSMathOperator.cpp:35 > +namespace WebCore { > + > +} // namespace WebCore Omit this file. > Source/WebCore/css/typedom/numeric/CSSNumericArray.h:44 > + Ref<CSSNumericValue> item(size_t index) { return m_array[index].copyRef(); } This really needs bounds checks. I'm not sure what to return if it's out of bounds. > Source/WebCore/css/typedom/numeric/CSSNumericType.cpp:34 > +namespace WebCore { > +} // namespace WebCore Omit this file.

Created attachment 437924 [details] Patch

Created attachment 437927 [details] Patch

Created attachment 437937 [details] Patch

Created attachment 437939 [details] Patch

Comment on attachment 437939 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=437939&action=review > Source/WebCore/css/typedom/CSSNumericValue.cpp:69 > + extra space > Source/WebCore/css/typedom/numeric/CSSNumericArray.cpp:45 > + Vector<Ref<CSSNumericValue>> values; > + for (auto&& numberish : numberishes) > + values.append(CSSNumericValue::rectifyNumberish(WTFMove(numberish))); This pattern can be replaced by one of two things to become more efficient: 1. WTF::map 2. Something like this: Vector<Ref> values; values.reserveInitialCapacity(numberishes.size()); for (auto&& numbers : numberishes) values.uncheckedAppend(...) Currently, each append needs to check the capacity of the Vector which adds unnecessary branches, and if there is a large number of numberishes then we will reallocate and copy several times. Using these patterns makes it so there is 1 allocation and 0 capacity checks. > Source/WebCore/css/typedom/numeric/CSSNumericType.cpp:34 > +namespace WebCore { > +} // namespace WebCore Please remove this file.

Created attachment 437982 [details] Patch

Created attachment 437984 [details] Patch

Created attachment 437985 [details] Patch

Created attachment 437986 [details] Patch

Created attachment 438000 [details] Patch

Committed r282356 (241619@main): <https://commits.webkit.org/241619@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 438000 [details].

<rdar://problem/83071294>