Fixes bug where clock button wasn't updating in response to a timezone

Fixes bug where clock button wasn't updating in response to a timezone
change.

BUG=104667
TEST=none
R=stevenjb@chromium.org


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

[stevenjb, 2011-11-18 21:18:29]

http://codereview.chromium.org/8600001/diff/1/chrome/browser/chromeos/status/...
File chrome/browser/chromeos/status/status_area_view_chromeos.h (right):

http://codereview.chromium.org/8600001/diff/1/chrome/browser/chromeos/status/...
chrome/browser/chromeos/status/status_area_view_chromeos.h:51: ClockMenuButton**
clock_button);
This seems a bit awkward to me. What do you think of adding GetButtonById() to
StatusAreaView instead?

[sky, 2011-11-18 21:23:24]

http://codereview.chromium.org/8600001/diff/1/chrome/browser/chromeos/status/...
File chrome/browser/chromeos/status/status_area_view_chromeos.h (right):

http://codereview.chromium.org/8600001/diff/1/chrome/browser/chromeos/status/...
chrome/browser/chromeos/status/status_area_view_chromeos.h:51: ClockMenuButton**
clock_button);
On 2011/11/18 21:18:29, Steven Bennetts wrote:
> This seems a bit awkward to me. What do you think of adding GetButtonById() to
> StatusAreaView instead?

I went this route because of type safety. GetViewById throws that out the
window.

[stevenjb, 2011-11-18 23:04:48]

lgtm

http://codereview.chromium.org/8600001/diff/1/chrome/browser/chromeos/status/...
File chrome/browser/chromeos/status/status_area_view_chromeos.h (right):

http://codereview.chromium.org/8600001/diff/1/chrome/browser/chromeos/status/...
chrome/browser/chromeos/status/status_area_view_chromeos.h:51: ClockMenuButton**
clock_button);
On 2011/11/18 21:23:24, sky wrote:
> On 2011/11/18 21:18:29, Steven Bennetts wrote:
> > This seems a bit awkward to me. What do you think of adding GetButtonById()
to
> > StatusAreaView instead?
> 
> I went this route because of type safety. GetViewById throws that out the
> window.
That's fair. Part of me wonders whether we should just duplicate the button
adding code in the two places we use it instead, but this seems fine for now.

[stevenjb, 2011-11-18 23:06:11]

http://codereview.chromium.org/8600001/diff/1/chrome/browser/chromeos/status/...
File chrome/browser/chromeos/status/status_area_view_chromeos.cc (right):

http://codereview.chromium.org/8600001/diff/1/chrome/browser/chromeos/status/...
chrome/browser/chromeos/status/status_area_view_chromeos.cc:64:
ClockMenuButton** clock_button) {
Oh, one more nit. We might want to allow this to be NULL and just not set it if
it is, instead of requiring a dummy local variable to pass in.

[sky, 2011-11-18 23:12:54]

http://codereview.chromium.org/8600001/diff/1/chrome/browser/chromeos/status/...
File chrome/browser/chromeos/status/status_area_view_chromeos.cc (right):

http://codereview.chromium.org/8600001/diff/1/chrome/browser/chromeos/status/...
chrome/browser/chromeos/status/status_area_view_chromeos.cc:64:
ClockMenuButton** clock_button) {
On 2011/11/18 23:06:11, Steven Bennetts wrote:
> Oh, one more nit. We might want to allow this to be NULL and just not set it
if
> it is, instead of requiring a dummy local variable to pass in.

Done.

[Ben Goodger (Google), 2011-11-18 23:15:04]

lgtm

[commit-bot: I haz the power, 2011-11-18 23:17:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sky@chromium.org/8600001/8001

[commit-bot: I haz the power, 2011-11-19 01:21:59]

Change committed as 110804