36.15 KB, text/html
19.68 KB, text/html
7 bytes, text/html
3.13 KB, patch
|Details | Diff | Splinter Review|
4.46 KB, patch
|Details | Diff | Splinter Review|
See upcoming case. It's a bit messy one, I'm afraid. Basically it consists of <nobr><input type="text" value="a lot of text, also some utf-8"> Loading that page is quite often causing a crash in Mozilla. TB19857249X USP10.dll + 0x3475f (0x74d4475f) UniscribeItem::Shape gfxWindowsTextRun::MeasureOrDrawUniscribe gfxWindowsTextRun::Draw nsThebesFontMetrics::DrawString nsThebesRenderingContext::DrawString nsLayoutUtils::SafeDrawString nsTextFrame::PaintAsciiText 0x03d99020 When it hasn't crashed and then I select some of the garbage in the text input, I crash quite often: TB19857402H USP10.dll + 0x3ab05 (0x74d4ab05) USP10.dll + 0x19a08 (0x74d29a08) USP10.dll + 0x12d44 (0x74d22d44) Uniscribe::Itemize I would expect that cairo builds behave the same as non-cairo builds, the page would load instantly without using 100%cpu, that it would not display garbage in the text input and that it would not crash.
this is the crash I'm seeing.. Not sure whats up with it. It isn't the one you were seeing though.
I can retest when that patch is checked in.
that crash is bug 342922 is it not?
Comment on attachment 228177 [details] [diff] [review] crash fix 1 yeah, that is 342922 apparently
Attachment #228177 - Attachment is obsolete: true
Hmm, currently I crash even with this testcase, it only contains ع It also crashes all older cairo builds. I'm pretty it didn't use to crash my trunk builds.
This doesn't crash anymore, but the performance of selecting something in the input in the first testcase is not good. Also, the text tends to disappear when selecting.
if fx fails to detect the character encoding, fx sometimes crashes. But the crash occurs randomly. This crash is serious...
I think that this crash bug is a bug of Windows. Even if we send the broken string to ScriptShape, it should not crash. But the crash doesn't always occur the same testing. I think that the script cache may be broken sometimes and it makes crash by the testing. If so, we can explain why the crash is random. However of course, I'm not sure. I hope that we can test this patch in nightly build. (Of course, this patch has performance regression.)
Attachment #247160 - Flags: review?(pavlov)
If we can fix this crash by my patch, we can use another way that is caching glyph informations for drawing/measuring in textRun. By we cache it in textRun, we can cut down the count of calling the uniscribe functions.
Comment on attachment 247160 [details] [diff] [review] disable script cache patch I really don't want to disable the script cache. I'd prefer to find some other way to reproduce this so we know what to look out for and _then_ maybe clear the cache.
Comment on attachment 247160 [details] [diff] [review] disable script cache patch I have a feedback that is a crash report with this patch. We need another idea of this cause...
Attachment #247160 - Flags: review?(pavlov) → review-
I found the cause of this bug. If the string has surrogate pair and disabled the shaping, ScriptShape API crashes in sometimes. We should not use "disable shaping".
I'll file a new bug for perf issue. This bug should fix only the crash.
Summary: A case where Mozilla is very slow, show garbage in text input an likely to crash in usp10.dll → crash in usp10.dll(ScriptShape API)
I filed the perf issue in bug 362631.
We have to disable shaping in order for ScriptShape to succeed at all in cases where we don't have an appropriate font to render the text we're trying to render. This isn't going to work either :/
(In reply to comment #17) > We have to disable shaping in order for ScriptShape to succeed at all in cases > where we don't have an appropriate font to render the text we're trying to > render. This isn't going to work either :/ Really? What's the case?
Kimura-san, do you have an idea?
Another approach, how about this?
Attachment #247329 - Flags: review?(pavlov)
Comment on attachment 247329 [details] [diff] [review] Replace the surrogate pair at disabling the shaping ok -- lets get this in for now to stop the crash. I'll ping microsoft and see if we can come up with a better fix.
Attachment #247329 - Flags: review?(pavlov) → review+
checked-in for a2.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
13 years ago
(In reply to comment #20) > Created an attachment (id=247329)  > Replace the surrogate pair at disabling the shaping > Another approach, how about this? | + mAlternativeString = (PRUnichar *)malloc(mLength * sizeof(PRUnichar)); | + memcpy((void *)mAlternativeString, (const void *)mString, | + mLength * sizeof(PRUnichar)); This can cause crash on OOM.
(In reply to comment #23) > (In reply to comment #20) > > Created an attachment (id=247329)  > > Replace the surrogate pair at disabling the shaping > > Another approach, how about this? > > | + mAlternativeString = (PRUnichar *)malloc(mLength * > sizeof(PRUnichar)); > | + memcpy((void *)mAlternativeString, (const void *)mString, > | + mLength * sizeof(PRUnichar)); > > This can cause crash on OOM. > Wow, you're right. I'll file and fix it.(after a business trip).
You need to log in before you can comment on or make changes to this bug.