Bug 45081 - Crash rendering <meter/> with percent padding
Summary: Crash rendering <meter/> with percent padding
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: James Kozianski
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-01 19:09 PDT by James Kozianski
Modified: 2011-01-18 22:01 PST (History)
6 users (show)

See Also:

Attachments
Reproduction (31 bytes, text/html)
2010-09-01 19:09 PDT, James Kozianski 		no flags Details
Stack trace (1.56 KB, text/plain)
2010-09-01 20:46 PDT, James Kozianski 		no flags Details
Proposed patch (3.25 KB, patch)
2010-09-02 01:22 PDT, James Kozianski 		tkent: review-
Details | Formatted Diff | Diff
Proposed patch (3.26 KB, patch)
2010-09-02 01:51 PDT, James Kozianski 		abarth: review-
Details | Formatted Diff | Diff
Proposed patch (4.08 KB, patch)
2010-09-06 21:29 PDT, James Kozianski 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description James Kozianski 2010-09-01 19:09:44 PDT 
Created attachment 66312 [details]
Reproduction

WebKit crashes when trying to render a <meter> element that has percentage padding, eg: <meter style="padding: 1%;" />

Test case attached.
Comment 1 James Kozianski 2010-09-01 20:46:18 PDT 
Created attachment 66318 [details]
Stack trace
Comment 2 James Kozianski 2010-09-01 21:22:24 PDT 
The crash is caused by a null dereference of containingBlock() in RenderBoxModelObject::paddingTop().

RenderMeter queries its size when determining whether it needs to be layed out, but initially it has no containing block, hence the null dereference.

I'll write a patch to make RenderMeter always request layout so this query never occurs.
Comment 3 James Kozianski 2010-09-02 01:22:10 PDT 
Created attachment 66338 [details]
Proposed patch
Comment 4 WebKit Review Bot 2010-09-02 01:24:17 PDT 
Attachment 66338 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Hajime Morrita 2010-09-02 01:47:35 PDT 
Comment on attachment 66338 [details]
Proposed patch

Hi, thank you for doing this! 
The change looks OK in general. Please fix what the bot claims.
You can use WebKitTools/Scripts/check-webkit-style
to correct coding convention errors before the bot bites us.

For the test, are we OK even for other than padding?
Having size-based properties like margin, width, height would be helpful.

For ChangeLog, please mention what caused the crash briefly.
Comment 6 James Kozianski 2010-09-02 01:51:35 PDT 
Created attachment 66339 [details]
Proposed patch
Comment 7 Kent Tamura 2010-09-02 01:52:03 PDT 
Comment on attachment 66338 [details]
Proposed patch

r- for the style error.
Comment 8 Kent Tamura 2010-09-05 23:32:45 PDT 
The second patch seems not to answer Morita-san's requests.

> For the test, are we OK even for other than padding?
> Having size-based properties like margin, width, height would be helpful.
> 
> For ChangeLog, please mention what caused the crash briefly.
Comment 9 Adam Barth 2010-09-05 23:43:39 PDT 
Comment on attachment 66339 [details]
Proposed patch

See comment above.
Comment 10 James Kozianski 2010-09-06 21:29:27 PDT 
Created attachment 66684 [details]
Proposed patch
Comment 11 Hajime Morrita 2010-09-07 00:49:30 PDT 
Looks fine for me.
Comment 12 Kent Tamura 2010-09-07 00:50:21 PDT 
Comment on attachment 66684 [details]
Proposed patch

OK.
Comment 13 WebKit Commit Bot 2010-09-07 01:12:45 PDT 
Comment on attachment 66684 [details]
Proposed patch

Clearing flags on attachment: 66684

Committed r66864: <http://trac.webkit.org/changeset/66864>
Comment 14 WebKit Commit Bot 2010-09-07 01:12:49 PDT 
All reviewed patches have been landed.  Closing bug.