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

ash/laser/laser_pointer_controller.cc

 // Copyright 2016 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/laser/laser_pointer_controller.h"
 
 #include "ash/common/system/chromeos/palette/palette_utils.h"
 #include "ash/laser/laser_pointer_view.h"
 #include "ash/shell.h"
 #include "ui/aura/window_event_dispatcher.h"
@@ skipping 55 matching lines @@
 
   if (event->pointer_details().pointer_type !=
       ui::EventPointerType::POINTER_TYPE_PEN)
     return;
 
   if (event->type() != ui::ET_MOUSE_DRAGGED &&
       event->type() != ui::ET_MOUSE_PRESSED &&
       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 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_) {
-    // 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);
-
     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) {
   SwitchTargetRootWindowIfNeeded(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_) {
     laser_pointer_view_.reset(new LaserPointerView(
         base::TimeDelta::FromMilliseconds(kPointLifeDurationMs), root_window));
   }
 }
 
+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
   // 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 (laser_pointer_view_->GetIsFadingAway())
+      DestroyLaserPointerView();
   }
   stationary_timer_repeat_count_++;
 }
 }  // namespace ash