Remove PrefObserver usages, batch 12.

Remove PrefObserver usages, batch 12.

TBR=ben@chromium.org
BUG=155525
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=170483

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

[Jói, 2012-11-21 11:12:35]

mnissler: Main reviewer.

davemoore: Please look at ChromeLauncherController and verify that it is no
longer expected that it receives notifications on changes to
prefs::kShelfAlignmentLocal or prefs::kShelfAutoHideBehaviorLocal, those bits of
dispatch code looked dead so I removed them.

Cheers,
Jói

[Mattias Nissler (ping if slow), 2012-11-21 11:31:04]

https://codereview.chromium.org/11280115/diff/2001/chrome/browser/ui/alternat...
File chrome/browser/ui/alternate_error_tab_observer.cc (right):

https://codereview.chromium.org/11280115/diff/2001/chrome/browser/ui/alternat...
chrome/browser/ui/alternate_error_tab_observer.cc:31:
OnAlternateErrorPagesEnabledChanged,
Indent by 4? I don't know what the style rule is though, that's just my gut
feeling. Would make it more obvious that this is not the second parameter.

https://codereview.chromium.org/11280115/diff/2001/chrome/browser/ui/ash/laun...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc (left):

https://codereview.chromium.org/11280115/diff/2001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc:786:
SetShelfAlignmentFromPrefs();
Hm, maybe the code is missing pref change notifications for these?

https://codereview.chromium.org/11280115/diff/2001/chrome/browser/ui/ash/laun...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc (right):

https://codereview.chromium.org/11280115/diff/2001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc:851:
pref_change_registrar_.Add(prefs::kPinnedLauncherApps, this);
I think you missed this instance.

[Jói, 2012-11-21 12:59:33]

https://codereview.chromium.org/11280115/diff/2001/chrome/browser/ui/alternat...
File chrome/browser/ui/alternate_error_tab_observer.cc (right):

https://codereview.chromium.org/11280115/diff/2001/chrome/browser/ui/alternat...
chrome/browser/ui/alternate_error_tab_observer.cc:31:
OnAlternateErrorPagesEnabledChanged,
On 2012/11/21 11:31:04, Mattias Nissler wrote:
> Indent by 4? I don't know what the style rule is though, that's just my gut
> feeling. Would make it more obvious that this is not the second parameter.

Done.

https://codereview.chromium.org/11280115/diff/2001/chrome/browser/ui/ash/laun...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc (left):

https://codereview.chromium.org/11280115/diff/2001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc:786:
SetShelfAlignmentFromPrefs();
On 2012/11/21 11:31:04, Mattias Nissler wrote:
> Hm, maybe the code is missing pref change notifications for these?

davemoore: Do you know? The code prior to this change certainly wasn't listening
for these notifications. I also checked my original change to switch prefs
notifications from content::NotificationObserver to PrefObserver, and it made no
change to the prefs notifications this class subscribes to (see
https://chromiumcodereview.appspot.com/11345008/diff/31023/chrome/browser/ui/...).

https://codereview.chromium.org/11280115/diff/2001/chrome/browser/ui/ash/laun...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc (right):

https://codereview.chromium.org/11280115/diff/2001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc:851:
pref_change_registrar_.Add(prefs::kPinnedLauncherApps, this);
On 2012/11/21 11:31:04, Mattias Nissler wrote:
> I think you missed this instance.

Thanks for the catch.  The trybots also failed on compile (it's Ash so didn't
get compiled locally on my Chrome for Linux build).  I've fixed this.

[Mattias Nissler (ping if slow), 2012-11-22 09:04:38]

LGTM

[Jói, 2012-11-27 22:10:41]

davemoore: Friendly ping, see review request plus a question to you in the
earlier review mails.

[sky, 2012-11-29 01:49:02]

https://codereview.chromium.org/11280115/diff/2001/chrome/browser/ui/ash/laun...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc (left):

https://codereview.chromium.org/11280115/diff/2001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc:786:
SetShelfAlignmentFromPrefs();
On 2012/11/21 12:59:33, Jói wrote:
> On 2012/11/21 11:31:04, Mattias Nissler wrote:
> > Hm, maybe the code is missing pref change notifications for these?
> 
> davemoore: Do you know? The code prior to this change certainly wasn't
listening
> for these notifications. I also checked my original change to switch prefs
> notifications from content::NotificationObserver to PrefObserver, and it made
no
> change to the prefs notifications this class subscribes to (see
>
https://chromiumcodereview.appspot.com/11345008/diff/31023/chrome/browser/ui/...).

See line 236 old.

[Mattias Nissler (ping if slow), 2012-11-30 12:45:56]

LGTM. Why the heck did they duplicate all that code in the split though? ;)

[Jói, 2012-11-30 13:15:55]

TBR=ben@ for chrome/browser/ui OWNERS stamp. This is an API update, API change
and whole change previously reviewed by mnissler@ who is an OWNER for the API
being updated.

Cheers,
Jói

[commit-bot: I haz the power, 2012-11-30 13:16:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joi@chromium.org/11280115/13006

[commit-bot: I haz the power, 2012-11-30 18:04:58]

Change committed as 170483

[commit-bot: I haz the power, 2012-12-03 14:59:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joi@chromium.org/11280115/11006

[commit-bot: I haz the power, 2012-12-03 16:56:12]

Retried try job too often on linux_rel for step(s) ash_unittests

[commit-bot: I haz the power, 2012-12-03 17:08:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joi@chromium.org/11280115/11006

[commit-bot: I haz the power, 2012-12-03 17:46:10]

Retried try job too often on linux_rel for step(s) ash_unittests

[commit-bot: I haz the power, 2012-12-03 18:09:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joi@chromium.org/11280115/11006

[commit-bot: I haz the power, 2012-12-03 18:14:32]

Change committed as 170759