Lock Rotation on user settings changes

ash/system/chromeos/rotation/tray_rotation_lock.cc

Index: ash/system/chromeos/rotation/tray_rotation_lock.cc
diff --git a/ash/system/chromeos/rotation/tray_rotation_lock.cc b/ash/system/chromeos/rotation/tray_rotation_lock.cc
index 293299ad20a1a41107f35bd1bca5b13eaeef9563..61c156c5a343da330b13f6f62c19d6d776089479 100644
--- a/ash/system/chromeos/rotation/tray_rotation_lock.cc
+++ b/ash/system/chromeos/rotation/tray_rotation_lock.cc
@@ -56,15 +56,10 @@ bool RotationLockDefaultView::PerformAction(const ui::Event& event) {
   MaximizeModeController* maximize_mode_controller = Shell::GetInstance()->
       maximize_mode_controller();
   bool rotation_locked = !maximize_mode_controller->rotation_locked();
-  maximize_mode_controller->set_rotation_locked(rotation_locked);
+  maximize_mode_controller->SetRotationLocked(rotation_locked);
 
   UpdateImage();
 

[flackr, 2014/05/28 14:48:04]

nit: I don't think the empty lines are necessary to break up this short
function.

[jonross, 2014/05/28 19:13:33]

Well at one point in the past it was a fat method.

Done.

-  // RotationLockDefaultView can only be created by a TrayRotationLock. The
-  // owner needs to be told of the action so that it can update its visibility.
-  static_cast<TrayRotationLock*>(owner())->tray_view()->
-      SetVisible(rotation_locked);

[flackr, 2014/05/28 14:48:04]

Nice to be able to remove this! :-)

-
   return true;
 }
 
@@ -115,6 +110,10 @@ TrayRotationLock::~TrayRotationLock() {
     Shell::GetInstance()->RemoveShellObserver(this);
 }
 
+void TrayRotationLock::OnRotationLockChanged() {
+  tray_view()->SetVisible(GetInitialVisibility());

[flackr, 2014/05/28 14:48:04]

Can you move the logic in GetInitialVisibility to something more appropriately
named. You can call this new function from GetInitialVisibility. Perhaps
GetCurrentVisibility, or ShouldBeVisible?

[jonross, 2014/05/28 19:13:33]

Done.

+}
+
 views::View* TrayRotationLock::CreateDefaultView(user::LoginStatus status) {
   if (on_primary_display_)
     return new tray::RotationLockDefaultView(this);
@@ -124,10 +123,16 @@ views::View* TrayRotationLock::CreateDefaultView(user::LoginStatus status) {
 void TrayRotationLock::OnMaximizeModeStarted() {
   tray_view()->SetVisible(
       Shell::GetInstance()->maximize_mode_controller()->rotation_locked());
+  Shell::GetInstance()->maximize_mode_controller()->AddObserver(this);
 }
 
 void TrayRotationLock::OnMaximizeModeEnded() {
   tray_view()->SetVisible(false);
+  MaximizeModeController* controller = Shell::GetInstance()->
+      maximize_mode_controller();
+  // Upon system shutdown the controller could already be cleaned up.

[flackr, 2014/05/28 14:48:04]

Isn't the controller at least around when OnMaximizeModeEnded is called? If not,
it might make sense to make sure that maximize mode ends before the controller
is destructed.

[jonross, 2014/05/28 19:13:33]

Done.

+  if (controller)
+    controller->RemoveObserver(this);
 }
 
 bool TrayRotationLock::GetInitialVisibility() {