129166 - nsTextFrame may still call linebreaker too much

Status: RESOLVED FIXED

(Core :: Layout, defect, P2)

Description

[Boris Zbarsky [:bzbarsky]]

From bug 98118:

There is another thing that I am not absolutely sure. In case "aIsBreakable" is
true, will we ever suffer in performance in certain situation? If there does
exists such a case, "aLineBreaker->Next" will be called again and again with
expanding buffer. The following patch should fix the problem if it does happen. 
+#define MAX_LOOK_BACK_FOR_WORDBREAK 80
   if(wordLen > 0)
   {
     memcpy((void*)&(aWordBuf[aRunningWordLen]), bp, sizeof(PRUnichar)*wordLen);

     PRUint32 breakP=0;
     PRBool needMore=PR_TRUE;
+    PRUint32 startPos = 0;
+    if (aRunningWordLen > MAX_LOOK_BACK_FOR_WORDBREAK)
+      startPos = aRunningWordLen - MAX_LOOK_BACK_FOR_WORDBREAK;
     nsresult lres = aLineBreaker->Next(aWordBuf, aRunningWordLen+wordLen,
-                                       0, &breakP, &needMore);
+                                       startPos, &breakP, &needMore);
     if(NS_SUCCEEDED(lres))


------------------------------------------------------------------

I'll try to come up with a testcase triggering this.....

Comment 1

[Christine Hoffman]

Changing QA contact

Comment 2

[Chris Petersen]

Changing Priority to P2.

Comment 3

[Frank Tang]

we need more information to tell is this worth it to nsbeta1+ this. When will
this happen ?

Comment 4

[nhottanscp]

nsbeta1- per i18n triage

Comment 5

[Frank Tang]

shanjian is no longer working on mozilla for 2 years and these bugs are still
here. Mark them won't fix. If you want to reopen it, find a good owner first. 

Comment 6

[Travis Chase]

Mass Reassign Please excuse the spam

Comment 7

[Travis Chase]

Mass Re-opening Bugs Frank Tang Closed on Wensday March 02 for no reason, all
the spam is his fault feel free to tar and feather him

Comment 8

[Jungshik Shin]

This is interesting in light of what I'm gonna do in bug 255990

Comment 9

[Jean-Marc Desperrier]

Closing this as the patch doesn't apply anymore to the new textframe.

The new textframe should not ahve the problem :
http://weblogs.mozillazine.org/roc/archives/2007/07/status_3.html
"the new-textframe infrastructure caches the results of line-break analysis in textruns."

Comment 10

[Boris Zbarsky [:bzbarsky]]

Actually, this is just fixed by the new textframe.