Do not start lock screen apps state controller if stylus is missing

This delayes final step in lock screen apps initialization (observing
session state, thus preventing the state controller from reacting on
session being locked) until stylus input is detected (for one settings
UI for controlling lock screen apps is only shown for stylus enabled
devices).
Remove somewhat hacky solution where lock screen apps were disabled by
simply not reporting any app lock screen enabled.

BUG=765029

Change-Id: I214dbe09e95babdd4779268fa6abfa5c1615e80b
Reviewed-on: https://chromium-review.googlesource.com/693961
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Reviewed-by: Stefan Kuhne <skuhne@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506904}

ash/system/palette/palette_tray.cc

@@ -185,7 +185,6 @@ PaletteTray::PaletteTray(Shelf* shelf)
   tray_container()->AddChildView(icon_);
 
   Shell::Get()->AddShellObserver(this);
-  ui::InputDeviceManager::GetInstance()->AddObserver(this);
 }
 
 PaletteTray::~PaletteTray() {
@@ -394,6 +393,8 @@ void PaletteTray::Initialize() {
 
   TrayBackgroundView::Initialize();
 
+  ui::InputDeviceManager::GetInstance()->AddObserver(this);
+
   // OnPaletteEnabledPrefChanged will get called with the initial pref value,
   // which will take care of showing the palette.
   palette_enabled_subscription_ = delegate->AddPaletteEnableListener(base::Bind(

chrome/browser/chromeos/lock_screen_apps/state_controller.cc

@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "ash/public/interfaces/constants.mojom.h"
+#include "ash/system/palette/palette_utils.h"
 #include "ash/wm/window_animations.h"
 #include "base/base64.h"
 #include "base/bind.h"
@@ -144,10 +145,8 @@ void StateController::SetPrimaryProfile(Profile* profile) {
   const user_manager::User* user =
       chromeos::ProfileHelper::Get()->GetUserByProfile(profile);
   if (!user || !user->HasGaiaAccount()) {
-    if (!ready_callback_.is_null()) {
-      ready_callback_.Run();
-      ready_callback_.Reset();
-    }
+    if (!ready_callback_.is_null())
+      std::move(ready_callback_).Run();
     return;
   }
 
@@ -241,6 +240,7 @@ void StateController::InitializeWithCryptoKey(Profile* profile,
       base::MakeUnique<extensions::lock_screen_data::LockScreenItemStorage>(
           profile, g_browser_process->local_state(), crypto_key,
           base_path.AppendASCII("lock_screen_app_data"));
+  lock_screen_data_->SetSessionLocked(false);
 
   chromeos::NoteTakingHelper::Get()->SetProfileWithEnabledLockScreenApps(
       profile);
@@ -251,16 +251,33 @@ void StateController::InitializeWithCryptoKey(Profile* profile,
   app_manager_->Initialize(profile, lock_screen_profile_->GetOriginalProfile());
 
   input_devices_observer_.Add(ui::InputDeviceManager::GetInstance());
+
+  // Do not start state controller if stylus input is not present as lock
+  // screen notes apps are geared towards stylus.
+  // State controller will observe inpt device changes and continue
+  // initialization if stylus input is found.
+  if (!ash::palette_utils::HasStylusInput()) {
+    stylus_input_missing_ = true;
+
+    if (!ready_callback_.is_null())
+      std::move(ready_callback_).Run();
+    return;
+  }
+
+  InitializeWithStylusInputPresent();
+}
+
+void StateController::InitializeWithStylusInputPresent() {
+  stylus_input_missing_ = false;
+
   power_manager_client_observer_.Add(
       chromeos::DBusThreadManager::Get()->GetPowerManagerClient());
   session_observer_.Add(session_manager::SessionManager::Get());
   OnSessionStateChanged();
 
   // SessionController is fully initialized at this point.
-  if (!ready_callback_.is_null()) {
-    ready_callback_.Run();
-    ready_callback_.Reset();
-  }
+  if (!ready_callback_.is_null())
+    std::move(ready_callback_).Run();
 }
 
 void StateController::AddObserver(StateObserver* observer) {
@@ -356,6 +373,11 @@ void StateController::OnStylusStateChanged(ui::StylusState state) {
     RequestNewLockScreenNote(LockScreenNoteOrigin::kStylusEject);
 }
 
+void StateController::OnTouchscreenDeviceConfigurationChanged() {
+  if (stylus_input_missing_ && ash::palette_utils::HasStylusInput())
+    InitializeWithStylusInputPresent();
+}
+
 void StateController::BrightnessChanged(int level, bool user_initiated) {
   if (level == 0 && !user_initiated) {
     ResetNoteTakingWindowAndMoveToNextState(

chrome/browser/chromeos/lock_screen_apps/state_controller.h

@@ -135,6 +135,7 @@ class StateController : public ash::mojom::TrayActionClient,
 
   // ui::InputDeviceEventObserver:
   void OnStylusStateChanged(ui::StylusState state) override;
+  void OnTouchscreenDeviceConfigurationChanged() override;
 
   // chromeos::PowerManagerClient::Observer
   void BrightnessChanged(int level, bool user_initiated) override;
@@ -174,11 +175,15 @@ class StateController : public ash::mojom::TrayActionClient,
   // Returns whether |crypto_key| was successfully retrieved.
   bool GetUserCryptoKey(Profile* profile, std::string* crypto_key);
 
-  // Finishes lock screen apps initialization with primary user profile and
+  // Continues lock screen apps initialization with primary user profile and
   // associated encryption key to be used for encrypting user data created in
   // lock screen context.
   void InitializeWithCryptoKey(Profile* profile, const std::string& crypto_key);
 
+  // Continues lock screen apps initialization. Should be called when stylus
+  // input has been detected.
+  void InitializeWithStylusInputPresent();
+
   // Called when app manager reports that note taking availability has changed.
   void OnNoteTakingAvailabilityChanged();
 
@@ -213,6 +218,11 @@ class StateController : public ash::mojom::TrayActionClient,
 
   Profile* lock_screen_profile_ = nullptr;
 
+  // Whether lock screen apps initialization was stopped due to stylus input
+  // missing (or stylus not being otherwise enabled). If stylus availability
+  // changes due to stylus input being detected, initialization will continue.
+  bool stylus_input_missing_ = false;
+
   std::unique_ptr<extensions::lock_screen_data::LockScreenItemStorage>
       lock_screen_data_;
 

chrome/browser/chromeos/lock_screen_apps/state_controller_unittest.cc

@@ -9,6 +9,7 @@
 #include <utility>
 #include <vector>
 
+#include "ash/public/cpp/ash_switches.h"
 #include "ash/public/interfaces/tray_action.mojom.h"
 #include "base/base64.h"
 #include "base/files/file_path.h"
@@ -394,6 +395,8 @@ class LockScreenAppStateTest : public BrowserWithTestWindowTest {
 
     ASSERT_TRUE(profile_manager_.SetUp());
 
+    SetUpStylusAvailability();
+
     auto power_client = base::MakeUnique<chromeos::FakePowerManagerClient>();
     power_manager_client_ = power_client.get();
     std::unique_ptr<chromeos::DBusThreadManagerSetter> dbus_setter =
@@ -483,6 +486,18 @@ class LockScreenAppStateTest : public BrowserWithTestWindowTest {
     fake_user_manager()->AddUser(account_id);
   }
 
+  // Sets up input device manager so stylus input is present.
+  // Virtual so test fixture can override initial stylus availability.
+  virtual void SetUpStylusAvailability() { SetStylusEnabled(); }
+
+  // Adds a command line switch to enable stylus.
+  void SetStylusEnabled() {
+    base::CommandLine::ForCurrentProcess()->AppendSwitch(
+        ash::switches::kAshForceEnableStylusTools);
+    ui::test::DeviceDataManagerTestAPI devices_test_api;
+    devices_test_api.NotifyObserversTouchscreenDeviceConfigurationChanged();
+  }
+
   void InitExtensionSystem(Profile* profile) {
     extensions::TestExtensionSystem* extension_system =
         static_cast<extensions::TestExtensionSystem*>(
@@ -536,6 +551,7 @@ class LockScreenAppStateTest : public BrowserWithTestWindowTest {
         ->AddExtension(app_.get());
 
     app_manager_->SetInitialAppState(kTestAppId, enable_app_launch);
+
     SetPrimaryProfileAndWaitUntilReady();
 
     if (target_state == TrayActionState::kNotAvailable)
@@ -673,6 +689,18 @@ class LockScreenAppStateKioskUserTest : public LockScreenAppStateTest {
   DISALLOW_COPY_AND_ASSIGN(LockScreenAppStateKioskUserTest);
 };
 
+// Tests that initially do not have stylus tools set as enabled.
+class LockScreenAppStateNoStylusInputTest : public LockScreenAppStateTest {
+ public:
+  LockScreenAppStateNoStylusInputTest() = default;
+  ~LockScreenAppStateNoStylusInputTest() override = default;
+
+  void SetUpStylusAvailability() override {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(LockScreenAppStateNoStylusInputTest);
+};
+
 }  // namespace
 
 TEST_F(LockScreenAppStateKioskUserTest, SetPrimaryProfile) {
@@ -685,6 +713,64 @@ TEST_F(LockScreenAppStateKioskUserTest, SetPrimaryProfile) {
   EXPECT_EQ(0u, observer()->observed_states().size());
 }
 
+TEST_F(LockScreenAppStateNoStylusInputTest,
+       StylusDetectedAfterInitializationAndScreenLock) {
+  ui::test::DeviceDataManagerTestAPI devices_test_api;
+
+  ASSERT_TRUE(InitializeNoteTakingApp(TrayActionState::kNotAvailable, true));
+  EXPECT_EQ(TestAppManager::State::kStopped, app_manager()->state());
+  EXPECT_TRUE(LockScreenItemStorage::GetIfAllowed(profile()));
+
+  session_manager()->SetSessionState(session_manager::SessionState::LOCKED);
+
+  // Even though session was locked, test app manager is still stopped, and
+  // lock screen apps are unavailable due to stylus not being detected.
+  EXPECT_EQ(TestAppManager::State::kStopped, app_manager()->state());
+  EXPECT_EQ(TrayActionState::kNotAvailable,
+            state_controller()->GetLockScreenNoteState());
+  EXPECT_EQ(0u, observer()->observed_states().size());
+
+  // Enable stylus input.
+  SetStylusEnabled();
+
+  // Given that stylus was enabled, lock screen apps should be avaialble.
+  EXPECT_EQ(TestAppManager::State::kStarted, app_manager()->state());
+  EXPECT_EQ(TrayActionState::kAvailable,
+            state_controller()->GetLockScreenNoteState());
+  ExpectObservedStatesMatch({TrayActionState::kAvailable}, "Stylus enabled");
+  ClearObservedStates();
+
+  // Ejecting the stylus should trigger lock screen app launch.
+  devices_test_api.NotifyObserversStylusStateChanged(ui::StylusState::REMOVED);
+  ExpectObservedStatesMatch({TrayActionState::kLaunching},
+                            "Launch on stylus ejected");
+}
+
+TEST_F(LockScreenAppStateNoStylusInputTest, StylusDetectedAfterInitialization) {
+  ui::test::DeviceDataManagerTestAPI devices_test_api;
+
+  ASSERT_TRUE(InitializeNoteTakingApp(TrayActionState::kNotAvailable, true));
+  EXPECT_EQ(TestAppManager::State::kStopped, app_manager()->state());
+
+  // Enable stylus input after state controller initialization finishes, but
+  // before screen lock.
+  SetStylusEnabled();
+
+  // Given that the session is still unlocked, lock screen apps are still
+  // unavailable.
+  EXPECT_EQ(TestAppManager::State::kStopped, app_manager()->state());
+  EXPECT_EQ(TrayActionState::kNotAvailable,
+            state_controller()->GetLockScreenNoteState());
+  EXPECT_EQ(0u, observer()->observed_states().size());
+
+  // Given that the screen is locked, lock screen apps should become available.
+  session_manager()->SetSessionState(session_manager::SessionState::LOCKED);
+
+  EXPECT_EQ(TrayActionState::kAvailable,
+            state_controller()->GetLockScreenNoteState());
+  EXPECT_EQ(TestAppManager::State::kStarted, app_manager()->state());
+}
+
 TEST_F(LockScreenAppStateNotSupportedTest, NoInstance) {
   EXPECT_FALSE(lock_screen_apps::StateController::IsEnabled());
 }

chrome/browser/chromeos/note_taking_helper.cc

@@ -204,9 +204,6 @@ NoteTakingAppInfos NoteTakingHelper::GetAvailableApps(Profile* profile) {
 
 std::unique_ptr<NoteTakingAppInfo> NoteTakingHelper::GetPreferredChromeAppInfo(
     Profile* profile) {
-  if (!ash::palette_utils::HasStylusInput())
-    return nullptr;
-
   std::string preferred_app_id =
       profile->GetPrefs()->GetString(prefs::kNoteTakingAppId);
   if (LooksLikeAndroidPackageName(preferred_app_id))