Date and Time dialog for when the clock isn't synced.

chrome/browser/ui/webui/options/chromeos/date_time_options_handler.cc

Index: chrome/browser/ui/webui/options/chromeos/date_time_options_handler.cc
diff --git a/chrome/browser/ui/webui/options/chromeos/date_time_options_handler.cc b/chrome/browser/ui/webui/options/chromeos/date_time_options_handler.cc
new file mode 100644
index 0000000000000000000000000000000000000000..fd1d9022ef94b5ada21def070202d7ce36a8440b
--- /dev/null
+++ b/chrome/browser/ui/webui/options/chromeos/date_time_options_handler.cc
@@ -0,0 +1,75 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/ui/webui/options/chromeos/date_time_options_handler.h"
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/strings/utf_string_conversions.h"

[Dan Beam, 2014/04/23 17:51:45]

^ why do you need this?

[michaelpg, 2014/04/24 01:32:23]

sorry! I registered a string I later removed, these are all extraneous.

+#include "base/values.h"
+#include "chrome/browser/chromeos/set_time_dialog.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/system_clock_client.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_view.h"
+#include "content/public/browser/web_ui.h"
+#include "grit/generated_resources.h"

[Dan Beam, 2014/04/23 17:51:45]

^ what's this for?

[michaelpg, 2014/04/24 01:32:23]

Done.

+#include "ui/base/l10n/l10n_util.h"

[Dan Beam, 2014/04/23 17:51:45]

^ what's this for?

[michaelpg, 2014/04/24 01:32:23]

Done.

+
+namespace chromeos {
+namespace options {
+
+DateTimeOptionsHandler::DateTimeOptionsHandler()
+    : can_set_time_(false),
+      page_initialized_(false) {
+}
+
+DateTimeOptionsHandler::~DateTimeOptionsHandler() {
+  DBusThreadManager::Get()->GetSystemClockClient()->RemoveObserver(this);

[Dan Beam, 2014/04/23 17:51:45]

can this class go down before InitializeHandler()?  probably not... but it might
be worthwhile to know what happens if you try to remove an observer that doesn't
exist.

[michaelpg, 2014/04/24 01:32:23]

I looked but there isn't a simple answer to the first question. However, there
is no harm in calling RemoveObserver on an observer list with an observer that
isn't in the list (other than the nanoseconds wasted searching for the
observer). I've added a comment to SystemClockClient to emphasize this.

[Dan Beam, 2014/04/24 21:32:23]

that's fine

+}
+
+void DateTimeOptionsHandler::RegisterMessages() {
+  // Callback for set time button.
+  web_ui()->RegisterMessageCallback("showSetTime",
+      base::Bind(&DateTimeOptionsHandler::HandleShowSetTime,
+      base::Unretained(this)));
+}
+
+void DateTimeOptionsHandler::GetLocalizedValues(
+    base::DictionaryValue* localized_strings) {
+}

[Dan Beam, 2014/04/23 17:51:45]

one wonders why this is pure virtual if some classes don't need it...

[stevenjb, 2014/04/23 21:32:04]

Frequently that is done so that implementations don't forget to include it. Not
sure that it matters either way here.

[michaelpg, 2014/04/24 01:32:23]

Interestingly it is the only pure virtual method in OptionsPageUIHandler.

[Dan Beam, 2014/04/24 21:32:23]

right, here I don't think it matters much
yes, I know... probably doesn't need to be.

+
+void DateTimeOptionsHandler::InitializeHandler() {
+  SystemClockClient* system_clock_client =
+      DBusThreadManager::Get()->GetSystemClockClient();
+  system_clock_client->AddObserver(this);
+
+  can_set_time_ = system_clock_client->CanSetTime();
+  SystemClockCanSetTimeChanged(can_set_time_);
+}
+
+void DateTimeOptionsHandler::InitializePage() {
+  page_initialized_ = true;
+  web_ui()->CallJavascriptFunction(
+      "BrowserOptions.canSetTimeChanged",
+      base::FundamentalValue(can_set_time_));

[Dan Beam, 2014/04/23 17:51:45]

change L54-56 to:

  SystemClockCanSetTimeChanged(can_set_time_);

[stevenjb, 2014/04/23 21:32:04]

This calls "BrowserOptions.canSetTimeChanged", that calls
"OptionsPage.canSetTimeChanged". I'm not sure what the difference is?

[michaelpg, 2014/04/24 01:32:23]

One exists and one doesn't... :-(

+}
+
+void DateTimeOptionsHandler::SystemClockCanSetTimeChanged(bool can_set_time) {
+  if (page_initialized_) {
+    web_ui()->CallJavascriptFunction(
+        "OptionsPage.canSetTimeChanged", base::FundamentalValue(can_set_time));
+  }
+  can_set_time_ = can_set_time;
+}
+
+void DateTimeOptionsHandler::HandleShowSetTime(const base::ListValue* args) {
+  if (can_set_time_) {

[Dan Beam, 2014/04/23 17:51:45]

should this code be hit if we can't set the time?  should this be
DCHECK(can_set_time_) or something along these lines?

[michaelpg, 2014/04/24 01:32:23]

Since the call to hide the set time button and the call to show the dialog are
asynchronous, a user could be unfortunate enough to click the button just before
the button is replaced by the "your time is automatically set" text. In that
case, we don't want to show the dialog, but it doesn't represent a failure.

(It's possible that the messages and the call to SystemClockCanSetTimeChanged
actually can't be ordered in a way that this would occur, but I don't know.)

+    SetTimeDialog::ShowDialog(
+        web_ui()->GetWebContents()->GetView()->GetTopLevelNativeWindow());
+  }
+}
+
+}  // namespace options
+}  // namespace chromeos