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..fd10f04e61da48d4a50c589e4ab25e44de9dd39a 100644
--- a/ash/laser/laser_pointer_controller.cc
+++ b/ash/laser/laser_pointer_controller.cc
@@ -73,41 +73,50 @@ 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();
+  gfx::Point event_location = event->root_location();
+  aura::Window* target = static_cast<aura::Window*>(event->target());
+  aura::Window* event_root = target->GetRootWindow();
+  aura::Window* current_window = GetCurrentRootWindow();
+
+  if (!current_window) {
+    SwitchTargetRootWindowIfNeeded(current_window);

[jdufault, 2016/09/23 23:59:19]

What is the purpose of this?

[sammiequon, 2016/09/26 19:30:38]

Done.

     return;
   }
 
-  // This will handle creating the initial laser pointer view on
-  // ET_MOUSE_PRESSED events.
-  SwitchTargetRootWindowIfNeeded(GetCurrentRootWindow());
+  // Remap point from where it was captured to the display it is actually on.
+  aura::Window::ConvertPointToTarget(event_root, current_window,

[jdufault, 2016/09/23 23:59:19]

Keep all of the point remapping logic together.

[sammiequon, 2016/09/26 19:30:38]

Done.

+                                     &event_location);
 
-  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);
+  // If the stylus is over the palette icon or widget do not display the laser.
+  if (PaletteContainsPointInScreen(event_location)) {
+    DestroyLaserPointerView();

[jdufault, 2016/09/23 23:59:19]

I think we should continuing showing the laser pointer if it is already active
and the user drags the stylus over the palette (similar to the magnifier).

[sammiequon, 2016/09/26 19:30:38]

Done.

+    return;

[jdufault, 2016/09/23 23:59:19]

We should not be creating the view in the first place if the cursor is over the
palette and the view is not already created. Right now it seems like if the user
clicks on the stylus we:

1. Create view
2. Destroy view

[sammiequon, 2016/09/26 19:30:38]

Done.

+  }
+
+  // If the stylus is pressed and the laser pointer view is currently active,
+  // create a new one and stop the timer.
+  if (event->type() == ui::ET_MOUSE_PRESSED)
+    DestroyLaserPointerView();
 
+  SwitchTargetRootWindowIfNeeded(current_window);
+
+  if (laser_pointer_view_) {
     current_mouse_location_ = event_location;
     laser_pointer_view_->AddNewPoint(current_mouse_location_);
 
     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) {
+      // Signal the view to start fading away if the mouse has been released.
+      if (event->type() == ui::ET_MOUSE_RELEASED)
+        laser_pointer_view_->SetIsFadingAway(true);
+      // Start the timer to add stationary points if dragged, or too advance the
+      // interal collections timestamps if released.

[jdufault, 2016/09/23 23:59:19]

interal -> internal

[sammiequon, 2016/09/26 19:30:38]

Done.

       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 +126,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,6 +135,16 @@ 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 nullptr.
+  stationary_timer_->Stop();
+  if (laser_pointer_view_) {
+    laser_pointer_view_->Stop();
+    laser_pointer_view_.reset();
+  }
+}
+
 void LaserPointerController::AddStationaryPoint() {
   laser_pointer_view_->AddNewPoint(current_mouse_location_);
   // We can stop repeating the timer once the mouse has been stationary for
@@ -134,6 +152,10 @@ void LaserPointerController::AddStationaryPoint() {
   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 (laser_pointer_view_->GetIsFadingAway())
+      DestroyLaserPointerView();
   }
   stationary_timer_repeat_count_++;
 }