Lock Rotation on user settings changes

ash/system/chromeos/rotation/tray_rotation_lock.cc

 // 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 "ash/system/chromeos/rotation/tray_rotation_lock.h"
 
 #include "ash/shell.h"
 #include "ash/system/tray/system_tray.h"
 #include "ash/system/tray/tray_item_more.h"
 #include "ash/wm/maximize_mode/maximize_mode_controller.h"
@@ skipping 38 matching lines @@
 }
 
 RotationLockDefaultView::~RotationLockDefaultView() {
   Shell::GetInstance()->RemoveShellObserver(this);
 }
 
 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;
 }
 
 void RotationLockDefaultView::OnMaximizeModeStarted() {
   UpdateImage();
   SetVisible(true);
 }
 
 void RotationLockDefaultView::OnMaximizeModeEnded() {
   SetVisible(false);
@@ skipping 30 matching lines @@
 
   if (on_primary_display_)
     Shell::GetInstance()->AddShellObserver(this);
 }
 
 TrayRotationLock::~TrayRotationLock() {
   if (on_primary_display_)
     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);
   return NULL;
 }
 
 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() {
   return on_primary_display_ &&
          Shell::GetInstance()->IsMaximizeModeWindowManagerEnabled() &&
          Shell::GetInstance()->maximize_mode_controller()->rotation_locked();
 }
 
 }  // namespace ash