635329 - Crash [@ nsBidiPresUtils::RemoveBidiContinuation] with large margin, changing text node

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Jesse Ruderman]

Attachment: testcase (crashes Firefox when loaded)

Comment 1

[Jesse Ruderman]

Nominating for blocking because this might be the same as a topcrash with the same signature (bug 632828).

Comment 2

[Jesse Ruderman]

Attachment: stack trace (mac)

Comment 3

[Simon Montagu :smontagu]

This turns out to be caused by bug 597627 deleting frames during bidi resolution. I think that we won't be losing much if we just don't do that optimization when SetLength is called from AdjustOffsetsForBidi, since bidi resolution blows away text runs for all frames it touches anyway. Mats, does that make sense?

Comment 4

[Simon Montagu :smontagu]

Attachment: Patch

Comment 5

[Mats Palmgren (inactive)]

Comment on attachment 513850 [details] [diff] [review]
Patch

Yes, this looks good to me.
Nit: prefix the parameter with "a"

Comment 6

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

Comment on attachment 513850 [details] [diff] [review]
Patch

+                 PRBool inBidiResolution = PR_FALSE);

I think you should just have a flags word with a flag "ALLOW_FRAME_CREATION_DESTRUCTION", and the default (0) should not allow it.

Comment 7

[Simon Montagu :smontagu]

Attachment: Addressed comments

Comment 8

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

Comment on attachment 513896 [details] [diff] [review]
Addressed comments

+#define ALLOW_FRAME_CREATION_DESTRUCTION 1

Hmm, might as well make it ALLOW_FRAME_CREATION_AND_DESTRUCTION :-). Also, make it 0x01. Also, should be an enum in nsTextFrame.h next to SetLength, i.e.
  enum { ALLOW_FRAME_CREATION_AND_DESTRUCTION = 0x01 };

Comment 9

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

Comment on attachment 513896 [details] [diff] [review]
Addressed comments

This should be very safe since it just enables selective non-use of the optimizations added in bug 597627.

Comment 10

[Simon Montagu :smontagu]

http://hg.mozilla.org/mozilla-central/rev/3db6693e136e