Chromium Code Reviews
Help | Chromium Project | Sign in
(300)

Issue 10985076: NativeThemeAndroid should inherit from NativeThemeBase (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 6 months ago by Peter Beverloo
Modified:
1 year, 6 months ago
Reviewers:
Xianzhu, sky, Rick Byers
CC:
chromium-reviews_chromium.org
Visibility:
Public.

Description

NativeThemeAndroid should inherit from NativeThemeBase

NativeThemeAndroid currently re-implements every method defined in the
NativeTheme class by duplicating the code, while it didn't actually modify
behavior. By changing its base class to NativeThemeBase, which has most
of the code we need, we only have to override GetSystemColor.

Visually, this lines up Android's button appearance with Linux, as Android's
code hadn't been updated for changing coming in to the general implementation.
It also enables the new checkbox appearance all of Chrome will be switching
too. Removing the old code is being tracked in Issue 133991.

BUG=152912
TEST=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=159483

Patch Set 1 #

Patch Set 2 : Patch #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -878 lines) Lint Patch
M ui/base/native_theme/native_theme_android.h View 1 1 chunk +3 lines, -153 lines 0 comments ? errors Download
M ui/base/native_theme/native_theme_android.cc View 3 chunks +0 lines, -725 lines 0 comments 0 errors Download
Commit:

Messages

Total messages: 10
Peter Beverloo
+Xianzhu for Android +Scott for ui/ OWNERS
1 year, 6 months ago #1
Rick Byers
Looks like a great clean-up to me. I didn't exhaustively verify that there's no other ...
1 year, 6 months ago #2
sky
I'm rubber stamping this and assuming Xianzhu will do a review.
1 year, 6 months ago #3
Xianzhu
LGTM. FYI we are likely to change the code to paint the web controls to ...
1 year, 6 months ago #4
Peter Beverloo
Thanks guys, that seems good to me. Putting this on the commit queue.
1 year, 6 months ago #5
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/peter@chromium.org/10985076/2001
1 year, 6 months ago #6
I haz the power (commit-bot)
Presubmit check for 10985076-2001 failed and returned exit status 1. Running presubmit commit checks ...
1 year, 6 months ago #7
sky
Forgot the rubber stamp: LGTM
1 year, 6 months ago #8
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/peter@chromium.org/10985076/2001
1 year, 6 months ago #9
I haz the power (commit-bot)
1 year, 6 months ago #10
Retried try job too often for step(s) compile
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1275:d14800f88434