239262 - [FIXr]Slow rendering with :first-line and missing images

Status: RESOLVED FIXED

(Core :: Layout, defect, P1)

Description

[Rene Pronk]

If I load the testcase from disk rendering takes ages and Mozilla slows down to
a crawl. It becomes very unresponsive but it doesn't really hang. It still
responds to the mouse and you can stop rendering ayntime but it may take a lot
of time.

The problem seems to be caused by the missing image and the use of :first-line.
It almost looks to me as if the page isn't reflown incrementally but completely
over again many times. If the image exists, the page renders in several seconds.

Comment 1

[Rene Pronk]

Attachment: testcase

Comment 2

[Rene Pronk]

Attachment: foo.png

Comment 3

[Boris Zbarsky [:bzbarsky]]

Note: this is spun off of bug 239075

This is a rather interesting situation.  The problem here is that with a broken
image nsImageFrame::Init detects the brokenness and calls HandleLoadError. 
HandleLoadError calls GetPrimaryFrameFor on the content node to find the frame
to call CantHandleReplacedElement on, because the image may be generated by an
<object>.

Now the node doesn't actually have a frame yet (the frame being created is not
in the frame tree), so GetPrimaryFrameFor comes up empty.  The only question is
how long it takes to do this.  When there is a first-line style, each line lives
in a line frame that has the same content node as the div.  So
FindFrameWithContent called with the div frame as parent has to recurse into
every single one of these guys.  The result is that each GetPrimaryFrameFor call
calls FindFrameWithContent O(N) times (on average), instead of O(1) times as it
does without the line frames hanging about.  A profile shows that almost all the
time is in fact spent in these millions of calls to FindFrameWithContent (about
36% of total pageload in it, and nearly 80% under it).  The problem was
exacerbated by the fact that since each tag flush took so long we ended up with
more tag flushes happening overall.

So there are a few separate issues here:

1)  Calling GetPrimaryFrameFor in nsImageFrame.
2)  Content sink timing including the time it takes to reflow. (This may be
    desirable, really).
3)  FindFrameWithContent taking lots of time (one thing that was interesting
    here was that we spent a lot of time getting overflow lists... are frames
    ever in the overflow list but not in the primary frame list?)
4)  The implementation of first-line creates all these line frames.

#2 is pretty easy to fix....

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: We really need a better arch for CantRenderReplacedElement.. :(

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 145173 [details] [diff] [review]
We really need a better arch for CantRenderReplacedElement.. :(

Sure, but can an embed ever have an image frame?  I don't think so.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 145173 [details] [diff] [review]
We really need a better arch for CantRenderReplacedElement.. :(

It can as far as I can tell.  See nsObjectFrame::Init; we don't allow
subdocument loads in an <embed>, but we do allow image loads.

Could this be approved for 1.7?  It's a rather safe fix that can speed up some
pathological cases like this drastically.

Comment 7

[chris hofmann]

Comment on attachment 145173 [details] [diff] [review]
We really need a better arch for CantRenderReplacedElement.. :(

a=chofmann for 1.7

Comment 8

[Boris Zbarsky [:bzbarsky]]

Checked in for 1.7.

David, do you think any of the other items in comment 3 are worth filing bugs on?