Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(22)

Issue 611001: Allow the Mac theme provider to give default colors/tints if requested. Switc... (Closed)

Created:
10 years, 10 months ago by Avi (use Gerrit)
Modified:
9 years, 7 months ago
CC:
chromium-reviews, John Grabowski, Paul Godavari, pam+watch_chromium.org, ben+cc_chromium.org
Visibility:
Public.

Description

Allow the Mac theme provider to give default colors/tints if requested. Switch the download shelf's "open downloads" link to directly use the theme provider. BUG=http://crbug.com/35554 TEST=no visible change in normal mode; incognito mode still being worked on

Patch Set 1 #

Patch Set 2 : Forgot a file... #

Total comments: 2

Patch Set 3 : Cleaned up set theme #

Patch Set 4 : '' #

Total comments: 2

Patch Set 5 : '' #

Patch Set 6 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+126 lines, -77 lines) Patch
M app/theme_provider.h View 1 2 3 4 1 chunk +9 lines, -6 lines 0 comments Download
M chrome/browser/browser_theme_provider.h View 1 2 3 4 4 chunks +17 lines, -4 lines 0 comments Download
M chrome/browser/browser_theme_provider.cc View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/browser_theme_provider_mac.mm View 1 2 3 4 4 chunks +67 lines, -30 lines 0 comments Download
M chrome/browser/cocoa/bookmark_bar_toolbar_view_unittest.mm View 2 3 4 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/cocoa/browser_theme_provider_init.mm View 1 2 3 4 6 chunks +13 lines, -12 lines 0 comments Download
M chrome/browser/cocoa/download_shelf_controller.mm View 1 2 3 4 4 chunks +14 lines, -22 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
Avi (use Gerrit)
10 years, 10 months ago (2010-02-16 22:18:32 UTC) #1
pink (ping after 24hrs)
LGTM http://codereview.chromium.org/611001/diff/3001/4003 File chrome/browser/browser_theme_provider_mac.mm (right): http://codereview.chromium.org/611001/diff/3001/4003#newcode87 chrome/browser/browser_theme_provider_mac.mm:87: bool is_default; worth initializing to a value, just ...
10 years, 10 months ago (2010-02-16 22:51:05 UTC) #2
Avi (use Gerrit)
BTW, new rev up. Much cleaner way of doing it (it was done the way ...
10 years, 10 months ago (2010-02-16 23:02:32 UTC) #3
pink (ping after 24hrs)
On Tue, Feb 16, 2010 at 6:02 PM, <avi@chromium.org> wrote: > Must we? It's very ...
10 years, 10 months ago (2010-02-17 13:49:20 UTC) #4
Avi (use Gerrit)
On 2010/02/17 13:49:20, pink wrote: > Until the code changes and someone forgets to set ...
10 years, 10 months ago (2010-02-17 14:39:57 UTC) #5
pink (ping after 24hrs)
If you want, I'll take another look at the "new way" after I get back ...
10 years, 10 months ago (2010-02-17 14:50:37 UTC) #6
pink (ping after 24hrs)
SLGTM http://codereview.chromium.org/611001/diff/9001/10006 File chrome/browser/cocoa/download_shelf_controller.mm (right): http://codereview.chromium.org/611001/diff/9001/10006#newcode109 chrome/browser/cocoa/download_shelf_controller.mm:109: ThemeProvider* provider = bridge_->browser()->profile()->GetThemeProvider(); the previous code was ...
10 years, 10 months ago (2010-02-17 15:54:45 UTC) #7
Avi (use Gerrit)
10 years, 10 months ago (2010-02-17 16:30:25 UTC) #8
http://codereview.chromium.org/611001/diff/9001/10006
File chrome/browser/cocoa/download_shelf_controller.mm (right):

http://codereview.chromium.org/611001/diff/9001/10006#newcode109
chrome/browser/cocoa/download_shelf_controller.mm:109: ThemeProvider* provider =
bridge_->browser()->profile()->GetThemeProvider();
On 2010/02/17 15:54:45, pink wrote:
> the previous code was checking if (provider), do you still want to/need to?

Hm. Probably a good idea; I'm not convinced it was unnecessary...

Powered by Google App Engine
This is Rietveld 408576698