Bug 108279 – webdatabase: Introducing WTF::TypeSafeEnum and DatabaseError

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

[Mark Lam]

This is a sub-task of https://bugs.webkit.org/show_bug.cgi?id=107475.  Breaking out as a step for easier review.  This patch introduces the WTF TypeSafeEnum template and the DatabaseError enum list which instantiates an instance of TypeSafeEnum.  The enums in DatabaseError is currently unused, but will be put to use later in bug 107475.

[Mark Lam]

Created attachment 185386 [details]
the patch.

[Geoffrey Garen]

Comment on attachment 185386 [details]
the patch.

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

r=me

> Source/WTF/wtf/TypeSafeEnum.h:51
> +//     MyEnum value1; // value1 is assigned MyEnum::VALUE_DEFAULT by default.

Typo: MyEnumDef.

> Source/WebCore/Modules/webdatabase/DatabaseError.h:35
> +struct DatabaseErrorDef {

We don't usually use abbreviations like "Def". How about calling this "DatabaseErrorStruct" or "DatabaseErrorDefinition"?

[Mark Lam]

Comment on attachment 185386 [details]
the patch.

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

>> Source/WTF/wtf/TypeSafeEnum.h:51
>> +//     MyEnum value1; // value1 is assigned MyEnum::VALUE_DEFAULT by default.
> 
> Typo: MyEnumDef.

This is not a typo.  With the TypeSafeEnum template, you can say MyEnum::VALUE_DEFAULT and it gets redirected to MyEnumDef::VALUE_DEFAULT.

... which reminds me, I should change that to MyEnum::ValueDefault which is what the style checker says is preferred.  I will update before committing.

>> Source/WebCore/Modules/webdatabase/DatabaseError.h:35
>> +struct DatabaseErrorDef {
> 
> We don't usually use abbreviations like "Def". How about calling this "DatabaseErrorStruct" or "DatabaseErrorDefinition"?

Will change to DatabaseErrorDefinition.

[Benjamin Poulain]

C++ 11 has strongly typed enum.

Can't we use that and people without C++ 11 would have weak typed enums? That would make the code simpler.

[Mark Lam]

(In reply to comment #4)
> C++ 11 has strongly typed enum.
> 
> Can't we use that and people without C++ 11 would have weak typed enums? That would make the code simpler.

The code is not all that complicated, and the type safety actually has an added benefit of scoping e.g. DatabaseError::None.  Otherwise, I will have to add a prefix.

I'm landing this for now, and will look into how we can abstract this to use C++11 enums if available later.

[Benjamin Poulain]

> The code is not all that complicated, and the type safety actually has an added benefit of scoping e.g. DatabaseError::None.  Otherwise, I will have to add a prefix.

enum class are scoped.

> I'm landing this for now, and will look into how we can abstract this to use C++11 enums if available later.

I looked into supporting both but that makes things worse. :(
Ignore me, go ahead with the patch.

[Mark Lam]

Landed in r141219: <http://trac.webkit.org/changeset/141219>.

[Benjamin Poulain]

Reopening to attach new patch.

[Benjamin Poulain]

Created attachment 185390 [details]
Patch

[Benjamin Poulain]

Comment on attachment 185390 [details]
Patch

sorry, wrong copy paste :(