Revert "Observe ClockModel directly from DateView/TimeView"

This reverts commit 59692b72.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 553433 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzU5NjkyYjcyYmMzNGNiYmM5ZGM3ZmY0NDE0ZjNjODQ2YTU4NzMyOWYM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.memory/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/27188

Sample Failed Step: mus_ash_unittests

Original change's description:
> Observe ClockModel directly from DateView/TimeView
> 
> This CL changes DateView and TimeView to directly observe ClockModel.
> It removes unrelated logic from SystemTrayItem and makes embeding these
> views easier.
> 
> TEST=ash_unittests
> BUG=none
> 
> Change-Id: I70b3a873c0ac259a05597d3f93b77fd7519dd462
> Reviewed-on: https://chromium-review.googlesource.com/1025615
> Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
> Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#553433}

No-Presubmit: true
No-Tree-Checks: true
No-Try: true
BUG=none

Change-Id: Ife07647a589f9caa276788bae5dccff68bcf41a0
Reviewed-on: https://chromium-review.googlesource.com/1027272
Cr-Commit-Position: refs/heads/master@{#553488}

ash/system/date/date_view.cc

@@ -4,6 +4,7 @@
 
 #include "ash/system/date/date_view.h"
 
+#include "ash/shell.h"
 #include "ash/strings/grit/ash_strings.h"
 #include "ash/system/model/clock_model.h"
 #include "ash/system/model/system_tray_model.h"
@@ -67,7 +68,6 @@ base::string16 FormatDayOfWeek(const base::Time& time) {
 }  // namespace
 
 BaseDateTimeView::~BaseDateTimeView() {
-  model_->RemoveObserver(this);
   timer_.Stop();
 }
 
@@ -78,37 +78,17 @@ void BaseDateTimeView::UpdateText() {
   SetTimer(now);
 }
 
-void BaseDateTimeView::UpdateTimeFormat() {
-  UpdateText();
-}
-
 void BaseDateTimeView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
   ActionableView::GetAccessibleNodeData(node_data);
   node_data->role = ax::mojom::Role::kTime;
 }
 
-void BaseDateTimeView::OnDateFormatChanged() {
-  UpdateTimeFormat();
-}
-
-void BaseDateTimeView::OnSystemClockTimeUpdated() {
-  UpdateTimeFormat();
-}
-
-void BaseDateTimeView::OnSystemClockCanSetTimeChanged(bool can_set_time) {}
-
-void BaseDateTimeView::Refresh() {}
-
-base::HourClockType BaseDateTimeView::GetHourTypeForTesting() const {
-  return model_->hour_clock_type();
-}
-
-BaseDateTimeView::BaseDateTimeView(SystemTrayItem* owner, ClockModel* model)
+BaseDateTimeView::BaseDateTimeView(SystemTrayItem* owner)
     : ActionableView(owner, TrayPopupInkDropStyle::INSET_BOUNDS),
-      model_(model) {
+      hour_type_(
+          Shell::Get()->system_tray_model()->clock()->hour_clock_type()) {
   SetTimer(base::Time::Now());
   SetFocusBehavior(FocusBehavior::NEVER);
-  model_->AddObserver(this);
 }
 
 void BaseDateTimeView::SetTimer(const base::Time& now) {
@@ -135,7 +115,7 @@ void BaseDateTimeView::SetTimer(const base::Time& now) {
 
 void BaseDateTimeView::UpdateTextInternal(const base::Time& now) {
   SetAccessibleName(base::TimeFormatTimeOfDayWithHourClockType(
-                        now, model_->hour_clock_type(), base::kKeepAmPm) +
+                        now, hour_type_, base::kKeepAmPm) +
                     base::ASCIIToUTF16(", ") +
                     base::TimeFormatFriendlyDate(now));
 
@@ -148,8 +128,8 @@ void BaseDateTimeView::ChildPreferredSizeChanged(views::View* child) {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-DateView::DateView(SystemTrayItem* owner, ClockModel* model)
-    : BaseDateTimeView(owner, model), action_(DateAction::NONE) {
+DateView::DateView(SystemTrayItem* owner)
+    : BaseDateTimeView(owner), action_(DateAction::NONE) {
   auto box_layout = std::make_unique<views::BoxLayout>(
       views::BoxLayout::kHorizontal,
       gfx::Insets(0, kTrayPopupLabelHorizontalPadding), 0);
@@ -163,8 +143,6 @@ DateView::DateView(SystemTrayItem* owner, ClockModel* model)
   TrayPopupItemStyle style(TrayPopupItemStyle::FontStyle::SYSTEM_INFO);
   style.SetupLabel(date_label_);
   AddChildView(date_label_);
-
-  OnSystemClockCanSetTimeChanged(model_->can_set_time());
 }
 
 DateView::~DateView() = default;
@@ -182,26 +160,28 @@ void DateView::SetAction(DateAction action) {
     SetInkDropMode(views::InkDropHostView::InkDropMode::ON);
 }
 
+void DateView::UpdateTimeFormat() {
+  hour_type_ = Shell::Get()->system_tray_model()->clock()->hour_clock_type();
+  UpdateText();
+}
+
+base::HourClockType DateView::GetHourTypeForTesting() const {
+  return hour_type_;
+}
+
 void DateView::UpdateTextInternal(const base::Time& now) {
   BaseDateTimeView::UpdateTextInternal(now);
   date_label_->SetText(l10n_util::GetStringFUTF16(
       IDS_ASH_STATUS_TRAY_DATE, FormatDayOfWeek(now), FormatDate(now)));
 }
 
-void DateView::OnSystemClockCanSetTimeChanged(bool can_set_time) {
-  // Outside of a logged-in session, the date button should launch the set time
-  // dialog if the time can be set.
-  if (model_->IsLoggedIn())
-    SetAction(can_set_time ? DateAction::SET_SYSTEM_TIME : DateAction::NONE);
-}
-
 bool DateView::PerformAction(const ui::Event& event) {
   switch (action_) {
     case DateAction::SHOW_DATE_SETTINGS:
-      model_->ShowDateSettings();
+      Shell::Get()->system_tray_controller()->ShowDateSettings();
       break;
     case DateAction::SET_SYSTEM_TIME:
-      model_->ShowSetTimeDialog();
+      Shell::Get()->system_tray_controller()->ShowSetTimeDialog();
       break;
     case DateAction::NONE:
       return false;
@@ -212,8 +192,7 @@ bool DateView::PerformAction(const ui::Event& event) {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-TimeView::TimeView(ClockLayout clock_layout, ClockModel* model)
-    : BaseDateTimeView(nullptr, model) {
+TimeView::TimeView(ClockLayout clock_layout) : BaseDateTimeView(nullptr) {
   SetupLabels();
   UpdateTextInternal(base::Time::Now());
   UpdateClockLayout(clock_layout);
@@ -221,6 +200,15 @@ TimeView::TimeView(ClockLayout clock_layout, ClockModel* model)
 
 TimeView::~TimeView() = default;
 
+void TimeView::UpdateTimeFormat() {
+  hour_type_ = Shell::Get()->system_tray_model()->clock()->hour_clock_type();
+  UpdateText();
+}
+
+base::HourClockType TimeView::GetHourTypeForTesting() const {
+  return hour_type_;
+}
+
 void TimeView::UpdateTextInternal(const base::Time& now) {
   // Just in case |now| is null, do NOT update time; otherwise, it will
   // crash icu code by calling into base::TimeFormatTimeOfDayWithHourClockType,
@@ -232,7 +220,7 @@ void TimeView::UpdateTextInternal(const base::Time& now) {
 
   BaseDateTimeView::UpdateTextInternal(now);
   base::string16 current_time = base::TimeFormatTimeOfDayWithHourClockType(
-      now, model_->hour_clock_type(), base::kDropAmPm);
+      now, hour_type_, base::kDropAmPm);
   horizontal_label_->SetText(current_time);
   horizontal_label_->SetTooltipText(base::TimeFormatFriendlyDate(now));
 
@@ -242,7 +230,7 @@ void TimeView::UpdateTextInternal(const base::Time& now) {
   base::string16 minute = current_time.substr(colon_pos + 1);
 
   // Sometimes pad single-digit hours with a zero for aesthetic reasons.
-  if (hour.length() == 1 && model_->hour_clock_type() == base::k24HourClock &&
+  if (hour.length() == 1 && hour_type_ == base::k24HourClock &&
       !base::i18n::IsRTL())
     hour = base::ASCIIToUTF16("0") + hour;
 
@@ -295,10 +283,6 @@ void TimeView::UpdateClockLayout(ClockLayout clock_layout) {
   Layout();
 }
 
-void TimeView::Refresh() {
-  UpdateText();
-}
-
 void TimeView::SetBorderFromLayout(ClockLayout clock_layout) {
   if (clock_layout == ClockLayout::HORIZONTAL_CLOCK)
     SetBorder(views::CreateEmptyBorder(gfx::Insets(0, kTrayImageItemPadding)));

ash/system/date/date_view.h

@@ -8,7 +8,6 @@
 #include <memory>
 
 #include "ash/ash_export.h"
-#include "ash/system/date/clock_observer.h"
 #include "ash/system/tray/actionable_view.h"
 #include "base/i18n/time_formatting.h"
 #include "base/macros.h"
@@ -24,42 +23,28 @@ class Label;
 }
 
 namespace ash {
-
-class ClockModel;
-
 namespace tray {
 
 // Abstract base class containing common updating and layout code for the
 // DateView popup and the TimeView tray icon. Exported for tests.
-class ASH_EXPORT BaseDateTimeView : public ActionableView,
-                                    public ClockObserver {
+class ASH_EXPORT BaseDateTimeView : public ActionableView {
  public:
   ~BaseDateTimeView() override;
 
   // Updates the displayed text for the current time and calls SetTimer().
   void UpdateText();
 
-  // Updates the format of the displayed time.
-  void UpdateTimeFormat();
-
   // views::View:
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
 
-  // ClockObserver:
-  void OnDateFormatChanged() override;
-  void OnSystemClockTimeUpdated() override;
-  void OnSystemClockCanSetTimeChanged(bool can_set_time) override;
-  void Refresh() override;
-
-  base::HourClockType GetHourTypeForTesting() const;
-
  protected:
-  BaseDateTimeView(SystemTrayItem* owner, ClockModel* model);
+  explicit BaseDateTimeView(SystemTrayItem* owner);
 
   // Updates labels to display the current time.
   virtual void UpdateTextInternal(const base::Time& now);
 
-  ClockModel* const model_;
+  // Time format (12/24hr) used for accessibility string.
+  base::HourClockType hour_type_;
 
  private:
   // Starts |timer_| to schedule the next update.
@@ -83,7 +68,7 @@ class ASH_EXPORT DateView : public BaseDateTimeView {
     SHOW_DATE_SETTINGS,
   };
 
-  DateView(SystemTrayItem* owner, ClockModel* model);
+  explicit DateView(SystemTrayItem* owner);
   ~DateView() override;
 
   // Sets the action the view should take. An actionable date view gives visual
@@ -91,8 +76,10 @@ class ASH_EXPORT DateView : public BaseDateTimeView {
   // or enter on the view executes the action.
   void SetAction(DateAction action);
 
-  // ClockObserver:
-  void OnSystemClockCanSetTimeChanged(bool can_set_time) override;
+  // Updates the format of the displayed time.
+  void UpdateTimeFormat();
+
+  base::HourClockType GetHourTypeForTesting() const;
 
  private:
   // Sets active rendering state and updates the color of |date_label_|.
@@ -120,14 +107,16 @@ class ASH_EXPORT TimeView : public BaseDateTimeView {
     VERTICAL_CLOCK,
   };
 
-  TimeView(ClockLayout clock_layout, ClockModel* model);
+  explicit TimeView(ClockLayout clock_layout);
   ~TimeView() override;
 
+  // Updates the format of the displayed time.
+  void UpdateTimeFormat();
+
   // Updates clock layout.
   void UpdateClockLayout(ClockLayout clock_layout);
 
-  // ClockObserver:
-  void Refresh() override;
+  base::HourClockType GetHourTypeForTesting() const;
 
  private:
   friend class TimeViewTest;

ash/system/date/date_view_unittest.cc

@@ -4,8 +4,6 @@
 
 #include "ash/system/date/date_view.h"
 
-#include "ash/shell.h"
-#include "ash/system/model/system_tray_model.h"
 #include "ash/test/ash_test_base.h"
 #include "ui/views/controls/label.h"
 
@@ -32,8 +30,7 @@ class TimeViewTest : public AshTestBase {
 
   // Creates a time view with horizontal or vertical |clock_layout|.
   void CreateTimeView(TimeView::ClockLayout clock_layout) {
-    time_view_.reset(
-        new TimeView(clock_layout, Shell::Get()->system_tray_model()->clock()));
+    time_view_.reset(new TimeView(clock_layout));
   }
 
  private:

ash/system/date/system_info_default_view.cc

@@ -6,9 +6,7 @@
 
 #include <memory>
 
-#include "ash/shell.h"
 #include "ash/system/date/date_view.h"
-#include "ash/system/model/system_tray_model.h"
 #include "ash/system/power/power_status.h"
 #include "ash/system/power/power_status_view.h"
 #include "ash/system/tray/tray_constants.h"
@@ -36,8 +34,7 @@ SystemInfoDefaultView::SystemInfoDefaultView(SystemTrayItem* owner)
   AddChildView(tri_view_);
   SetLayoutManager(std::make_unique<views::FillLayout>());
 
-  date_view_ =
-      new tray::DateView(owner, Shell::Get()->system_tray_model()->clock());
+  date_view_ = new tray::DateView(owner);
   tri_view_->AddView(TriView::Container::START, date_view_);
 
   if (PowerStatus::Get()->IsBatteryPresent()) {

ash/system/date/tray_system_info.cc

@@ -19,9 +19,14 @@ namespace ash {
 TraySystemInfo::TraySystemInfo(SystemTray* system_tray)
     : SystemTrayItem(system_tray, UMA_DATE),
       tray_view_(nullptr),
-      default_view_(nullptr) {}
+      default_view_(nullptr),
+      login_status_(LoginStatus::NOT_LOGGED_IN) {
+  Shell::Get()->system_tray_model()->clock()->AddObserver(this);
+}
 
-TraySystemInfo::~TraySystemInfo() = default;
+TraySystemInfo::~TraySystemInfo() {
+  Shell::Get()->system_tray_model()->clock()->RemoveObserver(this);
+}
 
 const tray::TimeView* TraySystemInfo::GetTimeTrayForTesting() const {
   return tray_view_;
@@ -41,8 +46,7 @@ views::View* TraySystemInfo::CreateTrayView(LoginStatus status) {
       system_tray()->shelf()->IsHorizontalAlignment()
           ? tray::TimeView::ClockLayout::HORIZONTAL_CLOCK
           : tray::TimeView::ClockLayout::VERTICAL_CLOCK;
-  tray_view_ = new tray::TimeView(clock_layout,
-                                  Shell::Get()->system_tray_model()->clock());
+  tray_view_ = new tray::TimeView(clock_layout);
   views::View* view = new TrayItemView(this);
   view->AddChildView(tray_view_);
   return view;
@@ -50,6 +54,12 @@ views::View* TraySystemInfo::CreateTrayView(LoginStatus status) {
 
 views::View* TraySystemInfo::CreateDefaultView(LoginStatus status) {
   default_view_ = new SystemInfoDefaultView(this);
+
+  // Save the login status we created the view with.
+  login_status_ = status;
+
+  OnSystemClockCanSetTimeChanged(
+      Shell::Get()->system_tray_model()->clock()->can_set_time());
   return default_view_;
 }
 
@@ -71,4 +81,34 @@ void TraySystemInfo::UpdateAfterShelfAlignmentChange() {
   }
 }
 
+void TraySystemInfo::OnDateFormatChanged() {
+  UpdateTimeFormat();
+}
+
+void TraySystemInfo::OnSystemClockTimeUpdated() {
+  UpdateTimeFormat();
+}
+
+void TraySystemInfo::OnSystemClockCanSetTimeChanged(bool can_set_time) {
+  // Outside of a logged-in session, the date button should launch the set time
+  // dialog if the time can be set.
+  if (default_view_ && login_status_ == LoginStatus::NOT_LOGGED_IN) {
+    default_view_->GetDateView()->SetAction(
+        can_set_time ? tray::DateView::DateAction::SET_SYSTEM_TIME
+                     : tray::DateView::DateAction::NONE);
+  }
+}
+
+void TraySystemInfo::Refresh() {
+  if (tray_view_)
+    tray_view_->UpdateText();
+}
+
+void TraySystemInfo::UpdateTimeFormat() {
+  if (tray_view_)
+    tray_view_->UpdateTimeFormat();
+  if (default_view_)
+    default_view_->GetDateView()->UpdateTimeFormat();
+}
+
 }  // namespace ash

ash/system/date/tray_system_info.h

@@ -26,7 +26,7 @@ class TimeView;
 
 // The bottom row of the system menu. The default view shows the current date
 // and power status. The tray view shows the current time.
-class ASH_EXPORT TraySystemInfo : public SystemTrayItem {
+class ASH_EXPORT TraySystemInfo : public SystemTrayItem, public ClockObserver {
  public:
   explicit TraySystemInfo(SystemTray* system_tray);
   ~TraySystemInfo() override;
@@ -43,10 +43,18 @@ class ASH_EXPORT TraySystemInfo : public SystemTrayItem {
   void OnDefaultViewDestroyed() override;
   void UpdateAfterShelfAlignmentChange() override;
 
+  // ClockObserver:
+  void OnDateFormatChanged() override;
+  void OnSystemClockTimeUpdated() override;
+  void OnSystemClockCanSetTimeChanged(bool can_set_time) override;
+  void Refresh() override;
+
   void SetupLabelForTimeTray(views::Label* label);
+  void UpdateTimeFormat();
 
   tray::TimeView* tray_view_;
   SystemInfoDefaultView* default_view_;
+  LoginStatus login_status_;
 
   DISALLOW_COPY_AND_ASSIGN(TraySystemInfo);
 };

ash/system/model/clock_model.cc

@@ -4,10 +4,7 @@
 
 #include "ash/system/model/clock_model.h"
 
-#include "ash/session/session_controller.h"
-#include "ash/shell.h"
 #include "ash/system/date/clock_observer.h"
-#include "ash/system/tray/system_tray_controller.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 
 namespace ash {
@@ -38,19 +35,6 @@ void ClockModel::SetUse24HourClock(bool use_24_hour) {
   NotifyDateFormatChanged();
 }
 
-bool ClockModel::IsLoggedIn() {
-  return Shell::Get()->session_controller()->login_status() ==
-         LoginStatus::NOT_LOGGED_IN;
-}
-
-void ClockModel::ShowDateSettings() {
-  Shell::Get()->system_tray_controller()->ShowDateSettings();
-}
-
-void ClockModel::ShowSetTimeDialog() {
-  Shell::Get()->system_tray_controller()->ShowSetTimeDialog();
-}
-
 void ClockModel::NotifyRefreshClock() {
   for (auto& observer : observers_)
     observer.Refresh();

ash/system/model/clock_model.h

@@ -27,12 +27,6 @@ class ClockModel : public chromeos::SystemClockClient::Observer,
 
   void SetUse24HourClock(bool use_24_hour);
 
-  // Return true if the session is logged in.
-  bool IsLoggedIn();
-
-  void ShowDateSettings();
-  void ShowSetTimeDialog();
-
   // Force observers to refresh clock views e.g. system is resumed or timezone
   // is changed.
   void NotifyRefreshClock();