Always open Terminal SWA tab window with pinned home tab

Mocks at http://b/223076712#comment13 Bug: b/223076712, 1308961 Change-Id: I290ed41e14f795238a8dd0c8807c4ff694a3150d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3538162 Reviewed-by: Jason Lin <lxj@google.com> Reviewed-by: Louise Brett <loubrett@google.com> Reviewed-by: Alan Cutter <alancutter@chromium.org> Commit-Queue: Joel Hockey <joelhockey@chromium.org> Reviewed-by: Nancy Wang <nancylingwang@chromium.org> Cr-Commit-Position: refs/heads/main@{#983670}
chromium · Mar 22, 2022 · dff4f7a · dff4f7a
1 parent cbfd22c
commit dff4f7a
Show file tree

Hide file tree

Showing 7 changed files with 54 additions and 6 deletions.
diff --git a/chrome/browser/ash/crostini/crostini_terminal.cc b/chrome/browser/ash/crostini/crostini_terminal.cc
@@ -32,6 +32,7 @@
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/ash/window_properties.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/extensions/application_launch.h"
 #include "chrome/browser/ui/web_applications/system_web_app_ui_utils.h"
@@ -55,12 +56,15 @@
 
 namespace crostini {
 
+const char kTerminalHomePath[] = "html/terminal_home.html";
+
 const char kShortcutKey[] = "shortcut";
 const char kShortcutValueSSH[] = "ssh";
 const char kShortcutValueTerminal[] = "terminal";
 const char kProfileIdKey[] = "profileId";
 
 namespace {
+
 constexpr char kSettingPrefix[] = "/hterm/profiles/default/";
 const size_t kSettingPrefixSize = std::size(kSettingPrefix) - 1;
 
@@ -82,7 +86,7 @@ base::flat_map<std::string, std::string> ExtrasFromShortcutId(
 
 void LaunchTerminalImpl(Profile* profile,
                         const GURL& url,
-                        const apps::AppLaunchParams& params) {
+                        apps::AppLaunchParams params) {
   // This function is called asynchronously, so we need to check whether
   // `profile` is still valid first.
   if (g_browser_process) {
@@ -95,8 +99,24 @@ void LaunchTerminalImpl(Profile* profile,
       //
       // System Web Apps managed by Web App publisher should call
       // LaunchSystemWebAppAsync.
-      web_app::LaunchSystemWebAppImpl(profile, web_app::SystemAppType::TERMINAL,
-                                      url, params);
+
+      // Launch without a pinned home tab (settings page).
+      if (!base::FeatureList::IsEnabled(chromeos::features::kTerminalSSH) ||
+          params.disposition == WindowOpenDisposition::NEW_POPUP) {
+        web_app::LaunchSystemWebAppImpl(
+            profile, web_app::SystemAppType::TERMINAL, url, params);
+        return;
+      }
+
+      // TODO(crbug.com/1308961): Migrate to use PWA pinned home tab when ready.
+      // For TerminalSSH, if opening a new tab, first pin home tab.
+      GURL home(base::StrCat(
+          {chrome::kChromeUIUntrustedTerminalURL, kTerminalHomePath}));
+      Browser* browser = web_app::LaunchSystemWebAppImpl(
+          profile, web_app::SystemAppType::TERMINAL, home, params);
+      if (url != home) {
+        chrome::AddTabAt(browser, url, /*index=*/1, /*foreground=*/true);
+      }
       return;
     }
   }
@@ -157,7 +177,7 @@ void LaunchTerminalHome(Profile* profile, int64_t display_id) {
   LaunchTerminalWithUrl(
       profile, display_id,
       GURL(base::StrCat(
-          {chrome::kChromeUIUntrustedTerminalURL, "html/terminal_home.html"})));
+          {chrome::kChromeUIUntrustedTerminalURL, kTerminalHomePath})));
 }
 
 void LaunchTerminalWithUrl(Profile* profile,

diff --git a/chrome/browser/ash/crostini/crostini_terminal.h b/chrome/browser/ash/crostini/crostini_terminal.h
@@ -18,6 +18,8 @@ class Profile;
 
 namespace crostini {
 
+extern const char kTerminalHomePath[];
+
 extern const char kShortcutKey[];
 extern const char kShortcutValueSSH[];
 extern const char kShortcutValueTerminal[];

diff --git a/chrome/browser/ash/web_applications/terminal_system_web_app_info.cc b/chrome/browser/ash/web_applications/terminal_system_web_app_info.cc
@@ -8,6 +8,7 @@
 
 #include "ash/constants/ash_features.h"
 #include "base/feature_list.h"
+#include "base/strings/strcat.h"
 #include "chrome/browser/ash/crostini/crostini_terminal.h"
 #include "chrome/browser/ash/web_applications/system_web_app_install_utils.h"
 #include "chrome/browser/ui/browser.h"
@@ -116,3 +117,8 @@ bool TerminalSystemAppDelegate::ShouldShowTabContextMenuShortcut(
   }
   return true;
 }
+
+bool TerminalSystemAppDelegate::ShouldPinTab(GURL url) const {
+  return url == GURL(base::StrCat({chrome::kChromeUIUntrustedTerminalURL,
+                                   crostini::kTerminalHomePath}));
+}
diff --git a/chrome/browser/ash/web_applications/terminal_system_web_app_info.h b/chrome/browser/ash/web_applications/terminal_system_web_app_info.h
@@ -31,6 +31,8 @@ class TerminalSystemAppDelegate : public web_app::SystemWebAppDelegate {
       ui::SimpleMenuModel::Delegate* delegate) const override;
   bool ShouldShowTabContextMenuShortcut(Profile* profile,
                                         int command_id) const override;
+  // TODO(crbug.com/1308961): Migrate to use PWA pinned home tab when ready.
+  bool ShouldPinTab(GURL url) const override;
 };
 
 // Returns a WebAppInstallInfo used to install the app.

diff --git a/chrome/browser/ui/web_applications/system_web_app_delegate_ui_impl.cc b/chrome/browser/ui/web_applications/system_web_app_delegate_ui_impl.cc
@@ -69,8 +69,14 @@ Browser* SystemWebAppDelegate::LaunchAndNavigateSystemWebApp(
       browser->tab_strip_model()->GetWebContentsAt(0);
   if (!web_contents || web_contents->GetURL() != url ||
       GetType() == SystemAppType::HELP) {
-    web_contents = NavigateWebApplicationWindow(
-        browser, params.app_id, url, WindowOpenDisposition::CURRENT_TAB);
+    NavigateParams nav_params(browser, url, ui::PAGE_TRANSITION_AUTO_BOOKMARK);
+#if BUILDFLAG(IS_CHROMEOS_ASH)
+    // TODO(crbug.com/1308961): Migrate to use PWA pinned home tab when ready.
+    if (ShouldPinTab(url)) {
+      nav_params.tabstrip_add_types |= TabStripModel::ADD_PINNED;
+    }
+#endif  // BUILDFLAG(IS_CHROMEOS_ASH)
+    web_contents = NavigateWebAppUsingParams(params.app_id, nav_params);
     navigating = true;
   }
 

diff --git a/chrome/browser/web_applications/system_web_apps/system_web_app_delegate.cc b/chrome/browser/web_applications/system_web_apps/system_web_app_delegate.cc
@@ -147,4 +147,10 @@ bool SystemWebAppDelegate::ShouldAnimateThemeChanges() const {
 }
 #endif  // BUILDFLAG(IS_CHROMEOS)
 
+#if BUILDFLAG(IS_CHROMEOS_ASH)
+bool SystemWebAppDelegate::ShouldPinTab(GURL url) const {
+  return false;
+}
+#endif  // BUILDFLAG(IS_CHROMEOS_ASH)
+
 }  // namespace web_app
diff --git a/chrome/browser/web_applications/system_web_apps/system_web_app_delegate.h b/chrome/browser/web_applications/system_web_apps/system_web_app_delegate.h
@@ -190,6 +190,12 @@ class SystemWebAppDelegate {
   virtual bool ShouldAnimateThemeChanges() const;
 #endif  // BUILDFLAG(IS_CHROMEOS)
 
+#if BUILDFLAG(IS_CHROMEOS_ASH)
+  // TODO(crbug.com/1308961): Migrate to use PWA pinned home tab when ready.
+  // Returns whether the specified tab should be pinned.
+  virtual bool ShouldPinTab(GURL url) const;
+#endif  // BUILDFLAG(IS_CHROMEOS_ASH)
+
  protected:
   Profile* profile() const { return profile_; }