stylus: Hide palette tray unless stylus event seen before.

This CL does not show the palette tray unless a stylus event has been previous seen (unless the device contains an internal stylus). Stores a pref to track whether a stylus has been seen before and if not, a pointer watcher is fired up to look for stylus events.

Bug: 740298
Test: ash_unittests "PaletteTray*Test.*"
Change-Id: I75d03dd382321a4f4e2324a5c9bec43b7b3c02c3
Reviewed-on: https://chromium-review.googlesource.com/592228
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491873}

ash/public/cpp/ash_pref_names.cc

@@ -8,6 +8,9 @@ namespace ash {
 
 namespace prefs {
 
+// A boolean pref which stores whether a stylus has been seen before.
+const char kHasSeenStylus[] = "ash.has_seen_stylus";
+
 // A boolean pref storing the enabled status of the NightLight feature.
 const char kNightLightEnabled[] = "ash.night_light.enabled";
 

ash/public/cpp/ash_pref_names.h

@@ -11,6 +11,7 @@ namespace ash {
 
 namespace prefs {
 
+ASH_PUBLIC_EXPORT extern const char kHasSeenStylus[];
 ASH_PUBLIC_EXPORT extern const char kNightLightEnabled[];
 ASH_PUBLIC_EXPORT extern const char kNightLightTemperature[];
 ASH_PUBLIC_EXPORT extern const char kNightLightScheduleType[];

ash/shell.cc

@@ -76,6 +76,7 @@
 #include "ash/system/network/sms_observer.h"
 #include "ash/system/network/vpn_list.h"
 #include "ash/system/night_light/night_light_controller.h"
+#include "ash/system/palette/palette_tray.h"
 #include "ash/system/power/power_event_observer.h"
 #include "ash/system/power/power_status.h"
 #include "ash/system/power/video_activity_notifier.h"
@@ -329,6 +330,7 @@ bool Shell::ShouldUseIMEService() {
 
 // static
 void Shell::RegisterLocalStatePrefs(PrefRegistrySimple* registry) {
+  PaletteTray::RegisterLocalStatePrefs(registry);
   WallpaperController::RegisterLocalStatePrefs(registry);
 }
 

ash/system/palette/palette_tray.cc

@@ -6,12 +6,15 @@
 
 #include "ash/accelerators/accelerator_controller.h"
 #include "ash/accessibility_delegate.h"
+#include "ash/public/cpp/ash_pref_names.h"
+#include "ash/public/cpp/config.h"
 #include "ash/resources/vector_icons/vector_icons.h"
 #include "ash/root_window_controller.h"
 #include "ash/session/session_controller.h"
 #include "ash/shelf/shelf.h"
 #include "ash/shelf/shelf_constants.h"
 #include "ash/shell.h"
+#include "ash/shell_port.h"
 #include "ash/strings/grit/ash_strings.h"
 #include "ash/system/palette/palette_tool_manager.h"
 #include "ash/system/palette/palette_utils.h"
@@ -24,12 +27,16 @@
 #include "ash/system/tray/tray_popup_item_style.h"
 #include "ash/system/tray/tray_popup_utils.h"
 #include "base/metrics/histogram_macros.h"
+#include "components/prefs/pref_change_registrar.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/pref_service.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/events/devices/input_device_manager.h"
 #include "ui/events/devices/stylus_state.h"
+#include "ui/events/event_constants.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/geometry/insets.h"
 #include "ui/gfx/paint_vector_icon.h"
@@ -38,6 +45,7 @@
 #include "ui/views/controls/separator.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/fill_layout.h"
+#include "ui/views/pointer_watcher.h"
 
 namespace ash {
 
@@ -140,6 +148,36 @@ class TitleView : public views::View, public views::ButtonListener {
 
 }  // namespace
 
+// StylusWatcher is used to monitor for stylus events, since we only want to
+// make the palette tray visible for devices without internal styluses once they
+// start using the stylus.
+class PaletteTray::StylusWatcher : views::PointerWatcher {
+ public:
+  explicit StylusWatcher(PrefService* pref_service)
+      : local_state_pref_service_(pref_service) {
+    ShellPort::Get()->AddPointerWatcher(this,
+                                        views::PointerWatcherEventTypes::BASIC);
+  }
+
+  ~StylusWatcher() override { ShellPort::Get()->RemovePointerWatcher(this); }
+
+  // views::PointerWatcher:
+  void OnPointerEventObserved(const ui::PointerEvent& event,
+                              const gfx::Point& location_in_screen,
+                              gfx::NativeView target) override {
+    if (event.pointer_details().pointer_type ==
+        ui::EventPointerType::POINTER_TYPE_PEN) {
+      if (local_state_pref_service_)
+        local_state_pref_service_->SetBoolean(prefs::kHasSeenStylus, true);
+    }
+  }
+
+ private:
+  PrefService* local_state_pref_service_ = nullptr;  // Not owned.
+
+  DISALLOW_COPY_AND_ASSIGN(StylusWatcher);
+};
+
 PaletteTray::PaletteTray(Shelf* shelf)
     : TrayBackgroundView(shelf),
       palette_tool_manager_(new PaletteToolManager(this)),
@@ -170,6 +208,11 @@ PaletteTray::~PaletteTray() {
   Shell::Get()->RemoveShellObserver(this);
 }
 
+// static
+void PaletteTray::RegisterLocalStatePrefs(PrefRegistrySimple* registry) {
+  registry->RegisterBooleanPref(prefs::kHasSeenStylus, false);
+}
+
 bool PaletteTray::ContainsPointInScreen(const gfx::Point& point) {
   if (icon_ && icon_->GetBoundsInScreen().Contains(point))
     return true;
@@ -340,6 +383,27 @@ void PaletteTray::Initialize() {
   // which will take care of showing the palette.
   palette_enabled_subscription_ = delegate->AddPaletteEnableListener(base::Bind(
       &PaletteTray::OnPaletteEnabledPrefChanged, weak_factory_.GetWeakPtr()));
+
+  local_state_pref_service_ = Shell::Get()->GetLocalStatePrefService();
+
+  // |local_state_pref_service_| may be null in tests.
+  // TODO(crbug.com/751191): Remove the check for Mash.
+  if (Shell::GetAshConfig() == Config::MASH || !local_state_pref_service_)
+    return;
+
+  // If a device has an internal stylus, mark the has seen stylus flag as true
+  // since we know the user has a stylus.
+  if (palette_utils::HasInternalStylus())
+    local_state_pref_service_->SetBoolean(prefs::kHasSeenStylus, true);
+
+  pref_change_registrar_ = base::MakeUnique<PrefChangeRegistrar>();
+  pref_change_registrar_->Init(local_state_pref_service_);
+  pref_change_registrar_->Add(
+      prefs::kHasSeenStylus,
+      base::Bind(&PaletteTray::OnHasSeenStylusPrefChanged,
+                 base::Unretained(this)));
+
+  OnHasSeenStylusPrefChanged();
 }
 
 bool PaletteTray::PerformAction(const ui::Event& event) {
@@ -443,9 +507,29 @@ void PaletteTray::OnPaletteEnabledPrefChanged(bool enabled) {
   }
 }
 
+void PaletteTray::OnHasSeenStylusPrefChanged() {
+  DCHECK(local_state_pref_service_);
+
+  has_seen_stylus_ =
+      local_state_pref_service_->GetBoolean(prefs::kHasSeenStylus);
+
+  // On reading the pref, do not bother monitoring stylus events if the device
+  // has seen a stylus event before, otherwise start monitoring for stylus
+  // events.
+  // TODO(sammiequon): Investigate if we can avoid starting the watcher if the
+  // device is not compatible with stylus.
+  if (has_seen_stylus_)
+    watcher_.reset();
+  else
+    watcher_ = base::MakeUnique<StylusWatcher>(local_state_pref_service_);
+
+  UpdateIconVisibility();
+}
+
 void PaletteTray::UpdateIconVisibility() {
-  SetVisible(is_palette_enabled_ && palette_utils::HasStylusInput() &&
-             ShouldShowOnDisplay(this) && IsInUserSession());
+  SetVisible(has_seen_stylus_ && is_palette_enabled_ &&
+             palette_utils::HasStylusInput() && ShouldShowOnDisplay(this) &&
+             IsInUserSession());
 }
 
 // TestApi. For testing purposes.

ash/system/palette/palette_tray.h

@@ -17,6 +17,10 @@
 #include "base/memory/weak_ptr.h"
 #include "ui/events/devices/input_device_event_observer.h"
 
+class PrefChangeRegistrar;
+class PrefRegistrySimple;
+class PrefService;
+
 namespace gfx {
 class Point;
 }
@@ -54,6 +58,8 @@ class ASH_EXPORT PaletteTray : public TrayBackgroundView,
       return palette_tray_->bubble_.get();
     }
 
+    bool IsStylusWatcherActive() { return !!palette_tray_->watcher_; }
+
    private:
     PaletteTray* palette_tray_ = nullptr;  // not owned
 
@@ -63,6 +69,8 @@ class ASH_EXPORT PaletteTray : public TrayBackgroundView,
   explicit PaletteTray(Shelf* shelf);
   ~PaletteTray() override;
 
+  static void RegisterLocalStatePrefs(PrefRegistrySimple* registry);
+
   // SessionObserver:
   void OnSessionStateChanged(session_manager::SessionState state) override;
 
@@ -91,6 +99,8 @@ class ASH_EXPORT PaletteTray : public TrayBackgroundView,
   bool ContainsPointInScreen(const gfx::Point& point);
 
  private:
+  class StylusWatcher;
+
   // ui::InputDeviceObserver:
   void OnTouchscreenDeviceConfigurationChanged() override;
   void OnStylusStateChanged(ui::StylusState stylus_state) override;
@@ -120,19 +130,27 @@ class ASH_EXPORT PaletteTray : public TrayBackgroundView,
   // Called when the palette enabled pref has changed.
   void OnPaletteEnabledPrefChanged(bool enabled);
 
+  // Called when the has seen stylus pref has changed.
+  void OnHasSeenStylusPrefChanged();
+
   std::unique_ptr<PaletteToolManager> palette_tool_manager_;
   std::unique_ptr<TrayBubbleWrapper> bubble_;
+  std::unique_ptr<StylusWatcher> watcher_;
 
   // Manages the callback OnPaletteEnabledPrefChanged callback registered to
   // the PaletteDelegate instance.
   std::unique_ptr<PaletteDelegate::EnableListenerSubscription>
       palette_enabled_subscription_;
 
+  PrefService* local_state_pref_service_ = nullptr;  // Not owned.
+  std::unique_ptr<PrefChangeRegistrar> pref_change_registrar_;
+
   // Weak pointer, will be parented by TrayContainer for its lifetime.
   views::ImageView* icon_;
 
-  // Cached palette enabled/disabled pref value.
+  // Cached palette pref values.
   bool is_palette_enabled_ = true;
+  bool has_seen_stylus_ = false;
 
   // Used to indicate whether the palette bubble is automatically opened by a
   // stylus eject event.

ash/system/palette/palette_tray_unittest.cc

@@ -5,14 +5,21 @@
 #include "ash/system/palette/palette_tray.h"
 
 #include "ash/ash_switches.h"
+#include "ash/public/cpp/ash_pref_names.h"
+#include "ash/public/cpp/config.h"
+#include "ash/shell.h"
 #include "ash/shell_test_api.h"
 #include "ash/system/palette/test_palette_delegate.h"
 #include "ash/system/status_area_widget.h"
 #include "ash/system/status_area_widget_test_helper.h"
 #include "ash/test/ash_test_base.h"
+#include "ash/test/ash_test_helper.h"
+#include "ash/test_shell_delegate.h"
 #include "base/command_line.h"
 #include "base/memory/ptr_util.h"
+#include "components/prefs/testing_pref_service.h"
 #include "ui/events/event.h"
+#include "ui/events/test/event_generator.h"
 
 namespace ash {
 
@@ -29,11 +36,30 @@ class PaletteTrayTest : public AshTestBase {
 
     AshTestBase::SetUp();
 
+    Shell::RegisterLocalStatePrefs(pref_service_.registry());
+    ash_test_helper()->test_shell_delegate()->set_local_state_pref_service(
+        &pref_service_);
+
     palette_tray_ =
         StatusAreaWidgetTestHelper::GetStatusAreaWidget()->palette_tray();
     test_api_ = base::MakeUnique<PaletteTray::TestApi>(palette_tray_);
 
+    // Set the test palette delegate here, since this requires an instance of
+    // shell to be available.
     ShellTestApi().SetPaletteDelegate(base::MakeUnique<TestPaletteDelegate>());
+    // Initialize the palette tray again since this test requires information
+    // from the palette delegate. (It was initialized without the delegate in
+    // AshTestBase::SetUp()).
+    palette_tray_->Initialize();
+  }
+
+  // Adds the command line flag which states this device has an internal stylus.
+  void InitForInternalStylus() {
+    base::CommandLine::ForCurrentProcess()->AppendSwitch(
+        switches::kHasInternalStylus);
+    // Initialize the palette tray again so the changes from adding this switch
+    // are applied.
+    palette_tray_->Initialize();
   }
 
   // Performs a tap on the palette tray button.
@@ -45,6 +71,7 @@ class PaletteTrayTest : public AshTestBase {
 
  protected:
   PaletteTray* palette_tray_ = nullptr;  // not owned
+  TestingPrefServiceSimple pref_service_;
 
   std::unique_ptr<PaletteTray::TestApi> test_api_;
 
@@ -52,9 +79,69 @@ class PaletteTrayTest : public AshTestBase {
   DISALLOW_COPY_AND_ASSIGN(PaletteTrayTest);
 };
 
-// Verify the palette tray button exists and is visible with the flags we added
-// during setup.
-TEST_F(PaletteTrayTest, PaletteTrayIsVisible) {
+// Verify the palette tray button exists and but is not visible initially.
+TEST_F(PaletteTrayTest, PaletteTrayIsInvisible) {
+  ASSERT_TRUE(palette_tray_);
+  EXPECT_FALSE(palette_tray_->visible());
+}
+
+// Verify that if the has seen stylus pref is not set initially, the palette
+// tray's touch event watcher should be active.
+TEST_F(PaletteTrayTest, PaletteTrayStylusWatcherAlive) {
+  // TODO(crbug.com/751191): Remove the check for Mash.
+  if (Shell::GetAshConfig() == Config::MASH)
+    return;
+
+  ASSERT_FALSE(palette_tray_->visible());
+
+  EXPECT_TRUE(test_api_->IsStylusWatcherActive());
+}
+
+// Verify if the has seen stylus pref is not set initially, the palette tray
+// should become visible after seeing a stylus event.
+TEST_F(PaletteTrayTest, PaletteTrayVisibleAfterStylusSeen) {
+  // TODO(crbug.com/751191): Remove the check for Mash.
+  if (Shell::GetAshConfig() == Config::MASH)
+    return;
+
+  ASSERT_FALSE(palette_tray_->visible());
+  ASSERT_FALSE(pref_service_.GetBoolean(prefs::kHasSeenStylus));
+  ASSERT_TRUE(test_api_->IsStylusWatcherActive());
+
+  // Send a stylus event.
+  GetEventGenerator().EnterPenPointerMode();
+  GetEventGenerator().PressTouch();
+  GetEventGenerator().ReleaseTouch();
+  GetEventGenerator().ExitPenPointerMode();
+
+  // Verify that the palette tray is now visible, the stylus event watcher is
+  // inactive and that the has seen stylus pref is now set to true.
+  EXPECT_TRUE(palette_tray_->visible());
+  EXPECT_FALSE(test_api_->IsStylusWatcherActive());
+}
+
+// Verify if the has seen stylus pref is initially set, the palette tray is
+// visible.
+TEST_F(PaletteTrayTest, StylusSeenPrefInitiallySet) {
+  // TODO(crbug.com/751191): Remove the check for Mash.
+  if (Shell::GetAshConfig() == Config::MASH)
+    return;
+
+  ASSERT_FALSE(palette_tray_->visible());
+  pref_service_.SetBoolean(prefs::kHasSeenStylus, true);
+
+  EXPECT_TRUE(palette_tray_->visible());
+  EXPECT_FALSE(test_api_->IsStylusWatcherActive());
+}
+
+// Verify the palette tray button exists and is visible if the device has an
+// internal stylus.
+TEST_F(PaletteTrayTest, PaletteTrayIsVisibleForInternalStylus) {
+  // TODO(crbug.com/751191): Remove the check for Mash.
+  if (Shell::GetAshConfig() == Config::MASH)
+    return;
+
+  InitForInternalStylus();
   ASSERT_TRUE(palette_tray_);
   EXPECT_TRUE(palette_tray_->visible());
 }

ash/test_shell_delegate.cc

@@ -140,7 +140,7 @@ PrefService* TestShellDelegate::GetActiveUserPrefService() const {
 }
 
 PrefService* TestShellDelegate::GetLocalStatePrefService() const {
-  return nullptr;
+  return local_state_pref_service_;
 }
 
 bool TestShellDelegate::IsTouchscreenEnabledInPrefs(

ash/test_shell_delegate.h

@@ -34,6 +34,10 @@ class TestShellDelegate : public ShellDelegate {
     active_user_pref_service_ = pref_service;
   }
 
+  void set_local_state_pref_service(PrefService* pref_service) {
+    local_state_pref_service_ = pref_service;
+  }
+
   // Overridden from ShellDelegate:
   ::service_manager::Connector* GetShellConnector() const override;
   bool IsIncognitoAllowed() const override;
@@ -82,6 +86,7 @@ class TestShellDelegate : public ShellDelegate {
   bool media_sessions_suspended_ = false;
   std::unique_ptr<ShelfInitializer> shelf_initializer_;
   PrefService* active_user_pref_service_ = nullptr;  // Not owned.
+  PrefService* local_state_pref_service_ = nullptr;  // Not owned.
 
   DISALLOW_COPY_AND_ASSIGN(TestShellDelegate);
 };