Closed
Bug 693801
Opened 8 years ago
Closed 8 years ago
nsHTMLReflowState's second constructor has redundant initialization for mFlags.mIsTopOfPage from parent reflow state
Categories
(Core :: Layout, defect)
Core
Layout
Not set
Tracking
()
RESOLVED
FIXED
mozilla10
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
(Whiteboard: [inbound])
Attachments
(1 file)
3.08 KB,
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
nsHTMLReflowState's second constructor looks like this: > 126 nsHTMLReflowState::nsHTMLReflowState(nsPresContext* aPresContext, [...] > 132 bool aInit) > 133 : nsCSSOffsetState(aFrame, aParentReflowState.rendContext) [...] > 136 , mFlags(aParentReflowState.mFlags) > 137 { [...] > 166 mFlags.mIsTopOfPage = aParentReflowState.mFlags.mIsTopOfPage; http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsHTMLReflowState.cpp#126 The mFlags.mIsTopOfPage assignment on line 166 is redundant, since mFlags is already initialized off of aParentReflowState's mFlags. This line was last modified in the patch[1] for bug 113424, which tweaked the boolean member-variable "nsHTMLReflowState::isTopOfPage" to be part of mFlags. Before that patch, this line looked like: > isTopOfPage = aParentReflowState.isTopOfPage; ...and it was necessary because isTopOfPage wasn't initialized elsewhere. But now it gets initialized as part of mFlags, so this line is redundant. [1] http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/layout/generic&command=DIFF_FRAMESET&file=nsHTMLReflowState.cpp&rev2=1.129&rev1=1.128
Assignee | ||
Comment 1•8 years ago
|
||
Patch attached. Including 30 lines of context (back to the mFlags initialization), to demonstrate the redundancy.
Comment 2•8 years ago
|
||
Comment on attachment 566333 [details] [diff] [review] fix r=dbaron, but maybe add a comment near the other flag initializations pointing out that mFlags is initialized above by copy-construction so that we only need to initialize those flags that need a value other than what we get by copying the parent.
Attachment #566333 -
Flags: review?(dbaron) → review+
Assignee | ||
Comment 3•8 years ago
|
||
Added a message as suggested (and added a blank line to set apart the mFlags.* assignments from the other member-variable initializations), and pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/c09b6f933f76
Whiteboard: [inbound]
Assignee | ||
Updated•8 years ago
|
OS: Linux → All
Hardware: x86_64 → All
Target Milestone: --- → mozilla10
Comment 4•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c09b6f933f76
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•