Bug 108707 - Remove duplicate code in RenderBoxModelObject::computedCSSPadding*
Summary: Remove duplicate code in RenderBoxModelObject::computedCSSPadding*
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Emil A Eklund
URL:
Keywords:
Depends on: 108728
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-01 15:19 PST by Emil A Eklund
Modified: 2013-02-04 09:46 PST (History)
7 users (show)

See Also:


Attachments
Patch (7.18 KB, patch)
2013-02-01 15:20 PST, Emil A Eklund
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Emil A Eklund 2013-02-01 15:19:17 PST
The computedCSSPaddingTop/Bottom/... methods in RenderBoxModelObject all do pretty much exactly the same thing yet share no code.
Comment 1 Emil A Eklund 2013-02-01 15:20:15 PST
Created attachment 186160 [details]
Patch
Comment 2 Levi Weintraub 2013-02-01 15:31:20 PST
Comment on attachment 186160 [details]
Patch

Hallelujah!
Comment 3 WebKit Review Bot 2013-02-01 17:11:37 PST
Comment on attachment 186160 [details]
Patch

Clearing flags on attachment: 186160

Committed r141669: <http://trac.webkit.org/changeset/141669>
Comment 4 WebKit Review Bot 2013-02-01 17:11:40 PST
All reviewed patches have been landed.  Closing bug.
Comment 5 Jessie Berlin 2013-02-01 17:22:56 PST
(In reply to comment #3)
> (From update of attachment 186160 [details])
> Clearing flags on attachment: 186160
> 
> Committed r141669: <http://trac.webkit.org/changeset/141669>

This broke the Windows build:

http://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/61773/steps/compile-webkit/logs/stdio

7>LINK : warning LNK4075: ignoring '/INCREMENTAL' due to '/LTCG' specification
7>WebCore.lib(RenderSVGAllInOne.obj) : error LNK2005: "public: class WebCore::LayoutUnit __thiscall WebCore::RenderBoxModelObject::computedCSSPaddingTop(void)const " (?computedCSSPaddingTop@RenderBoxModelObject@WebCore@@QBE?AVLayoutUnit@2@XZ) already defined in WebCore.lib(RenderingAllInOne.obj)
7>WebCore.lib(RenderSVGAllInOne.obj) : error LNK2005: "public: class WebCore::LayoutUnit __thiscall WebCore::RenderBoxModelObject::computedCSSPaddingBottom(void)const " (?computedCSSPaddingBottom@RenderBoxModelObject@WebCore@@QBE?AVLayoutUnit@2@XZ) already defined in WebCore.lib(RenderingAllInOne.obj)
7>WebCore.lib(RenderSVGAllInOne.obj) : error LNK2005: "public: class WebCore::LayoutUnit __thiscall WebCore::RenderBoxModelObject::computedCSSPaddingLeft(void)const " (?computedCSSPaddingLeft@RenderBoxModelObject@WebCore@@QBE?AVLayoutUnit@2@XZ) already defined in WebCore.lib(RenderingAllInOne.obj)
7>WebCore.lib(RenderSVGAllInOne.obj) : error LNK2005: "public: class WebCore::LayoutUnit __thiscall WebCore::RenderBoxModelObject::computedCSSPaddingRight(void)const " (?computedCSSPaddingRight@RenderBoxModelObject@WebCore@@QBE?AVLayoutUnit@2@XZ) already defined in WebCore.lib(RenderingAllInOne.obj)
7>WebCore.lib(RenderSVGAllInOne.obj) : error LNK2005: "public: class WebCore::LayoutUnit __thiscall WebCore::RenderBoxModelObject::computedCSSPaddingBefore(void)const " (?computedCSSPaddingBefore@RenderBoxModelObject@WebCore@@QBE?AVLayoutUnit@2@XZ) already defined in WebCore.lib(RenderingAllInOne.obj)
7>WebCore.lib(RenderSVGAllInOne.obj) : error LNK2005: "public: class WebCore::LayoutUnit __thiscall WebCore::RenderBoxModelObject::computedCSSPaddingAfter(void)const " (?computedCSSPaddingAfter@RenderBoxModelObject@WebCore@@QBE?AVLayoutUnit@2@XZ) already defined in WebCore.lib(RenderingAllInOne.obj)
7>WebCore.lib(RenderSVGAllInOne.obj) : error LNK2005: "public: class WebCore::LayoutUnit __thiscall WebCore::RenderBoxModelObject::computedCSSPaddingStart(void)const " (?computedCSSPaddingStart@RenderBoxModelObject@WebCore@@QBE?AVLayoutUnit@2@XZ) already defined in WebCore.lib(RenderingAllInOne.obj)
Comment 6 WebKit Review Bot 2013-02-01 17:23:37 PST
Re-opened since this is blocked by bug 108728
Comment 7 Emil A Eklund 2013-02-01 17:42:17 PST
Thanks for rolling it out.

Probably just needs to force a rebuild (or revalidation of the all in one file). Will try to figure out a way to commit in a way that doesn't break windows.
Comment 8 Eric Seidel (no email) 2013-02-01 21:21:19 PST
Comment on attachment 186160 [details]
Patch

Wow.
Comment 9 Eric Seidel (no email) 2013-02-01 21:21:49 PST
Oh, sorry, I guess you didn't need another r+.
Comment 10 Emil A Eklund 2013-02-04 09:46:48 PST
Committed r141775: <http://trac.webkit.org/changeset/141775>