467150 - nsContinuingTextFrame::Destroy doesn't need to clear the textrun if the frame is empty

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, defect, P3)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

See bug 455826. There's a reviewed patch there; the nsContinuingTextFrame::Destroy part needs to be landed.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Pushed to trunk, aaeb20c61fca

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: patch that landed on trunk

Performance win on bug 455826. Should be safe after it's baked on trunk a bit.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I backed this out to try to fix Windows random mochitest timeouts

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This was the cause of the random orange.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: revised patch

The random crashes were because in some cases, the textrun's userdata was referring to an nsContinuingTextFrame which had been deleted, so when we came to expire the textrun after 30 seconds, we looked at a deleted frame in ClearAllTextRunReferences. Now, most of the time we got to "if (aFrame->GetTextRun() != aTextRun)" and read some garbage value in aFrame->mTextRun and aborted safely. It seems only on Windows that memory has sometimes actually been unmapped and so we hit a seg fault 30 seconds after the test that actually triggered the bug.

So this patch sets the TEXT_IN_TEXTRUN_USER_DATA flag on every text frame that is mentioned in a textrun's userdata, and we always flush a textrun if we destroy a frame that's mentioned in its userdata. Also, this patch adds an assertion to ClearAllTextRunReferences that calls GetType on the frame, which means that if the frame has been deleted we will almost certainly crash on all platforms (and if not, we will almost certainly fail the assertion).

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 351037 [details] [diff] [review]
patch that landed on trunk

(clearing nom)

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Pushed 84cffcb0f3da to trunk.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 351493 [details] [diff] [review]
revised patch

Performance improvement related to bug 430332

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 351493 [details] [diff] [review]
revised patch

a191=beltzner

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Pushed e7d1975fdecb to 1.9.1

Comment 11

[Daniel Holbert [:dholbert]]

I just landed "revised patch" from here on 1.9.0.x, to fix bug 489647.