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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 11348165: Remove PrefObserver usages, batch 10. (Closed)

Created:
8 years, 1 month ago by tfarina
Modified:
8 years, 1 month ago
CC:
chromium-reviews, dbeam+watch-ntp_chromium.org, tfarina, estade+watch_chromium.org, chromium-apps-reviews_chromium.org, Aaron Boodman, sky, csilv
Visibility:
Public.

Description

Remove PrefObserver usages, batch 10. This also removes GetBackgroundPageBadge() from toolbar_view.h, because it is not implemented anywhere. BUG=155525 R=mnissler@chromium.org,joi@chromium.org TBR=sky@chromium.org,csilv@chromium.org # for views and webui changes. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=168968

Patch Set 1 #

Total comments: 6

Patch Set 2 : OnPreferenceChanged #

Patch Set 3 : fix android #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+68 lines, -115 lines) Patch
M chrome/browser/ui/views/tabs/browser_tab_strip_controller.h View 3 chunks +1 line, -7 lines 0 comments Download
M chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc View 2 chunks +4 lines, -12 lines 0 comments Download
M chrome/browser/ui/views/toolbar_view.h View 4 chunks +2 lines, -10 lines 0 comments Download
M chrome/browser/ui/views/toolbar_view.cc View 3 chunks +8 lines, -9 lines 0 comments Download
M chrome/browser/ui/webui/extensions/extension_settings_handler.h View 3 chunks +0 lines, -6 lines 0 comments Download
M chrome/browser/ui/webui/extensions/extension_settings_handler.cc View 2 chunks +6 lines, -8 lines 0 comments Download
M chrome/browser/ui/webui/ntp/app_launcher_handler.h View 1 4 chunks +3 lines, -7 lines 0 comments Download
M chrome/browser/ui/webui/ntp/app_launcher_handler.cc View 1 3 chunks +11 lines, -10 lines 0 comments Download
M chrome/browser/ui/webui/ntp/new_tab_ui.h View 3 chunks +2 lines, -6 lines 0 comments Download
M chrome/browser/ui/webui/ntp/new_tab_ui.cc View 2 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/ui/webui/ntp/ntp_login_handler.h View 3 chunks +1 line, -7 lines 0 comments Download
M chrome/browser/ui/webui/ntp/ntp_login_handler.cc View 2 chunks +4 lines, -7 lines 0 comments Download
M chrome/browser/ui/webui/ntp/ntp_resource_cache.h View 1 3 chunks +9 lines, -11 lines 0 comments Download
M chrome/browser/ui/webui/ntp/ntp_resource_cache.cc View 1 3 chunks +10 lines, -8 lines 2 comments Download
M chrome/browser/ui/webui/ntp/ntp_resource_cache_android.cc View 1 2 1 chunk +3 lines, -4 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
tfarina
Hi Mattias, Joi, could you review this to me? Thanks,
8 years, 1 month ago (2012-11-20 19:39:56 UTC) #1
Jói
LGTM with nits. https://codereview.chromium.org/11348165/diff/1/chrome/browser/ui/views/toolbar_view.h File chrome/browser/ui/views/toolbar_view.h (left): https://codereview.chromium.org/11348165/diff/1/chrome/browser/ui/views/toolbar_view.h#oldcode203 chrome/browser/ui/views/toolbar_view.h:203: gfx::ImageSkia GetBackgroundPageBadge(); Looks like you're removing ...
8 years, 1 month ago (2012-11-20 22:11:54 UTC) #2
tfarina
TBRing Scott and Chris. https://codereview.chromium.org/11348165/diff/1/chrome/browser/ui/views/toolbar_view.h File chrome/browser/ui/views/toolbar_view.h (left): https://codereview.chromium.org/11348165/diff/1/chrome/browser/ui/views/toolbar_view.h#oldcode203 chrome/browser/ui/views/toolbar_view.h:203: gfx::ImageSkia GetBackgroundPageBadge(); On 2012/11/20 22:11:55, ...
8 years, 1 month ago (2012-11-20 23:41:19 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tfarina@chromium.org/11348165/2012
8 years, 1 month ago (2012-11-20 23:41:48 UTC) #4
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
8 years, 1 month ago (2012-11-21 00:03:17 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tfarina@chromium.org/11348165/11002
8 years, 1 month ago (2012-11-21 01:19:23 UTC) #6
commit-bot: I haz the power
Change committed as 168968
8 years, 1 month ago (2012-11-21 04:11:43 UTC) #7
Mattias Nissler (ping if slow)
LGTM w/ a nit. https://codereview.chromium.org/11348165/diff/11002/chrome/browser/ui/webui/ntp/ntp_resource_cache.cc File chrome/browser/ui/webui/ntp/ntp_resource_cache.cc (right): https://codereview.chromium.org/11348165/diff/11002/chrome/browser/ui/webui/ntp/ntp_resource_cache.cc#newcode247 chrome/browser/ui/webui/ntp/ntp_resource_cache.cc:247: void NTPResourceCache::OnPreferenceChanged() { nit: This ...
8 years, 1 month ago (2012-11-21 09:39:34 UTC) #8
Dan Beam
8 years, 1 month ago (2012-11-22 00:32:48 UTC) #9 
https://codereview.chromium.org/11348165/diff/11002/chrome/browser/ui/webui/n...
File chrome/browser/ui/webui/ntp/ntp_resource_cache.cc (right):

https://codereview.chromium.org/11348165/diff/11002/chrome/browser/ui/webui/n...
chrome/browser/ui/webui/ntp/ntp_resource_cache.cc:247: void
NTPResourceCache::OnPreferenceChanged() {
On 2012/11/21 09:39:34, Mattias Nissler wrote:
> nit: This function should be named FlushCache.

FYI: you may notice there are various things that could be "flushed" here, and
this is only NULL'ing a part of these caches.

Powered by Google App Engine
This is Rietveld 408576698