I need this patch (https://codereview.chromium.org/12328030/) to land the CL below, so I'm taking over.

I removed a few NULL checks that aren't necessary.

Changes:
- Switch root window if the root window it is currently working on has been destroyed.
- Fixes a bug in OnMouseEvent(), changing the order of assigning to |point_of_interest_|. 
- Do not redraw the root window if that root window is being destroyed.

BUG=176335
TEST=interactive_ui_tests doesn't crash with https://codereview.chromium.org/12218045/


Review URL: https://chromiumcodereview.appspot.com/12314163

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@185105 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 54 insertions, 10 deletions

diff --git a/ash/magnifier/magnification_controller.cc b/ash/magnifier/magnification_controller.cc
index 487aa7a..ba1de9e 100644
--- a/ash/magnifier/magnification_controller.cc
+++ b/ash/magnifier/magnification_controller.cc
@@ -4,6 +4,7 @@
 
 #include "ash/magnifier/magnification_controller.h"
 
+#include "ash/display/display_controller.h"
 #include "ash/shell.h"
 #include "ash/shell_delegate.h"
 #include "ash/system/tray/system_tray_delegate.h"
@@ -46,7 +47,8 @@ namespace ash {
 
 class MagnificationControllerImpl : virtual public MagnificationController,
                                     public ui::EventHandler,
-                                    public ui::ImplicitAnimationObserver {
+                                    public ui::ImplicitAnimationObserver,
+                                    public aura::WindowObserver {
  public:
   MagnificationControllerImpl();
   virtual ~MagnificationControllerImpl();
@@ -70,6 +72,9 @@ class MagnificationControllerImpl : virtual public MagnificationController,
   // ui::ImplicitAnimationObserver overrides:
   virtual void OnImplicitAnimationsCompleted() OVERRIDE;
 
+  // aura::WindowObserver overrides:
+  virtual void OnWindowDestroying(aura::Window* root_window) OVERRIDE;
+
   // Redraws the magnification window with the given origin position and the
   // given scale. Returns true if the window is changed; otherwise, false.
   // These methods should be called internally just after the scale and/or
@@ -104,7 +109,8 @@ class MagnificationControllerImpl : virtual public MagnificationController,
   //  - Unzoom the current root_window.
   //  - Zoom the given new root_window |new_root_window|.
   //  - Switch the target window from current window to |new_root_window|.
-  void SwitchTargetRootWindow(aura::RootWindow* new_root_window);
+  void SwitchTargetRootWindow(aura::RootWindow* new_root_window,
+                              bool redraw_original_root_window);
 
   // Returns if the magnification scale is 1.0 or not (larger then 1.0).
   bool IsMagnified() const;
@@ -122,6 +128,7 @@ class MagnificationControllerImpl : virtual public MagnificationController,
   virtual void OnScrollEvent(ui::ScrollEvent* event) OVERRIDE;
   virtual void OnTouchEvent(ui::TouchEvent* event) OVERRIDE;
 
+  // Target root window. This must not be NULL.
   aura::RootWindow* root_window_;
 
   // True if the magnified window is currently animating a change. Otherwise,
@@ -157,12 +164,13 @@ MagnificationControllerImpl::MagnificationControllerImpl()
       move_cursor_after_animation_(false),
       scale_(kNonMagnifiedScale) {
   Shell::GetInstance()->AddPreTargetHandler(this);
-
-  if (root_window_)
-    point_of_interest_ = root_window_->bounds().CenterPoint();
+  root_window_->AddObserver(this);
+  point_of_interest_ = root_window_->bounds().CenterPoint();
 }
 
 MagnificationControllerImpl::~MagnificationControllerImpl() {
+  root_window_->RemoveObserver(this);
+
   Shell::GetInstance()->RemovePreTargetHandler(this);
 }
 
@@ -195,6 +203,8 @@ bool MagnificationControllerImpl::Redraw(const gfx::PointF& position,
 bool MagnificationControllerImpl::RedrawDIP(const gfx::PointF& position_in_dip,
                                             float scale,
                                             bool animate) {
+  DCHECK(root_window_);
+
   float x = position_in_dip.x();
   float y = position_in_dip.y();
 
@@ -299,6 +309,8 @@ void MagnificationControllerImpl::EnsurePointIsVisibleWithScale(
 }
 
 void MagnificationControllerImpl::OnMouseMove(const gfx::Point& location) {
+  DCHECK(root_window_);
+
   gfx::Point mouse(location);
 
   int x = origin_.x();
@@ -353,6 +365,8 @@ void MagnificationControllerImpl::OnMouseMove(const gfx::Point& location) {
 
 void MagnificationControllerImpl::AfterAnimationMoveCursorTo(
     const gfx::Point& location) {
+  DCHECK(root_window_);
+
   aura::client::CursorClient* cursor_client =
       aura::client::GetCursorClient(root_window_);
   if (cursor_client) {
@@ -416,16 +430,43 @@ void MagnificationControllerImpl::OnImplicitAnimationsCompleted() {
   is_on_animation_ = false;
 }
 
+void MagnificationControllerImpl::OnWindowDestroying(
+    aura::Window* root_window) {
+  if (root_window == root_window_) {
+    // There must be at least one root window because this controller is
+    // destroyed before the root windows get destroyed.
+    DCHECK(root_window);
+
+    aura::RootWindow* active_root_window = Shell::GetActiveRootWindow();
+    CHECK(active_root_window);
+
+    // The destroyed root window must not be active.
+    CHECK_NE(active_root_window, root_window);
+    // Don't redraw the old root window as it's being destroyed.
+    SwitchTargetRootWindow(active_root_window, false);
+    point_of_interest_ = active_root_window->bounds().CenterPoint();
+  }
+}
+
 void MagnificationControllerImpl::SwitchTargetRootWindow(
-    aura::RootWindow* new_root_window) {
+    aura::RootWindow* new_root_window,
+    bool redraw_original_root_window) {
+  DCHECK(new_root_window);
+
   if (new_root_window == root_window_)
     return;
 
+  // Stores the previous scale.
   float scale = GetScale();
 
-  RedrawKeepingMousePosition(1.0f, true);
+  // Unmagnify the previous root window.
+  root_window_->RemoveObserver(this);
+  if (redraw_original_root_window)
+    RedrawKeepingMousePosition(1.0f, true);
+
   root_window_ = new_root_window;
   RedrawKeepingMousePosition(scale, true);
+  root_window_->AddObserver(this);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -511,11 +552,14 @@ void MagnificationControllerImpl::OnMouseEvent(ui::MouseEvent* event) {
   gfx::Rect root_bounds = current_root->bounds();
 
   if (root_bounds.Contains(event->root_location())) {
-    if (current_root != root_window_)
-      SwitchTargetRootWindow(current_root);
-
+    // This must be before |SwitchTargetRootWindow()|.
     point_of_interest_ = event->root_location();
 
+    if (current_root != root_window_) {
+      DCHECK(current_root);
+      SwitchTargetRootWindow(current_root, true);
+    }
+
     if (IsMagnified() && event->type() == ui::ET_MOUSE_MOVED)
       OnMouseMove(event->root_location());
   }