cros: Laser pointer fades out on release, do not cover palette.

ash/laser/laser_pointer_controller.cc

Index: ash/laser/laser_pointer_controller.cc
diff --git a/ash/laser/laser_pointer_controller.cc b/ash/laser/laser_pointer_controller.cc
index 35a15d04f6954179b5db5f0c8f19cf5bb1a7cb5f..78f6f00b539650dc420b8c1a54b12e53f3ccabe4 100644
--- a/ash/laser/laser_pointer_controller.cc
+++ b/ash/laser/laser_pointer_controller.cc
@@ -73,41 +73,53 @@ void LaserPointerController::OnMouseEvent(ui::MouseEvent* event) {
       event->type() != ui::ET_MOUSE_RELEASED)
     return;
 
-  // Delete the LaserPointerView instance if mouse is released.
-  if (event->type() == ui::ET_MOUSE_RELEASED) {
-    stationary_timer_->Stop();
-    laser_pointer_view_->Stop();
-    laser_pointer_view_.reset();
+  aura::Window* current_window = GetCurrentRootWindow();
+
+  // Check if the current window is valid. Destroy the laser pointer view if
+  // not.
+  if (!current_window) {
+    DestroyLaserPointerView();
+    return;
+  }
+
+  // Compute the event coordinate relative to the display it is currently on
+  // (and not the one the event was captured on).
+  gfx::Point event_location = event->root_location();
+  aura::Window* target = static_cast<aura::Window*>(event->target());
+  aura::Window* event_root = target->GetRootWindow();
+  aura::Window::ConvertPointToTarget(event_root, current_window,
+                                     &event_location);
+
+  // If the stylus is over the palette icon or widget do not display the laser,

[jdufault, 2016/10/05 18:08:51]

I would say something like

// Do not start actively adding points to the laser if the stylus is over the
palette icon or widget. Note that the laser may be visible at this point if it
is fading away.

[sammiequon, 2016/10/05 21:15:22]

Done.

+  // unless it is in the process of being faded away.
+  if ((!laser_pointer_view_ || is_fading_away_) &&

[jdufault, 2016/10/05 18:08:50]

This logic is only applicable for ui::ET_MOUSE_PRESSED events, right? What about
moving this if condition inside of the if block on line 102? I think it will
make the data flow a bit easier to understand since it makes that assumption
explicit.

Actually, if you moved this logic into that if, you could just do

if (event->type() == ui::ET_MOUSE_PRESSED) {
  // Do not start a new laser session if the event is over the 
  // palette.
  if (PaletteContainsPointInScreen(event_location))
    return;

  DestroyLaserPointerView();
  SwitchTargetRootWindowIfNeeded(current_window);
}

Note that the SwitchTargetRootWindowIfNeeded is moved inside of the if. I think
this is fine because you can't move a stylus seamlessly across two displays -
there is going to be an up/down event at some point.

[sammiequon, 2016/10/05 21:15:21]

If SwitchTargetRootWindowIfNeeded is not outside the when you drag across two
windows and hold the mouse button down the laser will disappear and you will be
left trying to drag.

+      PaletteContainsPointInScreen(event_location)) {
     return;
   }
 
-  // This will handle creating the initial laser pointer view on
-  // ET_MOUSE_PRESSED events.
-  SwitchTargetRootWindowIfNeeded(GetCurrentRootWindow());
+  // If the stylus is pressed and the laser pointer view is currently active,
+  // create a new view and stop the timer.
+  if (event->type() == ui::ET_MOUSE_PRESSED)
+    DestroyLaserPointerView();
 
-  if (laser_pointer_view_) {
-    // Remap point from where it was captured to the display it is actually on.
-    gfx::Point event_location = event->root_location();
-    aura::Window* target = static_cast<aura::Window*>(event->target());
-    aura::Window* event_root = target->GetRootWindow();
-    aura::Window::ConvertPointToTarget(
-        event_root, laser_pointer_view_->GetRootWindow(), &event_location);
+  SwitchTargetRootWindowIfNeeded(current_window);
 
+  if (laser_pointer_view_) {
     current_mouse_location_ = event_location;
-    laser_pointer_view_->AddNewPoint(current_mouse_location_);
+    // Signal the view to start fading away if the mouse has been released.
+    is_fading_away_ = event->type() == ui::ET_MOUSE_RELEASED;
+    laser_pointer_view_->AddNewPoint(current_mouse_location_, is_fading_away_);
 
     stationary_timer_repeat_count_ = 0;
-    if (event->type() == ui::ET_MOUSE_DRAGGED) {
-      // Start the timer to add stationary points if dragged.
+    if (event->type() == ui::ET_MOUSE_DRAGGED ||
+        event->type() == ui::ET_MOUSE_RELEASED) {
+      // Start the timer to add stationary points if dragged, or to advance the
+      // internal collections timestamps if released.
       if (!stationary_timer_->IsRunning())
         stationary_timer_->Reset();
     }
-
-    // If the stylus is over the palette icon or widget, do not consume the
-    // event.
-    if (!PaletteContainsPointInScreen(current_mouse_location_))
-      event->StopPropagation();
   }
+  event->StopPropagation();
 }
 
 void LaserPointerController::OnWindowDestroying(aura::Window* window) {
@@ -117,8 +129,7 @@ void LaserPointerController::OnWindowDestroying(aura::Window* window) {
 void LaserPointerController::SwitchTargetRootWindowIfNeeded(
     aura::Window* root_window) {
   if (!root_window) {
-    stationary_timer_->Stop();
-    laser_pointer_view_.reset();
+    DestroyLaserPointerView();
   } else if (laser_pointer_view_) {
     laser_pointer_view_->ReparentWidget(root_window);
   } else if (enabled_) {
@@ -127,13 +138,27 @@ void LaserPointerController::SwitchTargetRootWindowIfNeeded(
   }
 }
 
+void LaserPointerController::DestroyLaserPointerView() {
+  // |stationary_timer_| should also be stopped so that it does not attempt to
+  // add points when |laser_pointer_view_| is null.
+  stationary_timer_->Stop();
+  if (laser_pointer_view_) {
+    is_fading_away_ = false;
+    laser_pointer_view_.reset();
+  }
+}
+
 void LaserPointerController::AddStationaryPoint() {
-  laser_pointer_view_->AddNewPoint(current_mouse_location_);
+  laser_pointer_view_->AddNewPoint(current_mouse_location_, is_fading_away_);
   // We can stop repeating the timer once the mouse has been stationary for
   // longer than the life of a point.
   if (stationary_timer_repeat_count_ * kAddStationaryPointsDelayMs >=
       kPointLifeDurationMs) {
     stationary_timer_->Stop();
+    // Reset the view if the timer expires and the view was in process of fading
+    // away.
+    if (is_fading_away_)
+      DestroyLaserPointerView();
   }
   stationary_timer_repeat_count_++;
 }