Commit 55326c30 authored by James Cook's avatar James Cook Committed by Commit Bot

Fix runtime error in chrome://set-time dialog

If the dialog is opened from OS settings, it does not show the
timezone. However, the C++ code can sometimes send a timezone update
while the dialog is open.

Only update the timezone select if it is showing. Always send the
current timezone from C++ when the dialog opens, such that we can
adjust the time picker appropriately if the timezone changes.

Bug: 1015179
Test: added to JS tests
Change-Id: I1cd641c377a209c114d4c1359acbce775bf438c2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2052565Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#741097}
parent 781d63b1
......@@ -137,7 +137,8 @@ Polymer({
isTimezoneVisible_: {
type: Boolean,
readonly: true,
value: () => loadTimeData.getValue('currentTimezoneId') != '',
value: () =>
/** @type {boolean} */ (loadTimeData.getValue('showTimezone')),
},
/**
......@@ -255,9 +256,11 @@ Polymer({
* @private
*/
setTimezone_(timezoneId) {
const timezoneSelect = this.$$('#timezoneSelect');
assert(timezoneSelect.childElementCount > 0);
timezoneSelect.value = timezoneId;
if (this.isTimezoneVisible_) {
const timezoneSelect = this.$$('#timezoneSelect');
assert(timezoneSelect.childElementCount > 0);
timezoneSelect.value = timezoneId;
}
const now = this.getInputTime_();
const timezoneDelta = getTimezoneDelta(timezoneId, this.selectedTimezone_);
......
......@@ -191,10 +191,9 @@ SetTimeUI::SetTimeUI(content::WebUI* web_ui) : WebDialogUI(web_ui) {
values.Set("timezoneList", chromeos::system::GetTimezoneList());
// If we are not logged in, we need to show the time zone dropdown.
// Otherwise, we can leave |currentTimezoneId| blank.
values.SetBoolean("showTimezone", SetTimeDialog::ShouldShowTimezone());
std::string current_timezone_id;
if (SetTimeDialog::ShouldShowTimezone())
CrosSettings::Get()->GetString(kSystemTimezone, &current_timezone_id);
CrosSettings::Get()->GetString(kSystemTimezone, &current_timezone_id);
values.SetString("currentTimezoneId", current_timezone_id);
values.SetDouble("buildTime", base::GetBuildTime().ToJsTime());
......
......@@ -52,6 +52,7 @@ suite('SetTimeDialog', function() {
suiteSetup(function() {
// Must use existing timezones in the test.
loadTimeData.overrideValues({
showTimezone: true,
currentTimezoneId: 'America/Sao_Paulo',
timezoneList: [
[
......@@ -190,15 +191,12 @@ suite('SetTimeDialog', function() {
expectEquals('America/Los_Angeles', newTimezone);
});
suite('NullTimezone', () => {
suite('HideTimezone', () => {
suiteSetup(() => {
loadTimeData.overrideValues({
currentTimezoneId: '',
timezoneList: [],
});
loadTimeData.overrideValues({showTimezone: false});
});
test('SetDateNullTimezone', async () => {
test('SetDate', async () => {
const dateInput = setTimeElement.$$('#dateInput');
assertTrue(!!dateInput);
......@@ -234,5 +232,12 @@ suite('SetTimeDialog', function() {
// Verify the page didn't try to change the timezone.
assertEquals(0, testBrowserProxy.getCallCount('setTimezone'));
});
test('TimezoneUpdate', () => {
assertEquals(null, setTimeElement.$$('#timezoneSelect'));
cr.webUIListenerCallback(
'system-timezone-changed', 'America/Los_Angeles');
// No crash.
});
});
});
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment