1146610 - Figure out whether we should prefer storing the stringified versions of WebIDL enums in cache

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[(no longer active)]

Currently we have a few cases where we store integers corresponding to the internal value of WebIDL enums in the cache database.  I realized this morning that this could be problematic, since now we're effectively relying on these numbers to never change, but adding new values in the WebIDL for example may change them, and this dependency is quite non-obvious.

I suggest going back to storing the string versions of these enum values in the database.  That's still obviously relying on the values to not be removed from the WebIDL, but such a change is probably not very likely since we don't really remove Web visible stuff once we've added them.

Ben, what do you think?

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

It seems like these enums should not be changing very often.  Can we just static_assert the current values with a comment that says the DB schema must be migrated if they change?

Comment 2

[(no longer active)]

Sure, that would prevent against someone accidentally changing the IDL and breaking everyone's caches!  I'll take this.

Comment 3

[(no longer active)]

Attachment: Add static_asserts that check the validity of the enum values that we write into the cache database

These assertions will catch future accidental changes to these enums
which will invalidate the data that we store in the Cache database.

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8582505 [details] [diff] [review]
Add static_asserts that check the validity of the enum values that we write into the cache database

Review of attachment 8582505 [details] [diff] [review]:
-----------------------------------------------------------------

I was thinking if the enums do change, it may be easier just to write a webidl-to-schema map function instead of doing a migration.  This is good for now, though.

Thanks!

Comment 5

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6773b2f30d90

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6773b2f30d90