Status

()

--
critical
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: alice0775, Assigned: jfkthame)

Tracking

({hang, reproducible, testcase})

2.0 Branch
mozilla29
x86_64
All
hang, reproducible, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments)

(Reporter)

Description

5 years ago
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.
(Reporter)

Comment 1

5 years ago
url is gone
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → INCOMPLETE
(Reporter)

Updated

5 years ago
Status: RESOLVED → REOPENED
Resolution: INCOMPLETE → ---

Comment 2

5 years ago
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

Updated

5 years ago
OS: Windows 7 → All
(Reporter)

Comment 3

5 years ago
Created attachment 8351611 [details]
zipped original html
(Reporter)

Comment 4

5 years ago
Created attachment 8351613 [details]
zipped reduced html
(Reporter)

Updated

5 years ago
Component: General → Layout
Product: Firefox → Core
Version: Trunk → 2.0 Branch
(Reporter)

Comment 5

5 years ago
*Firefox3.6 works fine.
*Firefox4 - 9 hangs. However Firefox4 - 9 does not hang if set html5.parser.enable = false.
*Firefox10-Nightly29.0a1 hang.
Keywords: reproducible
Attachment #8351611 - Attachment description: zipped html → zipped original html
Attachment #8351613 - Attachment description: reduced html → zipped reduced html
Created attachment 8351651 [details]
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.
(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. ;)
Keywords: testcase
Flags: needinfo?(jfkthame)
(Assignee)

Comment 8

5 years ago
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.
(Assignee)

Comment 9

5 years ago
Created attachment 8351683 [details] [diff] [review]
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.
Attachment #8351683 - Flags: review?(roc)
(Assignee)

Updated

5 years ago
Assignee: nobody → jfkthame
(Assignee)

Updated

5 years ago
Flags: needinfo?(jfkthame)
(Assignee)

Comment 10

5 years ago
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.
https://hg.mozilla.org/mozilla-central/rev/da02316148fe
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Reproduced the hang in Nightly 2013-12-27 using the testcases.
Verified fixed 29.0a1 2014-01-09, Win 7 x64.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.