953247 - Firefox hang

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Alice0775 White]

stack: 
bp-8db403f1-fb86-4039-b491-871d52131227

Firefox hangs with clean profile as well as safe mode.

Steps To Reproduce:
1 Open http://moeprog.web.fc2.com/moe/Win32API/GUI/ScrollBar.htm

Actual Results:
Firefox hangth, CPU usage is 100%.

Expected Results:
Not hang.
Chrome works well.

Comment 1

[Alice0775 White]

url is gone

Comment 2

[iamjayakumars]

It happens in Mac OS X 10.9 also.

Facing the issues
*Firefox Hangs
*CPU more than 90% 
*More than 1.4GB memory usage, after minimization also.
*Laptop heats when using firefox nightly

Comment 3

[Alice0775 White]

Attachment: zipped original html

Comment 4

[Alice0775 White]

Attachment: zipped reduced html

Comment 5

[Alice0775 White]

*Firefox3.6 works fine.
*Firefox4 - 9 hangs. However Firefox4 - 9 does not hang if set html5.parser.enable = false.
*Firefox10-Nightly29.0a1 hang.

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: backtrace of hang

I loaded the reduced html in a debug build and broke in GDB a few times, and each time it was inside of
 TabWidthStore::ApplySpacing

Specifically, we were in this loop:
===
  // We could binary-search for the first record that falls within the range,
  // but as the number of tabs is normally small and we usually process them
  // sequentially from the beginning of the line, it doesn't seem worth doing
  // at this point.
  for (uint32_t i = 0; i < mWidths.Length(); ++i) {
    TabWidth& tw = mWidths[i];
    if (tw.mOffset < aOffset) {
      continue;
    }
    if (tw.mOffset - aOffset >= aLength) {
      break;
    }
    aSpacing[tw.mOffset - aOffset].mAfter += tw.mWidth;
  }
===
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsTextFrame.cpp#124

...and despite the comment, the array we're iterating over is actually quite large. The last time I broke before posting this comment, I checked mWidths.Length() and got 434400.

Comment 7

[Daniel Holbert [:dholbert]]

(and "i" was 382723, so we're iterating over a large part of that array)

CC'ing jfkthame who has hg blame on that "normally small" code-comment quoted above. ;)

Comment 8

[Jonathan Kew [:jfkthame]]

Well, it -is- normally small! :P

The "problem page" here has a couple of lines within a <pre> element with over 4 million TAB characters in each line. That's not "normal", it's clearly a problem of some kind on the authoring side. There's no good reason for those tabs to be there, and they're -massively- bloating the size of the HTML page. (Maybe someone's cat fell asleep while leaning on the TAB key?)

Still, I guess we should try to do better here. As the comment suggests, a binary search would help.

Comment 9

[Jonathan Kew [:jfkthame]]

Attachment: use binary search instead of linear scan to find tab-width records

Something like this should help with such pathological cases. With this patch, the testcase loads in reasonable time even in my debug build. Pushed to tryserver: https://tbpl.mozilla.org/?tree=Try&rev=5e0f3a65552b.

Comment 10

[Jonathan Kew [:jfkthame]]

Comment on attachment 8351683 [details] [diff] [review]
use binary search instead of linear scan to find tab-width records

Review of attachment 8351683 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsTextFrame.cpp
@@ +142,5 @@
> +        continue;
> +      }
> +      if (tw.mOffset > aOffset) {
> +        // mWidths[i] is within (or beyond) the target range;
> +        // new search range is [lo, hi). If it turns out that

Uh, that should of course read [lo, i). Fixed locally.

Comment 11

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/da02316148fe

Comment 12

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/da02316148fe

Comment 13

[Paul Silaghi, QA [:pauly]]

Reproduced the hang in Nightly 2013-12-27 using the testcases.
Verified fixed 29.0a1 2014-01-09, Win 7 x64.