Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(165)

Issues Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

Issue 1102353008: Split ThemePainter out of LayoutTheme (Closed)

Created:
5 years ago by Xianzhu
Modified:
5 years ago
Reviewers:
chrishtr, pdr.
CC:
blink-reviews, blink-reviews-paint_chromium.org, blink-reviews-rendering, dshwang, eae+blinkwatch, eric.carlson_apple.com, feature-media-reviews_chromium.org, jchaffraix+rendering, leviw+renderwatch, pdr+renderingwatchlist_chromium.org, philipj_slow, slimming-paint-reviews_chromium.org, zoltan1
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Split ThemePainter out of LayoutTheme Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=194778

Patch Set 1 : #

Total comments: 3

Patch Set 2 : Rebase #

Patch Set 3 : Rebase #

Patch Set 4 : Fix Android build #

Total comments: 2

Patch Set 5 : themePainter() -> theme().painter() #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1883 lines, -2236 lines) Patch
M Source/core/core.gyp View 1 2 3 2 chunks +8 lines, -0 lines 0 comments Download
M Source/core/core.gypi View 1 2 3 3 chunks +8 lines, -2 lines 0 comments Download
M Source/core/layout/LayoutMediaControls.h View 1 chunk +0 lines, -50 lines 0 comments Download
M Source/core/layout/LayoutMediaControls.cpp View 1 chunk +0 lines, -483 lines 0 comments Download
M Source/core/layout/LayoutTextControlSingleLine.cpp View 1 2 3 4 2 chunks +2 lines, -1 line 0 comments Download
M Source/core/layout/LayoutTheme.h View 1 2 3 4 8 chunks +20 lines, -80 lines 0 comments Download
M Source/core/layout/LayoutTheme.cpp View 1 16 chunks +35 lines, -356 lines 0 comments Download
M Source/core/layout/LayoutThemeAndroid.cpp View 1 2 3 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/layout/LayoutThemeDefault.h View 1 5 chunks +5 lines, -49 lines 0 comments Download
M Source/core/layout/LayoutThemeDefault.cpp View 1 12 chunks +2 lines, -435 lines 0 comments Download
M Source/core/layout/LayoutThemeMac.h View 5 chunks +33 lines, -45 lines 0 comments Download
M Source/core/layout/LayoutThemeMac.mm View 1 18 chunks +11 lines, -686 lines 0 comments Download
M Source/core/paint/BoxPainter.cpp View 1 2 3 4 3 chunks +5 lines, -3 lines 0 comments Download
A + Source/core/paint/MediaControlsPainter.h View 1 chunk +6 lines, -7 lines 0 comments Download
A + Source/core/paint/MediaControlsPainter.cpp View 4 chunks +6 lines, -38 lines 0 comments Download
A Source/core/paint/ThemePainter.h View 1 chunk +92 lines, -0 lines 0 comments Download
A Source/core/paint/ThemePainter.cpp View 1 chunk +382 lines, -0 lines 0 comments Download
A Source/core/paint/ThemePainterDefault.h View 1 chunk +67 lines, -0 lines 0 comments Download
A Source/core/paint/ThemePainterDefault.cpp View 1 chunk +481 lines, -0 lines 0 comments Download
A Source/core/paint/ThemePainterMac.h View 1 chunk +67 lines, -0 lines 0 comments Download
A Source/core/paint/ThemePainterMac.mm View 1 chunk +653 lines, -0 lines 0 comments Download

Messages

Total messages: 18 (8 generated)
Xianzhu
https://codereview.chromium.org/1102353008/diff/120001/Source/core/layout/LayoutTheme.cpp File Source/core/layout/LayoutTheme.cpp (left): https://codereview.chromium.org/1102353008/diff/120001/Source/core/layout/LayoutTheme.cpp#oldcode423 Source/core/layout/LayoutTheme.cpp:423: { Previously this was overridden in LayoutThemeDefault and LayoutThemeMac ...
5 years ago (2015-04-30 04:18:30 UTC) #5
Xianzhu
PTAL. Please see inline comments in https://codereview.chromium.org/1102353008/#ps120001 for explanation of changes other than simple code ...
5 years ago (2015-04-30 18:57:46 UTC) #9
chrishtr
https://codereview.chromium.org/1102353008/diff/180001/Source/core/layout/LayoutTheme.h File Source/core/layout/LayoutTheme.h (right): https://codereview.chromium.org/1102353008/diff/180001/Source/core/layout/LayoutTheme.h#newcode57 Source/core/layout/LayoutTheme.h:57: static ThemePainter& themePainter() { return theme().painter(); } I'd say ...
5 years ago (2015-04-30 20:16:16 UTC) #10
Xianzhu
https://codereview.chromium.org/1102353008/diff/180001/Source/core/layout/LayoutTheme.h File Source/core/layout/LayoutTheme.h (right): https://codereview.chromium.org/1102353008/diff/180001/Source/core/layout/LayoutTheme.h#newcode57 Source/core/layout/LayoutTheme.h:57: static ThemePainter& themePainter() { return theme().painter(); } On 2015/04/30 ...
5 years ago (2015-04-30 21:08:42 UTC) #11
chrishtr
lgtm I hope all this is tested? LGTM but it's hard to verify.
5 years ago (2015-04-30 21:16:25 UTC) #12
Xianzhu
On 2015/04/30 21:16:25, chrishtr wrote: > lgtm > > I hope all this is tested? ...
5 years ago (2015-04-30 21:20:37 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1102353008/200001
5 years ago (2015-04-30 21:21:49 UTC) #15
commit-bot: I haz the power
Committed patchset #5 (id:200001) as https://src.chromium.org/viewvc/blink?view=rev&revision=194778
5 years ago (2015-04-30 23:56:20 UTC) #16
Noel Gordon
No bug tracking any of this work, as far as I can can tell.
5 years ago (2015-05-01 08:16:48 UTC) #17
Xianzhu
5 years ago (2015-05-01 16:04:50 UTC) #18
Message was sent while issue was closed. 
On 2015/05/01 08:16:48, noel gordon wrote:
> No bug tracking any of this work, as far as I can can tell.

I should have referenced http://crbug.com/412088.

Powered by Google App Engine
This is Rietveld 408576698