Closed Bug 1249157 Opened 6 years ago Closed 6 years ago

Wrap preference hash entry flags into a class, separate them from nsIPrefBranch enums

Categories

(Core :: Preferences: Backend, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: milan, Assigned: milan)

References

Details

Attachments

(1 file, 1 obsolete file)

We are mixing enums from prefapi.h and nsIPrefBranch.idl, using the same names, counting on values being the same.  Make the ones from prefapi to an enum class, so that type checking helps.  Hide the internal values for flags, make it harder for the user to check the wrong thing, or set the type to be both string and int and bool.
This is a first step in attempting larger things with preferences, but it should stand on its own and make things cleaner and safer.
Assignee: nobody → milan
Comment on attachment 8720544 [details]
MozReview Request: Bug 1249157: prefapi enums into class enums, explicit conversion, cleanup.  r=bsmedberg

https://reviewboard.mozilla.org/r/35359/#review32029

::: modules/libpref/prefapi.h:98
(Diff revision 1)
> +  

Whitespace at end of line.

::: modules/libpref/prefapi.h:106
(Diff revision 1)
> +  enum { 

whitespace

::: modules/libpref/prefapi.h:115
(Diff revision 1)
> +  uint16_t mValue;

Did you consider using c++ bitfields instead of an int here? e.g.

uint16_t mType:2;
bool mIsLocked:1;
bool mIsUserSet:1;
bool mIsConfig:1;

I'm not sure it really matters, so I'll leave it up to you, but manual bit-twiddling always worries me.

::: modules/libpref/prefapi.cpp:652
(Diff revision 1)
> -    }
> +        }

I may be being faked out by reviewboard here, but are there extraneous merge markers here?
Attachment #8720544 - Flags: review?(benjamin) → review+
I'll check for the extra merge markers and fix the whitespace.

Bitfield is a good idea.  I left what we had there before, so I'll either create a second patch for this bug or deal with that in another bug.
Comment on attachment 8720544 [details]
MozReview Request: Bug 1249157: prefapi enums into class enums, explicit conversion, cleanup.  r=bsmedberg

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35359/diff/1-2/
Attachment #8720544 - Attachment description: MozReview Request: Bug 1249157: prefapi enums into class enums, explicit conversion, cleanup. r?bsmedberg → MozReview Request: Bug 1249157: prefapi enums into class enums, explicit conversion, cleanup. r=bsmedberg
Thanks.  I pushed the previous version, instead of the one from comment 7.
Flags: needinfo?(milan)
https://hg.mozilla.org/mozilla-central/rev/27f40946fe2d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.