Bug 93747

Summary: Reduce the size of empty NinePieceImage objects.
Product: WebKit Reporter: Andreas Kling <kling>
Component: Layout and RenderingAssignee: Andreas Kling <kling>
Severity: Normal CC: eric, koivisto, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Description Flags
A patch apart
koivisto: review+
Patch for moon landing none

Description Andreas Kling 2012-08-10 15:25:43 PDT
Patch incoming.
Comment 1 Andreas Kling 2012-08-10 15:27:36 PDT
Created attachment 157819 [details]
A patch apart
Comment 2 WebKit Review Bot 2012-08-10 15:30:24 PDT
Attachment 157819 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/rendering/style/NinePieceImage.h:61:  The parameter name "o" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/rendering/style/NinePieceImage.h:95:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 2 in 3 files

If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Antti Koivisto 2012-08-10 16:45:01 PDT
Comment on attachment 157819 [details]
A patch apart

View in context: https://bugs.webkit.org/attachment.cgi?id=157819&action=review

> Source/WebCore/rendering/style/NinePieceImage.h:37
> -class NinePieceImage {
> +class NinePieceImageData {

I think it would be better to use the normal RenderStyle DataRef<>/SET_VAR pattern here. The advantage is that we can share the structure in cases where it already exists and some other surround property is modified. It is also more consistent.

We should also have separate version of SET_VAR for these two-level data structure cases as currently we end up creating unnecessary structures when a variable is set to the current value.
Comment 4 Antti Koivisto 2012-08-13 13:06:56 PDT
Comment on attachment 157819 [details]
A patch apart

Actually DataRefs in substructures don't make sense for non-inherited data as there is never anything to share (except the default value). Forgot what I mumbled above, the patch is fine.
Comment 5 Antti Koivisto 2012-08-13 13:09:11 PDT
It might be good idea to see if NinePieceImages are so rare compared to other properties in SurroundData it should go to rare data instead.
Comment 6 Andreas Kling 2012-08-13 14:13:27 PDT
Created attachment 158101 [details]
Patch for moon landing
Comment 7 WebKit Review Bot 2012-08-13 15:45:05 PDT
Comment on attachment 158101 [details]
Patch for moon landing

Clearing flags on attachment: 158101

Committed r125465: <http://trac.webkit.org/changeset/125465>
Comment 8 WebKit Review Bot 2012-08-13 15:45:08 PDT
All reviewed patches have been landed.  Closing bug.