Magnifier: add validation of root window.

ash/magnifier/magnification_controller.cc

 // Copyright (c) 2012 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/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"
 #include "ui/aura/client/cursor_client.h"
 #include "ui/aura/root_window.h"
 #include "ui/aura/window.h"
 #include "ui/aura/window_property.h"
 #include "ui/base/events/event.h"
 #include "ui/base/events/event_handler.h"
 #include "ui/compositor/dip_util.h"
@@ skipping 13 matching lines @@
 const float kMinMagnifiedScaleThreshold = 1.1f;
 const float kNonMagnifiedScale = 1.0f;
 
 const float kInitialMagnifiedScale = 2.0f;
 const float kScrollScaleChangeFactor = 0.05f;
 
 // Threadshold of panning. If the cursor moves to within pixels (in DIP) of
 // |kPanningMergin| from the edge, the view-port moves.
 const int kPanningMergin = 100;
 
+bool IsRootWindowValid(aura::RootWindow* root_window) {
+  if (!root_window)
+    return false;
+
+  std::vector<aura::RootWindow*> root_windows =
+      ash::Shell::GetInstance()->display_controller()->GetAllRootWindows();
+  if (std::find(root_windows.begin(), root_windows.end(), root_window) !=
+          root_windows.end()) {
+    return true;
+  }
+
+  return false;
+}

[oshima, 2013/02/21 16:45:06]

Just

return std::find(...) != root_windows.end();

[yoshiki, 2013/02/25 12:32:44]

Done.

+
 }  // namespace
 
 namespace ash {
 
 ////////////////////////////////////////////////////////////////////////////////
 // MagnificationControllerImpl:
 
 class MagnificationControllerImpl : virtual public MagnificationController,
                                     public ui::EventHandler,
-                                    public ui::ImplicitAnimationObserver {
+                                    public ui::ImplicitAnimationObserver,
+                                    public aura::WindowObserver {
  public:
   MagnificationControllerImpl();
   virtual ~MagnificationControllerImpl();
 
   // MagnificationController overrides:
   virtual void SetEnabled(bool enabled) OVERRIDE;
   virtual bool IsEnabled() const OVERRIDE;
   virtual void SetScale(float scale, bool animate) OVERRIDE;
   virtual float GetScale() const OVERRIDE { return scale_; }
   virtual void MoveWindow(int x, int y, bool animate) OVERRIDE;
   virtual void MoveWindow(const gfx::Point& point, bool animate) OVERRIDE;
   virtual gfx::Point GetWindowPosition() const OVERRIDE {
     return gfx::ToFlooredPoint(origin_);
   }
   virtual void EnsureRectIsVisible(const gfx::Rect& rect,
                                    bool animate) OVERRIDE;
   virtual void EnsurePointIsVisible(const gfx::Point& point,
                                     bool animate) OVERRIDE;
 
  private:
   // 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
   // the position are changed to redraw the window.
   bool Redraw(const gfx::PointF& position, float scale, bool animate);
   bool RedrawDIP(const gfx::PointF& position, float scale, bool animate);
 
   // Redraw with the given zoom scale keeping the mouse cursor location. In
   // other words, zoom (or unzoom) centering around the cursor.
   void RedrawKeepingMousePosition(float scale, bool animate);
@@ skipping 16 matching lines @@
   // the animation is completed. This should be called after animation is
   // started.
   void AfterAnimationMoveCursorTo(const gfx::Point& location);
 
   // Switch Magnified RootWindow to |new_root_window|. This does following:
   //  - 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);
 
+  // Checks the current target root window is still alive or not. If it's stale,
+  // the current primary root window is newly assigned as the current target
+  // root window instead.
+  bool IsRootwindowStale();
+
   // Returns if the magnification scale is 1.0 or not (larger then 1.0).
   bool IsMagnified() const;
 
   // Returns the rect of the magnification window.
   gfx::RectF GetWindowRectDIP(float scale) const;
   // Returns the size of the root window.
   gfx::Size GetHostSizeDIP() const;
 
   // Correct the givin scale value if nessesary.
   void ValidateScale(float* scale);
@@ skipping 42 matching lines @@
   if (root_window_)
     point_of_interest_ = root_window_->bounds().CenterPoint();
 }
 
 MagnificationControllerImpl::~MagnificationControllerImpl() {
   Shell::GetInstance()->RemovePreTargetHandler(this);
 }
 
 void MagnificationControllerImpl::RedrawKeepingMousePosition(
     float scale, bool animate) {
+  CHECK(root_window_);  // |root_window_| should be checked at the caller.
+
   gfx::Point mouse_in_root = point_of_interest_;
 
   // mouse_in_root is invalid value when the cursor is hidden.
   if (!root_window_->bounds().Contains(mouse_in_root))
     mouse_in_root = root_window_->bounds().CenterPoint();
 
   const gfx::PointF origin =
       gfx::PointF(mouse_in_root.x() -
                       (scale_ / scale) * (mouse_in_root.x() - origin_.x()),
                   mouse_in_root.y() -
                       (scale_ / scale) * (mouse_in_root.y() - origin_.y()));
   bool changed = RedrawDIP(origin, scale, animate);
   if (changed)
     AfterAnimationMoveCursorTo(mouse_in_root);
 }
 
 bool MagnificationControllerImpl::Redraw(const gfx::PointF& position,
                                          float scale,
                                          bool animate) {
+  CHECK(root_window_);  // |root_window_| should be checked at the caller.
+
   const gfx::PointF position_in_dip =
       ui::ConvertPointToDIP(root_window_->layer(), position);
   return RedrawDIP(position_in_dip, scale, animate);
 }
 
 bool MagnificationControllerImpl::RedrawDIP(const gfx::PointF& position_in_dip,
                                             float scale,
                                             bool animate) {
+  CHECK(root_window_);  // |root_window_| should be checked at the caller.
+
   float x = position_in_dip.x();
   float y = position_in_dip.y();
 
   ValidateScale(&scale);
 
   if (x < 0)
     x = 0;
   if (y < 0)
     y = 0;
 
@@ skipping 38 matching lines @@
   if (animate)
     is_on_animation_ = true;
 
   return true;
 }
 
 void MagnificationControllerImpl::EnsureRectIsVisibleWithScale(
     const gfx::Rect& target_rect,
     float scale,
     bool animate) {
+  CHECK(root_window_);  // |root_window_| should be checked at the caller.
+
   const gfx::Rect target_rect_in_dip =
       ui::ConvertRectToDIP(root_window_->layer(), target_rect);
   EnsureRectIsVisibleDIP(target_rect_in_dip, scale, animate);
 }
 
 void MagnificationControllerImpl::EnsureRectIsVisibleDIP(
     const gfx::Rect& target_rect,
     float scale,
     bool animate) {
   ValidateScale(&scale);
@@ skipping 26 matching lines @@
 void MagnificationControllerImpl::EnsurePointIsVisibleWithScale(
     const gfx::Point& point,
     float scale,
     bool animate) {
   EnsureRectIsVisibleWithScale(gfx::Rect(point, gfx::Size(0, 0)),
                                scale,
                                animate);
 }
 
 void MagnificationControllerImpl::OnMouseMove(const gfx::Point& location) {
+  CHECK(root_window_);  // |root_window_| should be checked at the caller.
+
   gfx::Point mouse(location);
 
   int x = origin_.x();
   int y = origin_.y();
   bool start_zoom = false;
 
   const gfx::Rect window_rect = gfx::ToEnclosingRect(GetWindowRectDIP(scale_));
   const int left = window_rect.x();
   const int right = window_rect.right();
   int margin = kPanningMergin / scale_;  // No need to consider DPI.
@@ skipping 34 matching lines @@
     if (ret) {
       // If the magnified region is moved, hides the mouse cursor and moves it.
       if (x_diff != 0 || y_diff != 0)
         root_window_->MoveCursorTo(mouse);
     }
   }
 }
 
 void MagnificationControllerImpl::AfterAnimationMoveCursorTo(
     const gfx::Point& location) {
+  CHECK(root_window_);  // |root_window_| should be checked at the caller.
+
   aura::client::CursorClient* cursor_client =
       aura::client::GetCursorClient(root_window_);
   if (cursor_client) {
     // When cursor is invisible, do not move or show the cursor after the
     // animation.
     if (!cursor_client->IsCursorVisible())
       return;
     cursor_client->DisableMouseEvents();
   }
   move_cursor_after_animation_ = true;
@@ skipping 43 matching lines @@
 
     aura::client::CursorClient* cursor_client =
         aura::client::GetCursorClient(root_window_);
     if (cursor_client)
       cursor_client->EnableMouseEvents();
   }
 
   is_on_animation_ = false;
 }
 
+void MagnificationControllerImpl::OnWindowDestroying(
+    aura::Window* root_window) {
+  if (root_window == root_window_)
+    root_window_ = NULL;

[oshima, 2013/02/21 16:45:06]

Shouldn't you switch the root window here?

This way, can't you assume root_window_ is never been null elsewhere?

[yoshiki, 2013/02/25 12:32:44]

Done. Switching root window here.

+}
+
 void MagnificationControllerImpl::SwitchTargetRootWindow(
     aura::RootWindow* 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);

[oshima, 2013/02/21 16:45:06]

root_window_ can be NULL at this point, can't it? (with current flow)

[yoshiki, 2013/02/25 12:32:44]

Done. I've added the check.

   RedrawKeepingMousePosition(1.0f, true);
-  root_window_ = new_root_window;
-  RedrawKeepingMousePosition(scale, true);
+
+  // Ensures that |new_root_window_| is valid.
+  if (IsRootWindowValid(new_root_window)) {

[oshima, 2013/02/21 16:45:06]

Looking through call flow, this should never be false.

[yoshiki, 2013/02/25 12:32:44]

Done.

+    root_window_ = new_root_window;
+    RedrawKeepingMousePosition(scale, true);
+    root_window_->AddObserver(this);
+  } else {
+    root_window_ = NULL;
+  }
+}
+
+bool MagnificationControllerImpl::IsRootwindowStale() {

[oshima, 2013/02/21 16:45:06]

Changing a state inside predicate style method like IsXXX
is confusing. If you need to keep this method, I'd sugges
something like
EnsureValidRootWindow or something like that. My recommendation
is to switch the root in OnWindowDestorying (or Destroyed if it works), and
eliminate this method if possible.

[yoshiki, 2013/02/25 12:32:44]

Removed.

+  // If the current target root window is stale, try to switch target to the
+  // primary root window.
+  if (!IsRootWindowValid(root_window_)) {
+    root_window_ = NULL;
+
+    aura::RootWindow* primary_root_window = Shell::GetPrimaryRootWindow();
+    SwitchTargetRootWindow(primary_root_window);
+
+    // When there are no root windows.
+    if (!root_window_)
+      return true;
+
+    point_of_interest_ = root_window_->bounds().CenterPoint();
+  }
+
+  return false;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // MagnificationControllerImpl: MagnificationController implementation
 
 void MagnificationControllerImpl::SetScale(float scale, bool animate) {
-  if (!is_enabled_)
+  if (!is_enabled_ || IsRootwindowStale())
     return;
 
   ValidateScale(&scale);
   ash::Shell::GetInstance()->delegate()->SaveScreenMagnifierScale(scale);
   RedrawKeepingMousePosition(scale, animate);
 }
 
 void MagnificationControllerImpl::MoveWindow(int x, int y, bool animate) {
-  if (!is_enabled_)
+  if (!is_enabled_ || IsRootwindowStale())
     return;
 
   Redraw(gfx::Point(x, y), scale_, animate);
 }
 
 void MagnificationControllerImpl::MoveWindow(const gfx::Point& point,
                                              bool animate) {
-  if (!is_enabled_)
+  if (!is_enabled_ || IsRootwindowStale())
     return;
 
   Redraw(point, scale_, animate);
 }
 
 void MagnificationControllerImpl::EnsureRectIsVisible(
     const gfx::Rect& target_rect,
     bool animate) {
-  if (!is_enabled_)
+  if (!is_enabled_ || IsRootwindowStale())
     return;
 
   EnsureRectIsVisibleWithScale(target_rect, scale_, animate);
 }
 
 void MagnificationControllerImpl::EnsurePointIsVisible(
     const gfx::Point& point,
     bool animate) {
-  if (!is_enabled_)
+  if (!is_enabled_ || IsRootwindowStale())
     return;
 
   EnsurePointIsVisibleWithScale(point, scale_, animate);
 }
 
 void MagnificationControllerImpl::SetEnabled(bool enabled) {
+  if (IsRootwindowStale())
+    return;
+
   if (enabled) {
     float scale =
         ash::Shell::GetInstance()->delegate()->GetSavedScreenMagnifierScale();
     if (scale <= 0.0f)
       scale = kInitialMagnifiedScale;
     ValidateScale(&scale);
 
     // Do nothing, if already enabled with same scale.
     if (is_enabled_ && scale == scale_)
       return;
@@ skipping 17 matching lines @@
 
 ////////////////////////////////////////////////////////////////////////////////
 // MagnificationControllerImpl: aura::EventFilter implementation
 
 void MagnificationControllerImpl::OnMouseEvent(ui::MouseEvent* event) {
   aura::Window* target = static_cast<aura::Window*>(event->target());
   aura::RootWindow* current_root = target->GetRootWindow();
   gfx::Rect root_bounds = current_root->bounds();
 
   if (root_bounds.Contains(event->root_location())) {
+    // This must be before |SwitchTargetRootWindow()|.
+    point_of_interest_ = event->root_location();
+
     if (current_root != root_window_)
       SwitchTargetRootWindow(current_root);
 
-    point_of_interest_ = event->root_location();
-
-    if (IsMagnified() && event->type() == ui::ET_MOUSE_MOVED)
+    if (IsMagnified() && root_window_ && event->type() == ui::ET_MOUSE_MOVED)
       OnMouseMove(event->root_location());
   }
 }
 
 void MagnificationControllerImpl::OnScrollEvent(ui::ScrollEvent* event) {
   if (event->IsAltDown() && event->IsControlDown()) {
     if (event->type() == ui::ET_SCROLL_FLING_START ||
         event->type() == ui::ET_SCROLL_FLING_CANCEL) {
       event->StopPropagation();
       return;
     }
 
     if (event->type() == ui::ET_SCROLL) {
       ui::ScrollEvent* scroll_event = static_cast<ui::ScrollEvent*>(event);
       float scale = GetScale();
       scale += scroll_event->y_offset() * kScrollScaleChangeFactor;
       SetScale(scale, true);
       event->StopPropagation();
       return;
     }
   }
 }
 
 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_) {
     gfx::Rect root_bounds = current_root->bounds();
     if (root_bounds.Contains(event->root_location()))
       point_of_interest_ = event->root_location();
   }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // MagnificationController:
 
 // static
 MagnificationController* MagnificationController::CreateInstance() {
   return new MagnificationControllerImpl();
 }
 
 }  // namespace ash