Magnifier: add validation of root window.

ash/magnifier/magnification_controller.cc

Index: ash/magnifier/magnification_controller.cc
diff --git a/ash/magnifier/magnification_controller.cc b/ash/magnifier/magnification_controller.cc
index 487aa7a3221d9c4fd82fd95b4b09b715ee89f904..98a239b109658def4d7cf14b1f404ea8e04084a8 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
@@ -122,6 +127,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,
@@ -158,16 +164,22 @@ MagnificationControllerImpl::MagnificationControllerImpl()
       scale_(kNonMagnifiedScale) {
   Shell::GetInstance()->AddPreTargetHandler(this);
 
-  if (root_window_)
-    point_of_interest_ = root_window_->bounds().CenterPoint();
+  CHECK(root_window_);

[oshima, 2013/02/27 19:07:04]

CHECK/DCHECK is useful if you want to fail early, but
this case it'll fail right away, so you don't need it.

+  root_window_->AddObserver(this);
+  point_of_interest_ = root_window_->bounds().CenterPoint();
 }
 
 MagnificationControllerImpl::~MagnificationControllerImpl() {
+  DCHECK(root_window_);

[oshima, 2013/02/27 19:07:04]

ditto

+  root_window_->RemoveObserver(this);
+
   Shell::GetInstance()->RemovePreTargetHandler(this);
 }
 
 void MagnificationControllerImpl::RedrawKeepingMousePosition(
     float scale, bool animate) {
+  DCHECK(root_window_);

[oshima, 2013/02/27 19:07:04]

ditto

+
   gfx::Point mouse_in_root = point_of_interest_;
 
   // mouse_in_root is invalid value when the cursor is hidden.
@@ -187,6 +199,8 @@ void MagnificationControllerImpl::RedrawKeepingMousePosition(
 bool MagnificationControllerImpl::Redraw(const gfx::PointF& position,
                                          float scale,
                                          bool animate) {
+  DCHECK(root_window_);

[oshima, 2013/02/27 19:07:04]

ditto

+
   const gfx::PointF position_in_dip =
       ui::ConvertPointToDIP(root_window_->layer(), position);
   return RedrawDIP(position_in_dip, scale, animate);
@@ -195,6 +209,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();
 
@@ -253,6 +269,8 @@ void MagnificationControllerImpl::EnsureRectIsVisibleWithScale(
     const gfx::Rect& target_rect,
     float scale,
     bool animate) {
+  DCHECK(root_window_);

[oshima, 2013/02/27 19:07:04]

ditto

+
   const gfx::Rect target_rect_in_dip =
       ui::ConvertRectToDIP(root_window_->layer(), target_rect);
   EnsureRectIsVisibleDIP(target_rect_in_dip, scale, animate);
@@ -299,6 +317,8 @@ void MagnificationControllerImpl::EnsurePointIsVisibleWithScale(
 }
 
 void MagnificationControllerImpl::OnMouseMove(const gfx::Point& location) {
+  DCHECK(root_window_);
+
   gfx::Point mouse(location);
 
   int x = origin_.x();
@@ -353,6 +373,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 +438,42 @@ 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);

[oshima, 2013/02/27 17:21:43]

what happens when the last root window gets detroyed?

+
+    // The destroyed root window must not be active.
+    CHECK_NE(active_root_window, root_window);
+
+    SwitchTargetRootWindow(active_root_window);
+    point_of_interest_ = active_root_window->bounds().CenterPoint();
+  }
+}
+
 void MagnificationControllerImpl::SwitchTargetRootWindow(
     aura::RootWindow* new_root_window) {
+  DCHECK(root_window_);
+  DCHECK(new_root_window);
+
   if (new_root_window == root_window_)
     return;
 
+  // Stores the previous scale.
   float scale = GetScale();
 
+  // Unmagnify the previous root window.
+  root_window_->RemoveObserver(this);
   RedrawKeepingMousePosition(1.0f, true);
+
   root_window_ = new_root_window;
   RedrawKeepingMousePosition(scale, true);
+  root_window_->AddObserver(this);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -511,12 +559,15 @@ 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 (IsMagnified() && event->type() == ui::ET_MOUSE_MOVED)
+    if (current_root != root_window_) {
+      DCHECK(current_root);
+      SwitchTargetRootWindow(current_root);
+    }
+
+    if (IsMagnified() && root_window_ && event->type() == ui::ET_MOUSE_MOVED)

[oshima, 2013/02/27 19:07:04]

you don't need to check root_window_ anymore, do you?

       OnMouseMove(event->root_location());
   }
 }
@@ -543,7 +594,7 @@ void MagnificationControllerImpl::OnScrollEvent(ui::ScrollEvent* event) {
 void MagnificationControllerImpl::OnTouchEvent(ui::TouchEvent* event) {
   aura::Window* target = static_cast<aura::Window*>(event->target());
   aura::RootWindow* current_root = target->GetRootWindow();
-  if (current_root == root_window_) {
+  if (current_root && current_root == root_window_) {

[oshima, 2013/02/27 19:07:04]

when can |current_root| be NULL?

     gfx::Rect root_bounds = current_root->bounds();
     if (root_bounds.Contains(event->root_location()))
       point_of_interest_ = event->root_location();