[Extensions] Remove support for multiple windows for loginScreenUi API

Since the created window has fixed size and is not draggable,
multi-window support does not make sense anymore. We only ever expect
there to be one login screen extension showing UI at a time.

Bug: 957573
Change-Id: I90fcb01d3d8098abe2350c8773df03ce0cdedcfe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1768643
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Reviewed-by: Zakhar Voit <voit@google.com>
Cr-Commit-Position: refs/heads/master@{#690357}

chrome/browser/chromeos/extensions/login_screen/login_screen_ui/login_screen_extension_ui_handler.cc

@@ -25,7 +25,7 @@ namespace chromeos {
 namespace {
 
 const char kErrorWindowAlreadyExists[] =
-    "Can't create more than one window per extension.";
+    "Login screen extension UI already in use.";
 const char kErrorNoExistingWindow[] = "No open window to close.";
 const char kErrorNotOnLoginOrLockScreen[] =
     "Windows can only be created on the login and lock screen.";
@@ -67,6 +67,13 @@ bool CanUseLoginScreenUiApi(const extensions::Extension* extension) {
 
 }  // namespace
 
+ExtensionIdToWindowMapping::ExtensionIdToWindowMapping(
+    const std::string& extension_id,
+    std::unique_ptr<LoginScreenExtensionUiWindow> window)
+    : extension_id(extension_id), window(std::move(window)) {}
+
+ExtensionIdToWindowMapping::~ExtensionIdToWindowMapping() = default;
+
 // static
 LoginScreenExtensionUiHandler* LoginScreenExtensionUiHandler::Get(
     bool can_create) {
@@ -107,15 +114,13 @@ bool LoginScreenExtensionUiHandler::Show(const extensions::Extension* extension,
     *error = kErrorNotOnLoginOrLockScreen;
     return false;
   }
-  if (HasOpenWindow(extension->id())) {
+  if (current_window_) {
     *error = kErrorWindowAlreadyExists;
     return false;
   }
 
-  if (!HasOpenWindow()) {
-    ash::LoginScreen::Get()->GetModel()->NotifyOobeDialogState(
-        ash::OobeDialogState::EXTENSION_LOGIN);
-  }
+  ash::LoginScreen::Get()->GetModel()->NotifyOobeDialogState(
+      ash::OobeDialogState::EXTENSION_LOGIN);
 
   LoginScreenExtensionUiCreateOptions create_options(
       GetHardcodedExtensionName(extension),
@@ -124,9 +129,9 @@ bool LoginScreenExtensionUiHandler::Show(const extensions::Extension* extension,
           base::IgnoreResult(
               &LoginScreenExtensionUiHandler::RemoveWindowForExtension),
           weak_ptr_factory_.GetWeakPtr(), extension->id()));
-  std::unique_ptr<LoginScreenExtensionUiWindow> window =
-      window_factory_->Create(&create_options);
-  windows_.emplace(extension->id(), std::move(window));
+
+  current_window_ = std::make_unique<ExtensionIdToWindowMapping>(
+      extension->id(), window_factory_->Create(&create_options));
 
   return true;
 }
@@ -144,26 +149,20 @@ bool LoginScreenExtensionUiHandler::Close(
 
 bool LoginScreenExtensionUiHandler::RemoveWindowForExtension(
     const std::string& extension_id) {
-  WindowMap::iterator it = windows_.find(extension_id);
-  if (it == windows_.end())
+  if (!HasOpenWindow(extension_id))
     return false;
-  windows_.erase(it);
 
-  if (!HasOpenWindow()) {
-    ash::LoginScreen::Get()->GetModel()->NotifyOobeDialogState(
-        ash::OobeDialogState::HIDDEN);
-  }
+  current_window_.reset(nullptr);
+
+  ash::LoginScreen::Get()->GetModel()->NotifyOobeDialogState(
+      ash::OobeDialogState::HIDDEN);
 
   return true;
 }
 
 bool LoginScreenExtensionUiHandler::HasOpenWindow(
     const std::string& extension_id) const {
-  return windows_.find(extension_id) != windows_.end();
-}
-
-bool LoginScreenExtensionUiHandler::HasOpenWindow() const {
-  return !windows_.empty();
+  return current_window_ && current_window_->extension_id == extension_id;
 }
 
 void LoginScreenExtensionUiHandler::UpdateSessionState() {
@@ -179,7 +178,7 @@ void LoginScreenExtensionUiHandler::UpdateSessionState() {
   login_or_lock_screen_active_ = new_login_or_lock_screen_active;
 
   if (!login_or_lock_screen_active_)
-    windows_.clear();
+    current_window_.reset(nullptr);
 }
 
 void LoginScreenExtensionUiHandler::OnSessionStateChanged() {
@@ -196,11 +195,10 @@ void LoginScreenExtensionUiHandler::OnExtensionUninstalled(
 LoginScreenExtensionUiWindow*
 LoginScreenExtensionUiHandler::GetWindowForTesting(
     const std::string& extension_id) {
-  WindowMap::iterator it = windows_.find(extension_id);
-  if (it == windows_.end())
+  if (!HasOpenWindow(extension_id))
     return nullptr;
 
-  return it->second.get();
+  return current_window_->window.get();
 }
 
 }  // namespace chromeos

chrome/browser/chromeos/extensions/login_screen/login_screen_ui/login_screen_extension_ui_handler.h

@@ -5,7 +5,6 @@
 #ifndef CHROME_BROWSER_CHROMEOS_EXTENSIONS_LOGIN_SCREEN_LOGIN_SCREEN_UI_LOGIN_SCREEN_EXTENSION_UI_HANDLER_H_
 #define CHROME_BROWSER_CHROMEOS_EXTENSIONS_LOGIN_SCREEN_LOGIN_SCREEN_UI_LOGIN_SCREEN_EXTENSION_UI_HANDLER_H_
 
-#include <map>
 #include <memory>
 #include <string>
 
@@ -28,9 +27,20 @@ namespace chromeos {
 class LoginScreenExtensionUiWindow;
 class LoginScreenExtensionUiWindowFactory;
 
+struct ExtensionIdToWindowMapping {
+  ExtensionIdToWindowMapping(
+      const std::string& extension_id,
+      std::unique_ptr<LoginScreenExtensionUiWindow> window);
+  ~ExtensionIdToWindowMapping();
+
+  const std::string extension_id;
+  std::unique_ptr<LoginScreenExtensionUiWindow> window;
+};
+
 // This class receives calls from the chrome.loginScreenUi API and manages the
-// associated windows. Windows can only be created on the login and lock screen
-// and are automatically closed when logging in or unlocking a user session.
+// associated window (only one window can be active at a time). Windows can only
+// be created on the login and lock screen and are automatically closed when
+// logging in or unlocking a user session.
 // TODO(hendrich): handle interference with other ChromeOS UI that pops up
 // during login (e.g. arc ToS).
 class LoginScreenExtensionUiHandler
@@ -62,7 +72,6 @@ class LoginScreenExtensionUiHandler
   bool RemoveWindowForExtension(const std::string& extension_id);
 
   bool HasOpenWindow(const std::string& extension_id) const;
-  bool HasOpenWindow() const;
 
   // session_manager::SessionManagerObserver
   void OnSessionStateChanged() override;
@@ -71,9 +80,6 @@ class LoginScreenExtensionUiHandler
       const std::string& extension_id);
 
  private:
-  using WindowMap =
-      std::map<std::string, std::unique_ptr<LoginScreenExtensionUiWindow>>;
-
   void UpdateSessionState();
 
   // extensions::ExtensionRegistryObserver
@@ -85,7 +91,7 @@ class LoginScreenExtensionUiHandler
 
   bool login_or_lock_screen_active_ = false;
 
-  WindowMap windows_;
+  std::unique_ptr<ExtensionIdToWindowMapping> current_window_;
 
   ScopedObserver<session_manager::SessionManager,
                  session_manager::SessionManagerObserver>

chrome/browser/chromeos/extensions/login_screen/login_screen_ui/login_screen_extension_ui_handler_unittest.cc

@@ -28,7 +28,7 @@
 namespace {
 
 const char kErrorWindowAlreadyExists[] =
-    "Can't create more than one window per extension.";
+    "Login screen extension UI already in use.";
 const char kErrorNoExistingWindow[] = "No open window to close.";
 const char kErrorNotOnLoginOrLockScreen[] =
     "Windows can only be created on the login and lock screen.";
@@ -285,7 +285,7 @@ TEST_F(LoginScreenExtensionUiHandlerUnittest, WindowClosedOnUnlock) {
   EXPECT_FALSE(ui_handler_->HasOpenWindow(extension_->id()));
 }
 
-TEST_F(LoginScreenExtensionUiHandlerUnittest, TwoExtensionsInParallel) {
+TEST_F(LoginScreenExtensionUiHandlerUnittest, OnlyOneWindow) {
   scoped_refptr<const extensions::Extension> other_extension =
       extensions::ExtensionBuilder(/*extension_name=*/"Imprivata")
           .SetID(kWhitelistedExtensionID2)
@@ -300,30 +300,24 @@ TEST_F(LoginScreenExtensionUiHandlerUnittest, TwoExtensionsInParallel) {
   EXPECT_TRUE(ui_handler_->HasOpenWindow(extension_->id()));
   EXPECT_FALSE(ui_handler_->HasOpenWindow(other_extension->id()));
 
+  // Try to open another window with extension 1.
+  CheckCannotOpenWindow(extension_.get(), kErrorWindowAlreadyExists);
+  EXPECT_TRUE(ui_handler_->HasOpenWindow(extension_->id()));
+  EXPECT_FALSE(ui_handler_->HasOpenWindow(other_extension->id()));
+
   // Open window with extension 2.
-  CheckCanOpenWindow(other_extension.get());
+  CheckCannotOpenWindow(other_extension.get(), kErrorWindowAlreadyExists);
   EXPECT_TRUE(ui_handler_->HasOpenWindow(extension_->id()));
-  EXPECT_TRUE(ui_handler_->HasOpenWindow(other_extension->id()));
+  EXPECT_FALSE(ui_handler_->HasOpenWindow(other_extension->id()));
 
   // Close window with extension 1.
   CheckCanCloseWindow(extension_.get());
   EXPECT_FALSE(ui_handler_->HasOpenWindow(extension_->id()));
-  EXPECT_TRUE(ui_handler_->HasOpenWindow(other_extension->id()));
-
-  // Close window with extension 1 again.
-  CheckCannotCloseWindow(extension_.get(), kErrorNoExistingWindow);
-  EXPECT_FALSE(ui_handler_->HasOpenWindow(extension_->id()));
-  EXPECT_TRUE(ui_handler_->HasOpenWindow(other_extension->id()));
+  EXPECT_FALSE(ui_handler_->HasOpenWindow(other_extension->id()));
 }
 
-TEST_F(LoginScreenExtensionUiHandlerUnittest, OnlyOneWindowPerExtension) {
-  // Open window with extension 1.
-  CheckCanOpenWindow(extension_.get());
-  EXPECT_TRUE(ui_handler_->HasOpenWindow(extension_->id()));
-
-  // Open window with extension 1 again.
-  CheckCannotOpenWindow(extension_.get(), kErrorWindowAlreadyExists);
-  EXPECT_TRUE(ui_handler_->HasOpenWindow(extension_->id()));
+TEST_F(LoginScreenExtensionUiHandlerUnittest, CannotCloseNoWindow) {
+  CheckCannotCloseWindow(extension_.get(), kErrorNoExistingWindow);
 }
 
 TEST_F(LoginScreenExtensionUiHandlerUnittest, ManualClose) {

chrome/test/data/extensions/api_test/login_screen_apis/extension/background.js

@@ -3,7 +3,7 @@
 // found in the LICENSE file.
 
 const cannotCreateMultipleWindowsErrorMessage =
-    'Can\'t create more than one window per extension.';
+    'Login screen extension UI already in use.';
 const cannotCloseNoWindowErrorMessage = 'No open window to close.';
 const cannotAccessLocalStorageErrorMessage =
     '"local" is not available for login screen extensions';

chrome/test/data/extensions/api_test/login_screen_apis/extension/manifest.json

 {
   "name": "Login screen APIs test extension",
-  "version": "1.18",
+  "version": "1.19",
   "manifest_version": 2,
   "description": "Login screen APIs test extension",
   "background": {

chrome/test/data/extensions/api_test/login_screen_apis/update_manifest.xml

@@ -7,6 +7,6 @@
   <app appid='oclffehlkdgibkainkilopaalpdobkan'>
     <updatecheck
         codebase='http://mock.http/extensions/api_test/login_screen_apis/extension.crx'
-        version='1.18' />
+        version='1.19' />
    </app>
 </gupdate>