Right now it assigns to members haphazardly in the body of the constructor. In bug 1386472 I found by accident that mDesiredPosSet is left uninitialized for example, and I haven't checked everything else.

I'll prefer per-member defaults because it's easier to read and maintain.

(In reply to Ting-Yu Lin [:TYLin] (UTC+8) from comment #1) > I'll prefer per-member defaults because it's easier to read and maintain. Me too. Patches on the way.

Comment on attachment 8929371 [details] Bug 1387176 - switch nsFrameSelection to use per-member defaults. https://reviewboard.mozilla.org/r/200696/#review205910 ::: layout/generic/nsFrameSelection.h:741 (Diff revision 1) > - nsCOMPtr<nsIContent> mStartSelectedCell; > - nsCOMPtr<nsIContent> mEndSelectedCell; > - nsCOMPtr<nsIContent> mAppendStartSelectedCell; > - nsCOMPtr<nsIContent> mUnselectCellOnMouseUp; > + nsCOMPtr<nsINode> mCellParent = nullptr; > + nsCOMPtr<nsIContent> mStartSelectedCell = nullptr; > + nsCOMPtr<nsIContent> mEndSelectedCell = nullptr; > + nsCOMPtr<nsIContent> mAppendStartSelectedCell = nullptr; > + nsCOMPtr<nsIContent> mUnselectCellOnMouseUp = nullptr; nsCOMPtr and RefPtr will use their default constructors to do initializtion, so it's fine to leave them as they were. But it's no harm to explicit initialize them to `nullptr` either. ::: layout/generic/nsFrameSelection.h:761 (Diff revision 1) > // Limit selection navigation to a child of this node. > - nsCOMPtr<nsIContent> mLimiter; > + nsCOMPtr<nsIContent> mLimiter = nullptr; > // Limit selection navigation to a descendant of this node. > - nsCOMPtr<nsIContent> mAncestorLimiter; > + nsCOMPtr<nsIContent> mAncestorLimiter = nullptr; > > - nsIPresShell *mShell; > + nsIPresShell *mShell = nullptr; Nit: While you're here, please associate the star with `nsIPresShell`, i.e. `nsIPressShell* mShell`. ::: layout/generic/nsFrameSelection.h:772 (Diff revision 1) > // Hint to tell if the selection is at the end of this line or beginning of next. > CaretAssociateHint mHint = mozilla::CARET_ASSOCIATE_BEFORE; > nsBidiLevel mCaretBidiLevel = BIDI_LEVEL_UNDEFINED; > nsBidiLevel mKbdBidiLevel = NSBIDI_LTR; > > - nsPoint mDesiredPos; > + nsPoint mDesiredPos = nsPoint(0, 0); Ditto. `nsPoint` should defaults to `(0, 0)`, but it's OK to explicitly initialize it.

Comment on attachment 8929370 [details] Bug 1387176 - move member initialization from the constructor to class definition for nsFrameSelection. https://reviewboard.mozilla.org/r/200694/#review205912

Comment on attachment 8929371 [details] Bug 1387176 - switch nsFrameSelection to use per-member defaults. https://reviewboard.mozilla.org/r/200696/#review205910 > Nit: While you're here, please associate the star with `nsIPresShell`, i.e. `nsIPressShell* mShell`. Ok, thank you for the review. :)

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fb7f11535cdd892f8c02de29d879176f81f9b6f4

Pushed by jichen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/27f247440ce7 move member initialization from the constructor to class definition for nsFrameSelection. r=TYLin https://hg.mozilla.org/integration/autoland/rev/8053ce4e6ab8 switch nsFrameSelection to use per-member defaults. r=TYLin

https://hg.mozilla.org/mozilla-central/rev/27f247440ce7 https://hg.mozilla.org/mozilla-central/rev/8053ce4e6ab8