Disable proxy config button and show banner if proxy prefs are managed.

Disable proxy config button and show banner if proxy prefs are managed.

Preferences.xib changes: Connect Enabled of the proxies configuration button to
a getter in PreferencesWindowController.

BUG=49538
TEST=Unit test in pref_set_observer_unittest.cc, manually configure proxy policies, check advanced options page.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=53457

[Mattias Nissler (ping if slow), 2010-07-21 14:12:13]

Please review. Do you feel comfortable reviewing the small win/linux changes or
should I bring in someone else for these?

[danno, 2010-07-21 16:16:08]

unit tests for set observer?

http://codereview.chromium.org/3038013/diff/1/4
File chrome/browser/cocoa/preferences_window_controller.mm (right):

http://codereview.chromium.org/3038013/diff/1/4#newcode1388
chrome/browser/cocoa/preferences_window_controller.mm:1388: [self
didChangeValueForKey:@"isProxiesConfigureButtonEnabled"];
As discussed, is there a cleaner way to do this without having an un-paired
didChangeValueForKey?

http://codereview.chromium.org/3038013/diff/1/8
File chrome/browser/pref_set_observer.h (right):

http://codereview.chromium.org/3038013/diff/1/8#newcode19
chrome/browser/pref_set_observer.h:19: size_t count,
match indenting above.

[Mattias Nissler (ping if slow), 2010-07-21 16:38:52]

I'll write the unit test tomorrow.

http://codereview.chromium.org/3038013/diff/1/4
File chrome/browser/cocoa/preferences_window_controller.mm (right):

http://codereview.chromium.org/3038013/diff/1/4#newcode1388
chrome/browser/cocoa/preferences_window_controller.mm:1388: [self
didChangeValueForKey:@"isProxiesConfigureButtonEnabled"];
On 2010/07/21 16:16:09, danno wrote:
> As discussed, is there a cleaner way to do this without having an un-paired
> didChangeValueForKey?

I've now changed to code to use a @property, which works as well, but is a bit
overkill IMHO. Note that other properties (e.g. homepageURL) with explicit
getters and setters also violate the contract by silently updating the value
returned by the getter without calling willChangeValue before. I'm now inclined
to just pair up the didChangeValue with a willChangeValue, but I don't have a
strong opinion. WDYT? Is the @property approach really nicer?

http://codereview.chromium.org/3038013/diff/1/8
File chrome/browser/pref_set_observer.h (right):

http://codereview.chromium.org/3038013/diff/1/8#newcode19
chrome/browser/pref_set_observer.h:19: size_t count,
On 2010/07/21 16:16:09, danno wrote:
> match indenting above.

Done.

[Mattias Nissler (ping if slow), 2010-07-22 14:20:32]

Danno, care to take another look?

[Paweł Hajdan Jr., 2010-07-22 17:33:30]

Drive-by with a minor test nit.

http://codereview.chromium.org/3038013/diff/16001/17008
File chrome/browser/pref_set_observer_unittest.cc (right):

http://codereview.chromium.org/3038013/diff/16001/17008#newcode56
chrome/browser/pref_set_observer_unittest.cc:56: std::wstring* pstr =
((Details<std::wstring>&) arg).ptr();
nit: Can we get rid of a C-style cast?

[danno, 2010-07-23 07:20:57]

LGTM

http://codereview.chromium.org/3038013/diff/16001/17007
File chrome/browser/pref_set_observer.h (right):

http://codereview.chromium.org/3038013/diff/16001/17007#newcode22
chrome/browser/pref_set_observer.h:22: // Add a |pref| to the set of preferences
to watch.
minor nit: is some places in the commands you use watch, some places track, in
other places is observed. Why not use observed everywhere? Perhaps also rename
the methods IsObserved?