Closed Bug 1774894 Opened 2 years ago Closed 2 years ago

nsDisplayTableItem's mPartHasFixedBackground and UpdateForFrameBackground (and mDrawsBackground) seem to be obsolete/dead code

Categories

(Core :: Layout: Tables, defect)

defect

Tracking

()

RESOLVED FIXED
103 Branch
Tracking Status
firefox103 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(1 file)

Following up on a review comment from emilio in https://phabricator.services.mozilla.com/D149337 (which was a downstream refactoring that happened to adjusted a callsite in nsDisplayTableItem::UpdateForFrameBackground) :

  1. It looks to me like nsDisplayTableItem::UpdateForFrameBackground is never called and can be removed.

  2. That means the member-variable it sets, mPartHasFixedBackground, is always false and can be assumed to be such, and hence can be removed.

  3. That means the nsDisplayTableItem's other member-variable, mDrawsBackground, is also unnecessary since it's only used once the following condition:

  if (mDrawsBackground && mPartHasFixedBackground) {

...which we know (per (2)) is just an if (... && false) check and as-such is dead code.

There may be further simplifications to be made as well.

As part of fixing this patch, we should probably double-check that it's OK/expected for this code to all be dead & removed... (Probably it was obsoleted by moving our table painting code to use display lists or something like that?)

(In reply to Daniel Holbert [:dholbert] from comment #0)

As part of fixing this patch, we should probably double-check that it's OK/expected for this code to all be dead & removed... (Probably it was obsoleted by moving our table painting code to use display lists or something like that?)

Yeah, looks like it.

UpdateForFrameBackground and mPartHasFixedBackground were added in bug 416735; and at that point, UpdateForFrameBackground did have several callsites, in nsTableFrame::DisplayGenericTablePart as well as in nsTableCellFrame::BuildDisplayList:
https://hg.mozilla.org/mozilla-central/rev/7c0e2f9dec6f56692f92daf3efcd5385487bb587#l4.106

And:

So this is likely-valid cleanup for left-over code that's cruft as-of bug 929484 & associated work.

Depends on: 929484

This patch doesn't affect behavior; it's dead-code removal.

UpdateForFrameBackground is never called, and hence can be removed.

The member-var that it conditionally set, mPartHasFixedBackground,
can be assumed to be always false, and hence can be removed.

The same struct's other member-var, mDrawsBackground, is irrelevant since it's
only checked in combination with mPartHasFixedBackground, and hence can be
removed.

This all seems to be code that's obsoleted by our (long-ago) move to using
display lists to draw tables.

Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bf46db9b7d3b
Remove unused function nsDisplayTableItem::UpdateForFrameBackground and its associated dead-code. r=emilio
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 103 Branch
You need to log in before you can comment on or make changes to this bug.