Refactored code for timezone settings.

Refactored code for timezone settings. Moved ui specific stuff to webui/options2.
Ensure that only known timezones are stored in TimezoneSettings.

BUG=chromium-os:27206
TEST=Set symlink /var/lib/timezone/localtime to an unknown timezone
(i.e. not in |kTimeZones|) but with the same rules as a known
timezone. The settings UI must show one of these equivalent
timezones.

Change the timezone in the settings and observe both the clock and the symlink being updated.

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

[pneubeck2, 2012-07-12 15:00:10]

Second larger refactoring. Please review and don't hold yourself back :-)

[Joao da Silva, 2012-07-12 18:34:09]

First round of comments.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
File chrome/browser/chromeos/system/timezone_settings.cc (right):

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:202: virtual const
std::vector<icu::TimeZone*> &GetTimezoneList() const;
Mark all of these with OVERRIDE.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:214: const icu::TimeZone
*GetKnownTimezoneOrNull(
Nit: move * next to the return type

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:233: const string16
TimezoneSettingsBaseImpl::GetCurrentTimezoneID() {
No need for const when returning by value.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:257: for (size_t i = 0; i <
arraysize(kTimeZones); i++) {
Nit: ++i

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:263: const icu::TimeZone
*TimezoneSettingsBaseImpl::GetKnownTimezoneOrNull(
Nit: move * next to the return type

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:280: class
TimezoneSettingsImpl : public TimezoneSettingsBaseImpl {
Put all the class declarations first in the file, and then the method
implementations.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:283: virtual void
SetTimezone(const icu::TimeZone& timezone);
OVERRIDE

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:289: friend struct
DefaultSingletonTraits<TimezoneSettingsImpl>;
Nit: remove the empty line above. Add DISALLOW_COPY_AND_ASSIGN at the end of the
class.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:296: // none exists go on
with |timezone|.
Start both sentences in uppercase.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:297: const icu::TimeZone
*known_timezone = GetKnownTimezoneOrNull(timezone);
Nit: * next to the type

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:307:
BrowserThread::PostTask(BrowserThread::FILE,
Post to the blocking pool instead:

BrowserThread::PostBlockingPoolTask(FROM_HERE, Bind(...));

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:330: const icu::TimeZone
*known_timezone = GetKnownTimezoneOrNull(*timezone_);
Nit: move * next to type

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:341: class
TimezoneSettingsStubImpl : public TimezoneSettingsBaseImpl {
Declare all classes before defining method implementations.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:344: virtual void
SetTimezone(const icu::TimeZone& timezone) {
OVERRIDE

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:346: // none exists go on
with |timezone|.
Start both sentences in uppercase.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:366: const icu::TimeZone
*known_timezone = GetKnownTimezoneOrNull(*timezone_);
Nit: move * next to the type

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:369: }
DISALLOW_COPY_AND_ASSIGN

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:385: string16
TimezoneSettings::GetTimezoneID(
Add a comment before this line like this:

// static

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:386: const icu::TimeZone&
timezone) {
No need to break the previous line?

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:392: 
Nit: remove extra newline

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
File chrome/browser/chromeos/system/timezone_settings.h (right):

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.h:20: class Observer {
Please add a virtual destructor to this, with an empty implementation on the .cc
file.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.h:30: virtual const string16
GetCurrentTimezoneID() = 0;
No need for const when returning a value.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.h:39: virtual const
std::vector<icu::TimeZone*> &GetTimezoneList() const = 0;
Nit: move the & next to the type

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
File chrome/browser/chromeos/system_settings_provider.cc (right):

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system_settings_provider.cc:41:
!in_value.GetAsString(&timezone_id))
GetAsString() is sufficient, no need to test for the type first.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/ui/w...
File chrome/browser/ui/webui/options2/browser_options_handler.cc (right):

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/ui/w...
chrome/browser/ui/webui/options2/browser_options_handler.cc:150: // resource
keys for timezones use ':' in place of '/'.
Nit: start sentence in uppercase

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/ui/w...
chrome/browser/ui/webui/options2/browser_options_handler.cc:177: const
icu::TimeZone& timezone) {
No need to break the previous line?

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/ui/w...
chrome/browser/ui/webui/options2/browser_options_handler.cc:231: } // namespace
anonymous
Two spaces before the comment, and just "namespace":

}  // namespace

[pneubeck2, 2012-07-13 10:31:39]

Fixed all of the comments.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
File chrome/browser/chromeos/system/timezone_settings.cc (right):

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:202: virtual const
std::vector<icu::TimeZone*> &GetTimezoneList() const;
On 2012/07/12 18:34:09, Joao da Silva wrote:
> Mark all of these with OVERRIDE.

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:214: const icu::TimeZone
*GetKnownTimezoneOrNull(
On 2012/07/12 18:34:09, Joao da Silva wrote:
> Nit: move * next to the return type

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:233: const string16
TimezoneSettingsBaseImpl::GetCurrentTimezoneID() {
On 2012/07/12 18:34:09, Joao da Silva wrote:
> No need for const when returning by value.

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:257: for (size_t i = 0; i <
arraysize(kTimeZones); i++) {
On 2012/07/12 18:34:09, Joao da Silva wrote:
> Nit: ++i

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:263: const icu::TimeZone
*TimezoneSettingsBaseImpl::GetKnownTimezoneOrNull(
On 2012/07/12 18:34:09, Joao da Silva wrote:
> Nit: move * next to the return type

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:280: class
TimezoneSettingsImpl : public TimezoneSettingsBaseImpl {
On 2012/07/12 18:34:09, Joao da Silva wrote:
> Put all the class declarations first in the file, and then the method
> implementations.

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:283: virtual void
SetTimezone(const icu::TimeZone& timezone);
On 2012/07/12 18:34:09, Joao da Silva wrote:
> OVERRIDE

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:289: friend struct
DefaultSingletonTraits<TimezoneSettingsImpl>;
On 2012/07/12 18:34:09, Joao da Silva wrote:
> Nit: remove the empty line above. Add DISALLOW_COPY_AND_ASSIGN at the end of
the
> class.

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:296: // none exists go on
with |timezone|.
On 2012/07/12 18:34:09, Joao da Silva wrote:
> Start both sentences in uppercase.

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:297: const icu::TimeZone
*known_timezone = GetKnownTimezoneOrNull(timezone);
On 2012/07/12 18:34:09, Joao da Silva wrote:
> Nit: * next to the type

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:307:
BrowserThread::PostTask(BrowserThread::FILE,
On 2012/07/12 18:34:09, Joao da Silva wrote:
> Post to the blocking pool instead:
> 
> BrowserThread::PostBlockingPoolTask(FROM_HERE, Bind(...));

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:330: const icu::TimeZone
*known_timezone = GetKnownTimezoneOrNull(*timezone_);
On 2012/07/12 18:34:09, Joao da Silva wrote:
> Nit: move * next to type

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:341: class
TimezoneSettingsStubImpl : public TimezoneSettingsBaseImpl {
On 2012/07/12 18:34:09, Joao da Silva wrote:
> Declare all classes before defining method implementations.

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:344: virtual void
SetTimezone(const icu::TimeZone& timezone) {
On 2012/07/12 18:34:09, Joao da Silva wrote:
> OVERRIDE

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:346: // none exists go on
with |timezone|.
On 2012/07/12 18:34:09, Joao da Silva wrote:
> Start both sentences in uppercase.

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:366: const icu::TimeZone
*known_timezone = GetKnownTimezoneOrNull(*timezone_);
On 2012/07/12 18:34:09, Joao da Silva wrote:
> Nit: move * next to the type

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:369: }
On 2012/07/12 18:34:09, Joao da Silva wrote:
> DISALLOW_COPY_AND_ASSIGN

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:385: string16
TimezoneSettings::GetTimezoneID(
On 2012/07/12 18:34:09, Joao da Silva wrote:
> Add a comment before this line like this:
> 
> // static

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:386: const icu::TimeZone&
timezone) {
On 2012/07/12 18:34:09, Joao da Silva wrote:
> No need to break the previous line?

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:392: 
On 2012/07/12 18:34:09, Joao da Silva wrote:
> Nit: remove extra newline

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
File chrome/browser/chromeos/system/timezone_settings.h (right):

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.h:20: class Observer {
On 2012/07/12 18:34:09, Joao da Silva wrote:
> Please add a virtual destructor to this, with an empty implementation on the
.cc
> file.

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.h:30: virtual const string16
GetCurrentTimezoneID() = 0;
On 2012/07/12 18:34:09, Joao da Silva wrote:
> No need for const when returning a value.

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.h:39: virtual const
std::vector<icu::TimeZone*> &GetTimezoneList() const = 0;
On 2012/07/12 18:34:09, Joao da Silva wrote:
> Nit: move the & next to the type

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
File chrome/browser/chromeos/system_settings_provider.cc (right):

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/chro...
chrome/browser/chromeos/system_settings_provider.cc:41:
!in_value.GetAsString(&timezone_id))
On 2012/07/12 18:34:09, Joao da Silva wrote:
> GetAsString() is sufficient, no need to test for the type first.

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/ui/w...
File chrome/browser/ui/webui/options2/browser_options_handler.cc (right):

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/ui/w...
chrome/browser/ui/webui/options2/browser_options_handler.cc:150: // resource
keys for timezones use ':' in place of '/'.
On 2012/07/12 18:34:09, Joao da Silva wrote:
> Nit: start sentence in uppercase

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/ui/w...
chrome/browser/ui/webui/options2/browser_options_handler.cc:177: const
icu::TimeZone& timezone) {
On 2012/07/12 18:34:09, Joao da Silva wrote:
> No need to break the previous line?

Done.

https://chromiumcodereview.appspot.com/10689175/diff/5001/chrome/browser/ui/w...
chrome/browser/ui/webui/options2/browser_options_handler.cc:231: } // namespace
anonymous
On 2012/07/12 18:34:09, Joao da Silva wrote:
> Two spaces before the comment, and just "namespace":
> 
> }  // namespace

Done.

[Joao da Silva, 2012-07-13 13:05:30]

LGTM after fixing the remaining nits.

https://chromiumcodereview.appspot.com/10689175/diff/1009/chrome/browser/chro...
File chrome/browser/chromeos/system/timezone_settings.cc (right):

https://chromiumcodereview.appspot.com/10689175/diff/1009/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:322: // It's safe to Change
the timezone config files in the background as the
s/Change/change

https://chromiumcodereview.appspot.com/10689175/diff/1009/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:398: string16
TimezoneSettings::GetTimezoneID(const icu::TimeZone& timezone) {
// static

[pneubeck2, 2012-07-13 13:20:45]

James, could you please review this second refactoring of the timezone code.
After this refactoring I will add the policy.

Fixed the commented places.

https://chromiumcodereview.appspot.com/10689175/diff/1009/chrome/browser/chro...
File chrome/browser/chromeos/system/timezone_settings.cc (right):

https://chromiumcodereview.appspot.com/10689175/diff/1009/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:322: // It's safe to Change
the timezone config files in the background as the
On 2012/07/13 13:05:30, Joao da Silva wrote:
> s/Change/change

Done.

https://chromiumcodereview.appspot.com/10689175/diff/1009/chrome/browser/chro...
chrome/browser/chromeos/system/timezone_settings.cc:398: string16
TimezoneSettings::GetTimezoneID(const icu::TimeZone& timezone) {
On 2012/07/13 13:05:30, Joao da Silva wrote:
> // static

Done.

[James Hawkins, 2012-07-13 14:25:04]

https://chromiumcodereview.appspot.com/10689175/diff/4004/chrome/browser/ui/w...
File chrome/browser/ui/webui/options2/browser_options_handler.cc (right):

https://chromiumcodereview.appspot.com/10689175/diff/4004/chrome/browser/ui/w...
chrome/browser/ui/webui/options2/browser_options_handler.cc:115: #if
defined(OS_CHROMEOS)
I'm not a fan of having this much CrOS-specific code in a cross-platform file.

[pneubeck2, 2012-07-16 11:32:38]

Moved the code to a chromeos file.

On the way I found a few places to replaces includes by forward declarations.
These are unrelated to timezones, nonetheless this should be fixed.

https://chromiumcodereview.appspot.com/10689175/diff/4004/chrome/browser/ui/w...
File chrome/browser/ui/webui/options2/browser_options_handler.cc (right):

https://chromiumcodereview.appspot.com/10689175/diff/4004/chrome/browser/ui/w...
chrome/browser/ui/webui/options2/browser_options_handler.cc:115: #if
defined(OS_CHROMEOS)
On 2012/07/13 14:25:04, James Hawkins wrote:
> I'm not a fan of having this much CrOS-specific code in a cross-platform file.

Done.

[Joao da Silva, 2012-07-16 11:44:53]

Still lgtm with a couple of nits

https://chromiumcodereview.appspot.com/10689175/diff/16010/chrome/browser/chr...
File chrome/browser/chromeos/device_settings_provider.cc (right):

https://chromiumcodereview.appspot.com/10689175/diff/16010/chrome/browser/chr...
chrome/browser/chromeos/device_settings_provider.cc:31: 
Nit: remove extra newline

https://chromiumcodereview.appspot.com/10689175/diff/16010/chrome/browser/pol...
File chrome/browser/policy/cloud_policy_validator.h (right):

https://chromiumcodereview.appspot.com/10689175/diff/16010/chrome/browser/pol...
chrome/browser/policy/cloud_policy_validator.h:30: class CloudPolicySettings;
Nit: alphabetic order

https://chromiumcodereview.appspot.com/10689175/diff/16010/chrome/browser/ui/...
File chrome/browser/ui/webui/options2/chromeos/timezone_options_util.h (right):

https://chromiumcodereview.appspot.com/10689175/diff/16010/chrome/browser/ui/...
chrome/browser/ui/webui/options2/chromeos/timezone_options_util.h:15:
base::ListValue* GetTimezoneList();
Suggestion: make the return type scoped_ptr<base::ListValue> so that it's clear
that the caller owns the result.

[pneubeck2, 2012-07-16 12:21:45]

Fixed the nits. Wrapped the returned TimezoneList in a scoped_ptr.

https://chromiumcodereview.appspot.com/10689175/diff/16010/chrome/browser/chr...
File chrome/browser/chromeos/device_settings_provider.cc (right):

https://chromiumcodereview.appspot.com/10689175/diff/16010/chrome/browser/chr...
chrome/browser/chromeos/device_settings_provider.cc:31: 
On 2012/07/16 11:44:54, Joao da Silva wrote:
> Nit: remove extra newline

Done.

https://chromiumcodereview.appspot.com/10689175/diff/16010/chrome/browser/pol...
File chrome/browser/policy/cloud_policy_validator.h (right):

https://chromiumcodereview.appspot.com/10689175/diff/16010/chrome/browser/pol...
chrome/browser/policy/cloud_policy_validator.h:30: class CloudPolicySettings;
On 2012/07/16 11:44:54, Joao da Silva wrote:
> Nit: alphabetic order

Done.

https://chromiumcodereview.appspot.com/10689175/diff/16010/chrome/browser/ui/...
File chrome/browser/ui/webui/options2/chromeos/timezone_options_util.h (right):

https://chromiumcodereview.appspot.com/10689175/diff/16010/chrome/browser/ui/...
chrome/browser/ui/webui/options2/chromeos/timezone_options_util.h:15:
base::ListValue* GetTimezoneList();
On 2012/07/16 11:44:54, Joao da Silva wrote:
> Suggestion: make the return type scoped_ptr<base::ListValue> so that it's
clear
> that the caller owns the result.

Done.

[pneubeck2, 2012-07-17 11:16:05]

James, I splitted the files as you suggested. Does the rest look good to you,
too?

[James Hawkins, 2012-07-17 14:10:46]

LGTM with nit.

https://chromiumcodereview.appspot.com/10689175/diff/23009/chrome/browser/ui/...
File chrome/browser/ui/webui/options2/chromeos/timezone_options_util.cc (right):

https://chromiumcodereview.appspot.com/10689175/diff/23009/chrome/browser/ui/...
chrome/browser/ui/webui/options2/chromeos/timezone_options_util.cc:35: string16
GetExemplarCity(const icu::TimeZone& zone) {
nit: Document method and param.

[pneubeck2, 2012-07-18 12:08:54]

Fixed the last nit. Thanks for the review.

https://chromiumcodereview.appspot.com/10689175/diff/23009/chrome/browser/ui/...
File chrome/browser/ui/webui/options2/chromeos/timezone_options_util.cc (right):

https://chromiumcodereview.appspot.com/10689175/diff/23009/chrome/browser/ui/...
chrome/browser/ui/webui/options2/chromeos/timezone_options_util.cc:35: string16
GetExemplarCity(const icu::TimeZone& zone) {
On 2012/07/17 14:10:46, James Hawkins wrote:
> nit: Document method and param.

Done.

[commit-bot: I haz the power, 2012-07-18 12:09:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pneubeck@google.com/10689175/18009

[commit-bot: I haz the power, 2012-07-18 12:29:14]

Try job failure for 10689175-18009 (retry) on linux_clang for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...

[commit-bot: I haz the power, 2012-07-18 14:27:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pneubeck@google.com/10689175/18009

[commit-bot: I haz the power, 2012-07-18 15:36:08]

Change committed as 147242