Consider removing nsIFrame::eBlockFrame flag for "IsFrameOfType", in favor of QueryFrame-backed "IsBlockFrameOrSubclass()" method

RESOLVED FIXED in Firefox 67

Status

()

enhancement
P3
normal
RESOLVED FIXED
4 months ago
3 months ago

People

(Reporter: dholbert, Assigned: TYLin)

Tracking

Trunk
mozilla67
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox67 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

4 months ago

For now, IsFrameOfType() is a virtual function call (unless/until we act on bug 1364813). So, calls to IsFrameOfType(nsIFrame::eBlockFrame) are kind of expensive.

In bug 1525628, I'm adding a non-virtual function, nsIFrame::IsBlockFrameOrSubclass(), which should serve the same purpose and which is implemented using do_QueryFrame (which is non-virtual as of bug 1364815).

In theory we should be able to remove nsIFrame::eBlockFrame and migrate its IsFrameOfType() callsites to use IsBlockFrameOrSubclass() instead.

One prerequisite: in its initial version, IsBlockFrameOrSubclass() is a non-const method. To make it correctly "const", we should probably also make the QueryFrame machinery become const-friendly. That might be the first part of this bug, and it'll be necessary to make this IsBlockFrameOrSubclass() function callable at some of our IsFrameOfType() callsites, e.g. this one:

https://searchfox.org/mozilla-central/rev/490ab7f9b84570573a49d7fa018673ce0d5ddf22/layout/generic/ReflowInput.cpp#127-128

(We could also just make IsBlockFrameOrSubclass do a hacky const_cast internally, but it seems like we should attempt to fix QueryFrame first before resorting to that.)

Reporter

Updated

4 months ago
Depends on: 1525628
Reporter

Updated

3 months ago
See Also: → 1527519
Reporter

Updated

3 months ago
Priority: P4 → P3
Assignee

Comment 1

3 months ago

do_QueryFrame becomes more const-friendly in bug 1527519. I think this bug is now doable.

Assignee: nobody → aethanyc
Status: NEW → ASSIGNED
Assignee

Comment 2

3 months ago

Only nsBlockFrame recognizes the eBlockFrame flag, so we can replace the
usage of IsFrameOfType(nsIFrame::eBlockFrame) with either
nsIFrame::IsBlockFrameOrSubclass() or a do_QueryFrame() call for
nsBlockFrame*.

Attachment #9045392 - Attachment description: Bug 1526097 - Remove nsIFrame::eBlockFrame flag. → Bug 1526097 - Remove nsIFrame::eBlockFrame flag. r?dholbert

Comment 3

3 months ago
Pushed by aethanyc@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f3471544869b
Remove nsIFrame::eBlockFrame flag. r=dholbert

Comment 4

3 months ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
You need to log in before you can comment on or make changes to this bug.