311378 - freeze/hang in several seconds when a character which does not exist in fonts is rendered

Status: VERIFIED FIXED

(Core Graveyard :: GFX: Win32, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

If we render a character which is not existing on the system fonts, Firefox and
Thunderbird freeze in several seconds(10 or more). But this is occured in first
time only. I cannot reprodue two or more times.
# This problem is occured by spam mail and non-UTF8 encoded URL.

I think that this problem is occured with U+FFFD or PUA characters.
The glyph of U+FFFD is not exiting on many fonts on Windows. And many systems
are not using EUDC characters too.

Comment 1

[Simon Montagu :smontagu]

Is this really All/All? 

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: test case1(EUDC)

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: test case2(�)

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #1)
> Is this really All/All? 

The Japanese tester said. This problem is reproduced on Mac. But other tester
said,  cannot reproduce on Linux. I think this problem is related the count of
fonts. If user's system has many fonts, this freeze may be also long.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I think that this is GFX's performance bug. *But* do we use U+FFFD in converting
Native code to Unicode? And should we support EUDC on default setting in
converting Shift_JIS to Unicode?

Comment 6

[Majken Connor [:Kensie]]

Hmm this might be the bug I'm looking for.  I'll describe, and if it's not the same I'll keep looking.

I've only come across this in xp. When someone has a *lot* of fonts installed (reporters of the problem commonly had an adobe program installed that came with many fonts) and they try to load a page that has an international font that they *don't* have (like japanese) cpu goes to 100% for the csrss.exe process and freezes the browser.  I don't have a lot of fonts myself but I still saw a spike, but not as large and it went away pretty quickly. The problem can actually be seen in about:config if the user scrolls to the fonts entries.

The thread that I figured this out in is here http://forums.mozillazine.org/viewtopic.php?p=1671364#1671364
 that post is around where we started figuring it out.

Is this the same issue?

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Yeah, maybe, does it occur when the "network.IDN.blacklist_chars" or "font.name-list.*" is shown? If so, this is same problem.

Comment 8

[Majken Connor [:Kensie]]

Yes the font.name-list entries are the about:config entries I meant. Like you, I seem to recall the cpu spike in the cases I've seen only happen the first time in a session, but if you restart the browser it happens again.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

The Mac tester go to vacation. We cannot get other report for this problem on Mac.
We have a proposal patch in bugizlla-jp. Therefore, we will fix this bug on Win only. If you can reproduce this problem on non-Win OS, please file a new bug.

Comment 10

[Masatoshi Kimura [:emk]]

Attachment: Patch rv 1.0

timeless:
Could you take a look?

Comment 11

[Masatoshi Kimura [:emk]]

Comment on attachment 205727 [details] [diff] [review]
Patch rv 1.0

timeless:
Could you take a look?

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Curreny performance for font finding score is 7.94277 sec.
I get this score by following patch.
http://bugzilla.mozilla.gr.jp/attachment.cgi?id=3029&action=view

With following patch
http://bugzilla.mozilla.gr.jp/attachment.cgi?id=3030&action=view

the score is 0.487671 sec!!
This is great!!!

Both tests are done immediately after system reboot.
The test environment is:
Athlon64 3700+, Memory 1G * 2(Dual channel),
Win2000, Fonts folder has 470 files.

We want this patch for 1.8.x.
Let's fix it.

Comment 13

[timeless]

Comment on attachment 205727 [details] [diff] [review]
Patch rv 1.0

>Index: gfx/src/windows/nsFontMetricsWin.cpp
>diff -u -8 -p -r3.239 nsFontMetricsWin.cpp
>@@ -2725,69 +2725,120 @@ nsFontMetricsWin::SameAsPreviousMap(int 
>+#ifndef WINCE
>+static
>+void BitFromUnicodeRange(PRUint32 range, DWORD usb[4])
>+{
>+  memset(usb, 0, sizeof(DWORD) * 4);
>+  for (int i = 0, dword = 0; dword < 3; ++dword) {
>+    for (int bit = 0; bit < sizeof(DWORD) * 8; ++bit) {
>+      if (range == gBitToUnicodeRange[i]) {
>+        usb[dword] |= 1 << bit;
>+      }
>+      ++i;

any reason not to do:
+    for (int bit = 0; bit < sizeof(DWORD) * 8; ++bit, ++i) {
?

Comment 14

[Masatoshi Kimura [:emk]]

Attachment: Patch rv 1.1

Comment 15

[rbs]

Comment on attachment 205757 [details] [diff] [review]
Patch rv 1.1

Nice fix. It fits nicely with the spirit of not doing premature optimizations.

+ memset(usb, 0, sizeof(DWORD) * 4);

To increase the readability a little bit, use: memset(usb, 0, sizeof(usb));

Comment 16

[rbs]

Actually, I don't like the usb[4] as argument. I like the following way better:

+static
+void BitFromUnicodeRange(PRUint32 range, DWORD* usb)
+{
+  for (int i = 0, dword = 0; dword < 3; ++dword) {
+    for (int bit = 0; bit < sizeof(DWORD) * 8; ++bit, ++i) {
+      if (range == gBitToUnicodeRange[i]) {
+        usb[dword] |= 1 << bit;
+      }
+    }
+  }
+}

[...]

+  PRUint32 range = (c <= 0xFFFF) ? FindCharUnicodeRange(c) : kRangeSurrogate;
+  DWORD usb[4];
+  memset(usb, 0, sizeof(usb));
...

Comment 17

[Masatoshi Kimura [:emk]]

Attachment: Patch rv 1.2

Integrated rbs' comment.

Comment 18

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

checked-in.

Thank you, Mr.emk!

# unfortunately, today's nightly build has been built.

Comment 19

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 205824 [details] [diff] [review]
Patch rv 1.2

Let's go to 1.8.1 too. We don't have any regression reports. And this patch's performance is so fantastic!

Comment 20

[Stefan Sitter [:ssitter]]

Could the check in for this bug be the cause for Bug 324311 (Profile creation fails on windows if non-ascii characters in profile path)? Works in win32-2005-12-13-07-trunk build, fails in win32-2005-12-14-08-trunk build.

Comment 21

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I don't think so. Because the patch changed only gfx code.

Comment 22

[rbs]

What does this branch‑1.8.1 thing mean? (i.e., how does it differ from approval1.8.1). And why is it assigned to me?

Comment 23

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

rbs:

See http://developer.mozilla.org/devnews/index.php/2006/01/30/tree-rules-for-the-18-branch/

Now, we don't need approval from drivers@m.o for checkin to 1.8 branch. But we need approval from module owner or peers.

Comment 24

[rbs]

Comment on attachment 205824 [details] [diff] [review]
Patch rv 1.2

+'ing for the 1.8 branch. Go for it then.

Comment 25

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

checked-in to 1.8 branch.

Comment 26

[Daniel Veditz [:dveditz]]

How can this bug depend on a WONTFIX bug? (bug 319034). Someone please fix the inconsistency (remove dependency or reopen the other bug)

Comment 27

[Daniel Veditz [:dveditz]]

Comment on attachment 205824 [details] [diff] [review]
Patch rv 1.2

approved for 1.8.0 branch, a=dveditz for drivers

Comment 28

[Masatoshi Kimura [:emk]]

Bug 319034 was proposed as a workaround. It's no longer required to fix the problem.
Removing dependency.

Comment 29

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

-> v.

Comment 30

[tim]

I'm still suffering this problem in FF 1.5.0.3 I am a graphic designer and have approx 4000 fonts installed. Just to note, I have Adobe's Type Manager software installed.
One page that freezes up is on Google. Click on 'About Google', then click on 'Google Labs'. At this point, FF freezes, HDD starts churning away and the csrss.exe process canes the CPU at 100% for a couple of minutes. Then, all is well. Frustrating.

cheers
Tim

Comment 31

[Leonard Mada]

I am still experiencing this bug in the latest Firefox dev-build, and I am fairly convinced that this is the bug, please see:
https://bugzilla.mozilla.org/show_bug.cgi?id=553854

I have still adobe ATM installed on my Win2k system, and probably a fair amount of fonts. However, Seamonkey (latest official build) runs smoothly, so this issue has been introduced recently (in one of the FF dev builds).