| Summary: | REGRESSION (r82580): Reproducible crash in CSSPrimitiveValue::computeLengthDouble | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | David Barr <davidbarr> | ||||||||
| Component: | CSS | Assignee: | David Barr <davidbarr> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | ap, davidbarr, eric, evan, koivisto, leviw, macpherson, mikelawther, mitz, parag, simon.fraser, webkit.review.bot | ||||||||
| Priority: | P1 | Keywords: | Regression | ||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| URL: | http://crbug.com/83633 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
David Barr
2011-06-02 20:18:33 PDT
Created attachment 95848 [details]
Full stack trace from chromium crash
Crashes WebKit Nightly (r87828) and Chrome Canary (13.0.782.3 canary) on Mac Confirmed using r87633. Does not crash Safari 5.0.5. Exception Type: EXC_BAD_ACCESS (SIGSEGV) Exception Codes: KERN_INVALID_ADDRESS at 0x0000000000000040 Crashed Thread: 0 Dispatch queue: com.apple.main-thread Thread 0 Crashed: Dispatch queue: com.apple.main-thread 0 com.apple.WebCore 0x000000010381e3e4 WTF::RefPtr<WebCore::StyleInheritedData>::get() const + 12 (RefPtr.h:60) 1 com.apple.WebCore 0x00000001038ad2c9 WebCore::DataRef<WebCore::StyleInheritedData>::get() const + 21 (DataRef.h:33) 2 com.apple.WebCore 0x00000001038ad2e1 WebCore::DataRef<WebCore::StyleInheritedData>::operator->() const + 21 (DataRef.h:36) 3 com.apple.WebCore 0x00000001038ad2fd WebCore::RenderStyle::fontDescription() const + 25 (RenderStyle.h:479) 4 com.apple.WebCore 0x000000010388bcb4 WebCore::CSSPrimitiveValue::computeLengthDouble(WebCore::RenderStyle*, WebCore::RenderStyle*, double, bool) + 330 (CSSPrimitiveValue.cpp:319) 5 com.apple.WebCore 0x000000010388bee4 WebCore::CSSPrimitiveValue::computeLengthIntForLength(WebCore::RenderStyle*, WebCore::RenderStyle*, double) + 52 (CSSPrimitiveValue.cpp:271) 6 com.apple.WebCore 0x00000001038b6486 WebCore::ApplyPropertyLength<(WebCore::LengthAuto)1, (WebCore::LengthIntrinsic)0, (WebCore::LengthMinIntrinsic)0, (WebCore::LengthNone)0, (WebCore::LengthUndefined)0>::applyValue(WebCore::CSSStyleSelector*, WebCore::CSSValue*) const + 504 (CSSStyleApplyProperty.cpp:232) 7 com.apple.WebCore 0x00000001038e3a79 WebCore::CSSStyleApplyProperty::applyValue(WebCore::CSSPropertyID, WebCore::CSSStyleSelector*, WebCore::CSSValue*) const + 131 (CSSStyleApplyProperty.h:68) ... The bug seems to apply to any property that references m_rootElementStyle, eg <html hidden><title style="line-height: 1rem;">a</title></html> also causes a crash. In eliminating possible culprits, we've confirmed the crash occurs on MacOS as far back as Webkit r85429 (using r85429.dmg">http://builds.nightly.webkit.org/files/trunk/mac/WebKit-SVN-r85429.dmg). I did a binary search of chromium continuous build revisions using the supplied test case. The first breakage was between chromium revisions 79962 and 80134. These correspond to webkit revisions 82507 and 82657 respectively. Just to give some background: - when we see a <title> tag, we attempt to compute the text of the tag - when we compute the text, we use the computed style to see if it's LTR or RTL - this 'hidden' attr is probably making some style-related computation fail Created attachment 124539 [details]
Proposed Patch
Added a null check for RenderStyle of root element while calculating length for css3 rems unit
Comment on attachment 124539 [details] Proposed Patch View in context: https://bugs.webkit.org/attachment.cgi?id=124539&action=review > Source/WebCore/ChangeLog:9 > + of the root element.In this case as HTML element has a property 'hidden' and hence renderer is NULL Space after . please. > Source/WebCore/ChangeLog:12 > + Test: fast/css/bug61989-fontsize-unit-rems-crash.html You don't really need the bug number in the filename. > LayoutTests/fast/css/bug61989-fontsize-unit-rems-crash.html:1 > +<html hidden><title style="line-height: 1rem;">Test case for 61989</title> The hidden attribute seems contentious <http://peter.sh/2010/06/thoughts-on-the-html5-hidden-attribute/> Does the bug still reproduce with display:none? That might be a better way to test it. Created attachment 124544 [details]
Updated Patch
Updated patch with all review comments implemented
Comment on attachment 124544 [details] Updated Patch Clearing flags on attachment: 124544 Committed r106251: <http://trac.webkit.org/changeset/106251> All reviewed patches have been landed. Closing bug. (In reply to comment #12) > All reviewed patches have been landed. Closing bug. Is there any reason we're not using dumpAsText on the fast/css/fontsize-unit-rems-crash.html layout test? > Is there any reason we're not using dumpAsText on the fast/css/fontsize-unit-rems-crash.html layout test?
yes, Actually when i was using dumpAsText on fast/css/fontsize-unit-rems-crash.html it was dumping all the content of the html page including the script as the test case has html property hidden.
(In reply to comment #14) > > Is there any reason we're not using dumpAsText on the fast/css/fontsize-unit-rems-crash.html layout test? > > yes, Actually when i was using dumpAsText on fast/css/fontsize-unit-rems-crash.html it was dumping all the content of the html page including the script as the test case has html property hidden. Is this an issue for test coverage? > Is this an issue for test coverage? I am not sure if this is an issue with dumprendertree but when i was using it with this test case i was getting the expected output as Test case for 61989 This is test for Bug 61989 No crash means test PASS.if (window.layoutTestController) layoutTestController.dumpAsText(); which includes the title text and script content as well. |