Bug 1526097 Comment 0 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Original comment by
on 
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 uses QueryFrame (which is non-virtual as of bug 1364815).

In theory we should be able to remove nsIFrame::eBlockFrame and migrate IsFrameOfType() calls 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 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.)
Revision 1 by
on 
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 IsFrameOfType() calls 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 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.)
Revision 2 by
on 
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 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.)
Revision 3 by
on 
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.)
Revision 4 by
on 
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.)

Back to Bug 1526097 Comment 0