Bug 225597 – Make WebCore::HitTestRequest::RequestType an enum class

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Wenson Hsieh]

(...for better type safety, and also because we prefer `enum class` over `enum` in modern code)

[Wenson Hsieh]

Created attachment 428178 [details]
Patch

[Wenson Hsieh]

Comment on attachment 428178 [details]
Patch

Thank you for the review!

[EWS]

Committed r277295 (237556@main): <https://commits.webkit.org/237556@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 428178 [details].

[Radar WebKit Bug Importer]

<rdar://problem/77788970>

[Darin Adler]

Comment on attachment 428178 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=428178&action=review

> Source/WebCore/rendering/HitTestRequest.h:32
> +    enum class Type {

enum class Type : uint16_t?
enum class Type : uint32_t?
Is it better to not specify an underlying type?

[Wenson Hsieh]

(In reply to Darin Adler from comment #5)
> Comment on attachment 428178 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=428178&action=review
> 
> > Source/WebCore/rendering/HitTestRequest.h:32
> > +    enum class Type {
> 
> enum class Type : uint16_t?
> enum class Type : uint32_t?
> Is it better to not specify an underlying type?

Potentially! I don't have strong preferences as to whether or not we should specify an explicit bit width here.

I did this refactoring because I was planning to add another hit test request type in the near future. Currently, there are exactly 16 types, and this new type would require a `uint32_t`.

That said, AFAICT this type flag only represents transient state (usually just stored on the stack when performing a hit test), so I don't think we'll get much of a win from shrinking this enum width.