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

RESOLVED FIXED in Firefox 47

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: milan, Assigned: milan)

Tracking

Trunk
mozilla47
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.