chromeos: Set minimum size for Settings system web app

Ensure the app window is at least 200x100 DIPs, which ensures the
user can see the "Settings" label in the top-left corner.

Bug: 1001466
Test: added to browser_tests

Change-Id: If2edeb232b07f4fb294b862143891c8ec0d124dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1815905
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699837}

chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.cc

@@ -41,6 +41,7 @@
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/browser/ui/views/web_apps/web_app_frame_toolbar_view.h"
 #include "chrome/browser/ui/web_applications/app_browser_controller.h"
+#include "chrome/browser/ui/web_applications/system_web_app_ui_utils.h"
 #include "chromeos/constants/chromeos_features.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/service_manager_connection.h"
@@ -423,6 +424,14 @@ void BrowserNonClientFrameViewAsh::GetAccessibleNodeData(
 }
 
 gfx::Size BrowserNonClientFrameViewAsh::GetMinimumSize() const {
+  // System web apps (e.g. Settings) may have a fixed minimum size.
+  Browser* browser = browser_view()->browser();
+  if (web_app::IsSystemWebApp(browser)) {
+    gfx::Size minimum_size = web_app::GetSystemWebAppMinimumWindowSize(browser);
+    if (!minimum_size.IsEmpty())
+      return minimum_size;
+  }
+
   gfx::Size min_client_view_size(frame()->client_view()->GetMinimumSize());
   const int min_frame_width = frame_header_->GetMinimumHeaderWidth();
   int min_width = std::max(min_frame_width, min_client_view_size.width());

chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc

@@ -43,6 +43,7 @@
 #include "chrome/browser/ui/exclusive_access/fullscreen_controller.h"
 #include "chrome/browser/ui/exclusive_access/fullscreen_controller_test.h"
 #include "chrome/browser/ui/passwords/passwords_client_ui_delegate.h"
+#include "chrome/browser/ui/settings_window_manager_chromeos.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/toolbar/browser_actions_bar_browsertest.h"
 #include "chrome/browser/ui/views/bookmarks/bookmark_bar_view.h"
@@ -63,6 +64,8 @@
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/browser/ui/views/web_apps/web_app_frame_toolbar_view.h"
 #include "chrome/browser/ui/views/web_apps/web_app_menu_button.h"
+#include "chrome/browser/web_applications/system_web_app_manager.h"
+#include "chrome/browser/web_applications/web_app_provider.h"
 #include "chrome/common/web_application_info.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
@@ -350,6 +353,28 @@ IN_PROC_BROWSER_TEST_P(BrowserNonClientFrameViewAshTest,
   EXPECT_GT(min_window_size.height(), min_height_no_bookmarks);
 }
 
+IN_PROC_BROWSER_TEST_P(BrowserNonClientFrameViewAshTest,
+                       SettingsSystemWebAppHasMinimumWindowSize) {
+  // Install the Settings System Web App.
+  web_app::WebAppProvider::Get(browser()->profile())
+      ->system_web_app_manager()
+      .InstallSystemAppsForTesting();
+
+  // Open a settings window.
+  auto* settings_manager = chrome::SettingsWindowManager::GetInstance();
+  settings_manager->ShowOSSettings(browser()->profile());
+  Browser* settings_browser =
+      settings_manager->FindBrowserForProfile(browser()->profile());
+
+  // Try to set the bounds to a tiny value.
+  settings_browser->window()->SetBounds(gfx::Rect(1, 1));
+
+  // The window has a reasonable size.
+  gfx::Rect actual_bounds = settings_browser->window()->GetBounds();
+  EXPECT_LE(200, actual_bounds.width());
+  EXPECT_LE(100, actual_bounds.height());
+}
+
 // This is a regression test that session restore minimized browser should
 // re-layout the header (https://crbug.com/827444).
 IN_PROC_BROWSER_TEST_P(BrowserNonClientFrameViewAshTest,

chrome/browser/ui/web_applications/system_web_app_ui_utils.cc

@@ -7,6 +7,7 @@
 #include <string>
 #include <utility>
 
+#include "base/logging.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
@@ -15,6 +16,7 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/extensions/app_launch_params.h"
 #include "chrome/browser/ui/extensions/application_launch.h"
+#include "chrome/browser/ui/web_applications/app_browser_controller.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "chrome/browser/web_applications/system_web_app_manager.h"
 #include "chrome/browser/web_applications/web_app_provider.h"
@@ -109,4 +111,27 @@ Browser* FindSystemWebAppBrowser(Profile* profile, SystemAppType app_type) {
   return nullptr;
 }
 
+bool IsSystemWebApp(Browser* browser) {
+  DCHECK(browser);
+  return browser->app_controller() &&
+         browser->app_controller()->IsForSystemWebApp();
+}
+
+gfx::Size GetSystemWebAppMinimumWindowSize(Browser* browser) {
+  DCHECK(browser);
+  if (!browser->app_controller())
+    return gfx::Size();  // Not an app.
+
+  base::Optional<AppId> app_id = browser->app_controller()->GetAppId();
+  if (!app_id)
+    return gfx::Size();
+
+  auto* provider = WebAppProvider::Get(browser->profile());
+  if (!provider)
+    return gfx::Size();
+
+  return provider->system_web_app_manager().GetMinimumWindowSize(
+      app_id.value());
+}
+
 }  // namespace web_app

chrome/browser/ui/web_applications/system_web_app_ui_utils.h

@@ -33,6 +33,13 @@ Browser* LaunchSystemWebApp(Profile* profile,
 // not found.
 Browser* FindSystemWebAppBrowser(Profile* profile, SystemAppType app_type);
 
+// Returns true if the |browser| is a system web app.
+bool IsSystemWebApp(Browser* browser);
+
+// Returns the minimum window size for a system web app, or an empty size if
+// the app does not specify a minimum size.
+gfx::Size GetSystemWebAppMinimumWindowSize(Browser* browser);
+
 }  // namespace web_app
 
 #endif  // CHROME_BROWSER_UI_WEB_APPLICATIONS_SYSTEM_WEB_APP_UI_UTILS_H_

chrome/browser/web_applications/system_web_app_manager.cc

@@ -63,6 +63,9 @@ base::flat_map<SystemAppType, SystemAppInfo> CreateSystemWebApps() {
     infos[SystemAppType::SETTINGS].uninstall_and_replace = {
         app_list::kInternalAppIdSettings};
   }
+  // Large enough to see the heading text "Settings" in the top-left.
+  infos[SystemAppType::SETTINGS].minimum_window_size = {200, 100};
+
   if (SystemWebAppManager::IsAppEnabled(SystemAppType::TERMINAL)) {
     constexpr char kChromeTerminalPWAURL[] = "chrome://terminal/html/pwa.html";
     infos[SystemAppType::TERMINAL].install_url = GURL(kChromeTerminalPWAURL);
@@ -194,6 +197,17 @@ bool SystemWebAppManager::IsSystemWebApp(const AppId& app_id) const {
       app_id, ExternalInstallSource::kSystemInstalled);
 }
 
+gfx::Size SystemWebAppManager::GetMinimumWindowSize(const AppId& app_id) const {
+  auto app_type_it = app_id_to_app_type_.find(app_id);
+  if (app_type_it == app_id_to_app_type_.end())
+    return gfx::Size();
+  const SystemAppType& app_type = app_type_it->second;
+  auto app_info_it = system_app_infos_.find(app_type);
+  if (app_info_it == system_app_infos_.end())
+    return gfx::Size();
+  return app_info_it->second.minimum_window_size;
+}
+
 void SystemWebAppManager::SetSystemAppsForTesting(
     base::flat_map<SystemAppType, SystemAppInfo> system_apps) {
   system_app_infos_ = std::move(system_apps);
@@ -229,6 +243,15 @@ void SystemWebAppManager::OnAppsSynchronized(
   RecordExternalAppInstallResultCode(kInstallResultHistogramName,
                                      install_results);
 
+  // Build the map from installed app id to app type.
+  for (const auto& it : system_app_infos_) {
+    const SystemAppType& app_type = it.first;
+    base::Optional<AppId> app_id =
+        registrar_->LookupExternalAppId(it.second.install_url);
+    if (app_id.has_value())
+      app_id_to_app_type_[app_id.value()] = app_type;
+  }
+
   // May be called more than once in tests.
   if (!on_apps_synchronized_->is_signaled())
     on_apps_synchronized_->Signal();

chrome/browser/web_applications/system_web_app_manager.h

@@ -16,6 +16,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/one_shot_event.h"
 #include "chrome/browser/web_applications/components/pending_app_manager.h"
+#include "ui/gfx/geometry/size.h"
 #include "url/gurl.h"
 
 namespace base {
@@ -55,6 +56,11 @@ struct SystemAppInfo {
   // If specified, the apps in |uninstall_and_replace| will have their data
   // migrated to this System App.
   std::vector<AppId> uninstall_and_replace;
+
+  // Minimum window size in DIPs. Empty if the app does not have a minimum.
+  // TODO(https://github.com/w3c/manifest/issues/436): Replace with PWA manifest
+  // properties for window size.
+  gfx::Size minimum_window_size;
 };
 
 // Installs, uninstalls, and updates System Web Apps.
@@ -104,6 +110,10 @@ class SystemWebAppManager {
   // Returns whether |app_id| points to an installed System App.
   bool IsSystemWebApp(const AppId& app_id) const;
 
+  // Returns the minimum window size for |app_id| or an empty size if the app
+  // doesn't specify a minimum.
+  gfx::Size GetMinimumWindowSize(const AppId& app_id) const;
+
   const base::OneShotEvent& on_apps_synchronized() const {
     return *on_apps_synchronized_;
   }
@@ -127,6 +137,8 @@ class SystemWebAppManager {
 
   base::flat_map<SystemAppType, SystemAppInfo> system_app_infos_;
 
+  base::flat_map<AppId, SystemAppType> app_id_to_app_type_;
+
   PrefService* pref_service_;
 
   // Used to install, uninstall, and update apps. Should outlive this class.