Skip to content

script: Use CryptoAlgorithm in name fields of subtle dictionaries#43055

Merged
TimvdLippe merged 1 commit into
servo:mainfrom
kkoyung:dictionary-name-enum
Mar 6, 2026
Merged

script: Use CryptoAlgorithm in name fields of subtle dictionaries#43055
TimvdLippe merged 1 commit into
servo:mainfrom
kkoyung:dictionary-name-enum

Conversation

@kkoyung
Copy link
Copy Markdown
Member

@kkoyung kkoyung commented Mar 6, 2026

We currently use String to store cryptographic algorithm name in subtle dictionaries in WebCrypto code. The algorithm normalization guarantees that there are only a limited number of possible strings that can be assigned to the name fields.

We switch to use the CryptoAlgorithm enum, which contains all possible algorithm names, to represent these values. This reduces string comparison and saves a small amount of memory.

The constant string ALG_* in subtlecrypto.rs, which are the recognized algorithm name for the algorithms, are also removed since they are already embedded in the CryptoAlgorithm enum.

Testing: Refactoring. Existing tests suffice.
Fixes: Part of #42579

We currently use `String` to store cryptographic algorithm name in
subtle dictionaries in WebCrypto code. The algorithm normalization
guarantees that there are only a limited number of possible strings that
can be assigned to the name fields.

We switch to use the `CryptoAlgorithm` enum, which contains all possible
algorithm names, to represent these values. This reduces string
comparison and saves a small amount of memory.

The constant string `ALG_*` in `subtlecrypto.rs`, which are the
recognized algorithm name for the algorithms, are also removed since
they are already embedded in the `CryptoAlgorithm` enum.

Testing: Refactoring. Existing tests suffice.
Fixes: Part of 42579

Signed-off-by: Kingsley Yung <[email protected]>
@kkoyung kkoyung requested a review from gterzian as a code owner March 6, 2026 07:00
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Mar 6, 2026
Copy link
Copy Markdown
Contributor

@TimvdLippe TimvdLippe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a lot of refactoring, but given its simplicity I will approve. Trusting on both the compiler and WPT that no mistakes were introduced. Nice cleanup!

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Mar 6, 2026
@TimvdLippe TimvdLippe added this pull request to the merge queue Mar 6, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Mar 6, 2026
Merged via the queue into servo:main with commit 8df3819 Mar 6, 2026
33 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Mar 6, 2026
@kkoyung kkoyung deleted the dictionary-name-enum branch March 7, 2026 03:08
offline-ant pushed a commit to offline-ant/havi that referenced this pull request Jun 4, 2026
…ervo#43055)

We currently use `String` to store cryptographic algorithm name in
subtle dictionaries in WebCrypto code. The algorithm normalization
guarantees that there are only a limited number of possible strings that
can be assigned to the name fields.

We switch to use the `CryptoAlgorithm` enum, which contains all possible
algorithm names, to represent these values. This reduces string
comparison and saves a small amount of memory.

The constant string `ALG_*` in `subtlecrypto.rs`, which are the
recognized algorithm name for the algorithms, are also removed since
they are already embedded in the `CryptoAlgorithm` enum.

Testing: Refactoring. Existing tests suffice.
Fixes: Part of servo#42579

Signed-off-by: Kingsley Yung <[email protected]>
(cherry picked from commit 8df3819)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants