Closed Bug 1146610 Opened 9 years ago Closed 9 years ago

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


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

Not set



Tracking Status
firefox39 --- fixed


(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)




(1 file)

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?
Flags: needinfo?(bkelly)
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?
Flags: needinfo?(bkelly)
Sure, that would prevent against someone accidentally changing the IDL and breaking everyone's caches!  I'll take this.
Assignee: nobody → ehsan
These assertions will catch future accidental changes to these enums
which will invalidate the data that we store in the Cache database.
Attachment #8582505 - Flags: review?(bkelly)
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.

Attachment #8582505 - Flags: review?(bkelly) → review+
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.