Fix stylus tools palette.

HandleShowStylusTools crashes at desktop ChromeOS UI and shows stylus
tools on device does not have stylus. Adding more checks when we should
HandleShowStylusTools and when to AddPaletteTray.

BUG=712887, 717674, 717422
TEST=Manual && AboutFlagsHistogramTest.CheckHistograms

Review-Url: https://codereview.chromium.org/2825383003
Cr-Commit-Position: refs/heads/master@{#469418}

ash/accelerators/accelerator_controller.cc

@@ -16,7 +16,6 @@
 #include "ash/media_controller.h"
 #include "ash/multi_profile_uma.h"
 #include "ash/new_window_controller.h"
-#include "ash/palette_delegate.h"
 #include "ash/resources/vector_icons/vector_icons.h"
 #include "ash/root_window_controller.h"
 #include "ash/rotator/window_rotation.h"
@@ -517,14 +516,7 @@ void HandleShowStylusTools() {
 }
 
 bool CanHandleShowStylusTools() {
-  StatusAreaWidget* status_area_widget = Shell::GetWmRootWindowForNewWindows()
-                                             ->GetRootWindowController()
-                                             ->GetShelf()
-                                             ->GetStatusAreaWidget();
-  // Tests (clusterfuzz) can trigger this before the status area is ready.
-  return status_area_widget && status_area_widget->palette_tray() &&
-         Shell::Get()->palette_delegate() &&
-         Shell::Get()->palette_delegate()->ShouldShowPalette();
+  return palette_utils::ShouldShowPalette();
 }
 
 void HandleSuspend() {

ash/ash_switches.cc

@@ -67,8 +67,8 @@ const char kAshDisableSmoothScreenRotation[] =
 const char kAshEstimatedPresentationDelay[] =
     "ash-estimated-presentation-delay";
 
-// Enables the palette next to the status area.
-const char kAshForceEnablePalette[] = "ash-force-enable-palette";
+// Enables the stylus tools next to the status area.
+const char kAshForceEnableStylusTools[] = "force-enable-stylus-tools";
 
 // Enables required things for the selected UI mode, regardless of whether the
 // Chromebook is currently in the selected UI mode.

ash/ash_switches.h

@@ -29,7 +29,7 @@ ASH_EXPORT extern const char kAshEnablePaletteOnAllDisplays[];
 ASH_EXPORT extern const char kAshEnableTouchView[];
 ASH_EXPORT extern const char kAshEnableMirroredScreen[];
 ASH_EXPORT extern const char kAshEstimatedPresentationDelay[];
-ASH_EXPORT extern const char kAshForceEnablePalette[];
+ASH_EXPORT extern const char kAshForceEnableStylusTools[];
 ASH_EXPORT extern const char kAshForceTabletMode[];
 ASH_EXPORT extern const char kAshForceTabletModeAuto[];
 ASH_EXPORT extern const char kAshForceTabletModeClamshell[];

ash/system/palette/palette_tray.cc

@@ -27,6 +27,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "ui/display/display.h"
 #include "ui/events/devices/input_device_manager.h"
 #include "ui/events/devices/stylus_state.h"
 #include "ui/gfx/color_palette.h"
@@ -69,6 +70,16 @@ bool IsInUserSession() {
              LoginStatus::KIOSK_APP;
 }
 
+// Returns true if the |palette_tray| is on an internal display or on every
+// display if requested from the command line.
+bool ShouldShowOnDisplay(PaletteTray* palette_tray) {
+  const display::Display& display =
+      WmWindow::Get(palette_tray->GetWidget()->GetNativeWindow())
+          ->GetDisplayNearestWindow();
+  return display.IsInternal() ||
+         palette_utils::IsPaletteEnabledOnEveryDisplay();
+}
+
 class TitleView : public views::View, public views::ButtonListener {
  public:
   explicit TitleView(PaletteTray* palette_tray) : palette_tray_(palette_tray) {
@@ -390,7 +401,7 @@ void PaletteTray::OnPaletteEnabledPrefChanged(bool enabled) {
 
 void PaletteTray::UpdateIconVisibility() {
   SetVisible(is_palette_enabled_ && palette_utils::HasStylusInput() &&
-             IsInUserSession());
+             ShouldShowOnDisplay(this) && IsInUserSession());
 }
 
 }  // namespace ash

ash/system/palette/palette_tray.h

@@ -33,7 +33,8 @@ class PaletteToolManager;
 
 // The PaletteTray shows the palette in the bottom area of the screen. This
 // class also controls the lifetime for all of the tools available in the
-// palette.
+// palette. PaletteTray has one instance per-display. It is only made visible if
+// the display is primary and if the device has stylus hardware.
 class ASH_EXPORT PaletteTray : public TrayBackgroundView,
                                public SessionObserver,
                                public ShellObserver,

ash/system/palette/palette_utils.cc

@@ -5,12 +5,15 @@
 #include "ash/system/palette/palette_utils.h"
 
 #include "ash/ash_switches.h"
+#include "ash/palette_delegate.h"
 #include "ash/shelf/wm_shelf.h"
+#include "ash/shell.h"
 #include "ash/shell_port.h"
 #include "ash/system/palette/palette_tray.h"
 #include "ash/system/status_area_widget.h"
 #include "ash/wm_window.h"
 #include "base/command_line.h"
+#include "ui/display/display.h"
 #include "ui/events/devices/input_device_manager.h"
 #include "ui/events/devices/touchscreen_device.h"
 #include "ui/gfx/geometry/point.h"
@@ -22,7 +25,7 @@ bool HasStylusInput() {
   // Allow the user to force enable or disable by passing a switch. If both are
   // present, enabling takes precedence over disabling.
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kAshForceEnablePalette)) {
+          switches::kAshForceEnableStylusTools)) {
     return true;
   }
 
@@ -43,6 +46,14 @@ bool IsPaletteEnabledOnEveryDisplay() {
       switches::kAshEnablePaletteOnAllDisplays);
 }
 
+bool ShouldShowPalette() {
+  return HasStylusInput() &&
+         (display::Display::HasInternalDisplay() ||
+          IsPaletteEnabledOnEveryDisplay()) &&
+         Shell::Get()->palette_delegate() &&
+         Shell::Get()->palette_delegate()->ShouldShowPalette();
+}
+
 bool PaletteContainsPointInScreen(const gfx::Point& point) {
   for (WmWindow* window : ShellPort::Get()->GetAllRootWindows()) {
     PaletteTray* palette_tray =

ash/system/palette/palette_utils.h

@@ -22,6 +22,11 @@ ASH_EXPORT bool HasStylusInput();
 // Returns true if the palette should be shown on every display.
 ASH_EXPORT bool IsPaletteEnabledOnEveryDisplay();
 
+// Returns true if the palette should be shown to the user. This happens when:
+// there is a stylus input, there is an internal display, and the user has not
+// disabled it in settings. This can be overridden by passing switches.
+ASH_EXPORT bool ShouldShowPalette();
+
 // Returns true if either the palette icon or the palette widget contain the
 // given point (in screen space).
 ASH_EXPORT bool PaletteContainsPointInScreen(const gfx::Point& point);

ash/system/status_area_widget.cc

@@ -4,6 +4,7 @@
 
 #include "ash/system/status_area_widget.h"
 
+#include "ash/public/cpp/config.h"
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/root_window_controller.h"
 #include "ash/shelf/wm_shelf.h"
@@ -21,6 +22,7 @@
 #include "ash/wm_window.h"
 #include "base/i18n/time_formatting.h"
 #include "ui/display/display.h"
+#include "ui/events/devices/input_device_manager.h"
 #include "ui/native_theme/native_theme_dark_aura.h"
 
 namespace ash {
@@ -196,16 +198,8 @@ void StatusAreaWidget::AddLogoutButtonTray() {
 }
 
 void StatusAreaWidget::AddPaletteTray() {
-  const display::Display& display =
-      WmWindow::Get(this->GetNativeWindow())->GetDisplayNearestWindow();
-
-  // Create the palette only on the internal display, where the stylus is
-  // available. We also create a palette on every display if requested from the
-  // command line.
-  if (display.IsInternal() || palette_utils::IsPaletteEnabledOnEveryDisplay()) {
-    palette_tray_ = new PaletteTray(wm_shelf_);
-    status_area_widget_delegate_->AddTray(palette_tray_);
-  }
+  palette_tray_ = new PaletteTray(wm_shelf_);
+  status_area_widget_delegate_->AddTray(palette_tray_);
 }
 
 void StatusAreaWidget::AddVirtualKeyboardTray() {

ash/system/status_area_widget_unittest.cc

@@ -41,9 +41,7 @@ TEST_F(StatusAreaWidgetTest, Basics) {
   EXPECT_TRUE(status->logout_button_tray_for_testing());
   EXPECT_TRUE(status->ime_menu_tray());
   EXPECT_TRUE(status->virtual_keyboard_tray_for_testing());
-
-  // Stylus palette is only constructed if hardware supports it.
-  EXPECT_FALSE(status->palette_tray());
+  EXPECT_TRUE(status->palette_tray());
 
   // Needed because WebNotificationTray updates its initial visibility
   // asynchronously.
@@ -69,7 +67,7 @@ class StatusAreaWidgetPaletteTest : public test::AshTestBase {
     // to ui::InputDeviceManager in a mus environment, which it can't.
     if (aura::Env::GetInstance()->mode() != aura::Env::Mode::MUS) {
       base::CommandLine* cmd = base::CommandLine::ForCurrentProcess();
-      cmd->AppendSwitch(switches::kAshForceEnablePalette);
+      cmd->AppendSwitch(switches::kAshForceEnableStylusTools);
       // It's difficult to write a test that marks the primary display as
       // internal before the status area is constructed. Just force the palette
       // for all displays.

chrome/browser/about_flags.cc

@@ -2609,7 +2609,7 @@ const FeatureEntry kFeatureEntries[] = {
     {"force-enable-stylus-tools",
      flag_descriptions::kForceEnableStylusToolsName,
      flag_descriptions::kForceEnableStylusToolsDescription, kOsCrOS,
-     SINGLE_VALUE_TYPE(ash::switches::kAshForceEnablePalette)},
+     SINGLE_VALUE_TYPE(ash::switches::kAshForceEnableStylusTools)},
 #endif // defined(OS_CHROMEOS)
 
     {"enable-midi-manager-dynamic-instantiation",

chrome/browser/chromeos/login/chrome_restart_request.cc

@@ -181,7 +181,7 @@ void DeriveCommandLine(const GURL& start_url,
     app_list::switches::kDisableSyncAppList,
     app_list::switches::kEnableSyncAppList,
     ash::switches::kAshEnableTouchView,
-    ash::switches::kAshForceEnablePalette,
+    ash::switches::kAshForceEnableStylusTools,
     ash::switches::kAshEnablePaletteOnAllDisplays,
     ash::switches::kAshTouchHud,
     ash::switches::kAuraLegacyPowerButton,

chrome/browser/chromeos/note_taking_helper_unittest.cc

@@ -190,7 +190,7 @@ class NoteTakingHelperTest : public BrowserWithTestWindowTest,
 
     if (flags & ENABLE_PALETTE) {
       base::CommandLine::ForCurrentProcess()->AppendSwitch(
-          ash::switches::kAshForceEnablePalette);
+          ash::switches::kAshForceEnableStylusTools);
     }
 
     // TODO(derat): Sigh, something in ArcAppTest appears to be re-enabling ARC.

tools/metrics/histograms/enums.xml

@@ -21194,7 +21194,6 @@ from previous Chrome versions.
   <int value="201343576" label="enable-password-change-support:enabled"/>
   <int value="203776499" label="enable-virtual-keyboard-overscroll"/>
   <int value="223662457" label="BackgroundLoadingForDownloads:enabled"/>
-  <int value="243557364" label="ash-force-enable-palette"/>
   <int value="244697230" label="enable-theme-color-in-tabbed-mode"/>
   <int value="262382944" label="GuestViewCrossProcessFrames:disabled"/>
   <int value="266702296" label="disable-plugin-power-saver"/>
@@ -21589,6 +21588,7 @@ from previous Chrome versions.
   <int value="1730236697" label="force-device-scale-factor"/>
   <int value="1730416578" label="NTPCondensedLayout:enabled"/>
   <int value="1731522433" label="enable-offer-store-unmasked-wallet-cards"/>
+  <int value="1733390925" label="force-enable-stylus-tools"/>
   <int value="1747279677" label="disable-delegated-renderer"/>
   <int value="1752168018" label="enable-stale-while-revalidate"/>
   <int value="1766676896" label="affiliation-based-matching:disabled"/>

ui/events/devices/input_device.cc

@@ -12,8 +12,10 @@ namespace ui {
 const int InputDevice::kInvalidId = 0;
 
 InputDevice::InputDevice()
-    : id(kInvalidId), type(InputDeviceType::INPUT_DEVICE_UNKNOWN) {
-}
+    : id(kInvalidId),
+      type(InputDeviceType::INPUT_DEVICE_UNKNOWN),
+      vendor_id(0),
+      product_id(0) {}
 
 InputDevice::InputDevice(int id, InputDeviceType type, const std::string& name)
     : id(id), type(type), name(name), vendor_id(0), product_id(0) {

ui/events/devices/touchscreen_device.cc

@@ -10,8 +10,7 @@
 
 namespace ui {
 
-TouchscreenDevice::TouchscreenDevice() : touch_points(0) {
-}
+TouchscreenDevice::TouchscreenDevice() : touch_points(0), is_stylus(false) {}
 
 TouchscreenDevice::TouchscreenDevice(int id,
                                      InputDeviceType type,
@@ -33,4 +32,6 @@ TouchscreenDevice::TouchscreenDevice(const InputDevice& input_device,
       is_stylus(false) {
 }
 
+TouchscreenDevice::~TouchscreenDevice() {}
+
 }  // namespace ui

ui/events/devices/touchscreen_device.h

@@ -28,6 +28,8 @@ struct EVENTS_DEVICES_EXPORT TouchscreenDevice : public InputDevice {
                     const gfx::Size& size,
                     int touch_points);
 
+  ~TouchscreenDevice() override;
+
   gfx::Size size;    // Size of the touch screen area.
   int touch_points;  // The number of touch points this device supports (0 if
                      // unknown).