desktop-pwas: LaunchService has single command line launch method

LaunchApplication() replaces OpenApplicationWindow and
OpenApplicationTab. The user's preferred launch container for
the web app (standalone window or browser tab) is used.

If the app is not installed, a browser window opens at the new tab page.

TBR=pbos@chromium.org

Bug: 1003610, 1022556
Change-Id: I1655a944ac205a93c4ebfebef8c13e21da4cd79b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1897377
Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737128}

chrome/browser/apps/apps_launch.cc

@@ -17,7 +17,7 @@ bool OpenApplicationWithReenablePrompt(
       profile, app_id, command_line, current_directory);
 }
 
-bool OpenAppShortcutWindow(Profile* profile, const GURL& url) {
+content::WebContents* OpenAppShortcutWindow(Profile* profile, const GURL& url) {
   return OpenExtensionAppShortcutWindow(profile, url);
 }
 

chrome/browser/apps/apps_launch.h

@@ -15,6 +15,10 @@ class CommandLine;
 class FilePath;
 }  // namespace base
 
+namespace content {
+class WebContents;
+}  // namespace content
+
 namespace apps {
 
 // TODO(crbug.com/966288): Move these methods into LaunchService.
@@ -26,9 +30,9 @@ bool OpenApplicationWithReenablePrompt(Profile* profile,
                                        const base::CommandLine& command_line,
                                        const base::FilePath& current_directory);
 
-// Returns true if |url| was successfully opened in a window, and false
-// otherwise.
-bool OpenAppShortcutWindow(Profile* profile, const GURL& url);
+// Returns web contents if |url| was successfully opened in a window, and
+// nullptr otherwise.
+content::WebContents* OpenAppShortcutWindow(Profile* profile, const GURL& url);
 
 }  // namespace apps
 

chrome/browser/apps/launch_service/extension_app_launch_manager.cc

@@ -7,6 +7,9 @@
 #include "base/feature_list.h"
 #include "chrome/browser/apps/platform_apps/platform_app_launch.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/extensions/application_launch.h"
 #include "chrome/browser/ui/web_applications/web_app_launch_manager.h"
 #include "chrome/common/chrome_features.h"
@@ -39,18 +42,26 @@ content::WebContents* ExtensionAppLaunchManager::OpenApplication(
   return ::OpenApplication(profile(), params);
 }
 
-bool ExtensionAppLaunchManager::OpenApplicationWindow(
+void ExtensionAppLaunchManager::LaunchApplication(
     const std::string& app_id,
     const base::CommandLine& command_line,
-    const base::FilePath& current_directory) {
-  RecordBookmarkLaunch(profile(), app_id);
-
-  return OpenExtensionApplicationWindow(profile(), app_id, command_line,
-                                        current_directory);
-}
-
-bool ExtensionAppLaunchManager::OpenApplicationTab(const std::string& app_id) {
-  return OpenExtensionApplicationTab(profile(), app_id);
+    const base::FilePath& current_directory,
+    base::OnceCallback<void(Browser* browser,
+                            apps::mojom::LaunchContainer container)> callback) {
+  apps::mojom::LaunchContainer container;
+  if (OpenExtensionApplicationWindow(profile(), app_id, command_line,
+                                     current_directory)) {
+    RecordBookmarkLaunch(profile(), app_id);
+    container = apps::mojom::LaunchContainer::kLaunchContainerWindow;
+  } else if (OpenExtensionApplicationTab(profile(), app_id)) {
+    container = apps::mojom::LaunchContainer::kLaunchContainerTab;
+  } else {
+    // Open an empty browser window as the app_id is invalid.
+    CreateNewTabBrowser();
+    container = apps::mojom::LaunchContainer::kLaunchContainerNone;
+  }
+  std::move(callback).Run(BrowserList::GetInstance()->GetLastActive(),
+                          container);
 }
 
 }  // namespace apps

chrome/browser/apps/launch_service/extension_app_launch_manager.h

@@ -22,11 +22,13 @@ class ExtensionAppLaunchManager final : public LaunchManager {
   // apps::LaunchManager:
   content::WebContents* OpenApplication(const AppLaunchParams& params) override;
 
-  bool OpenApplicationWindow(const std::string& app_id,
-                             const base::CommandLine& command_line,
-                             const base::FilePath& current_directory) override;
-
-  bool OpenApplicationTab(const std::string& app_id) override;
+  void LaunchApplication(
+      const std::string& app_id,
+      const base::CommandLine& command_line,
+      const base::FilePath& current_directory,
+      base::OnceCallback<void(Browser* browser,
+                              apps::mojom::LaunchContainer container)> callback)
+      override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ExtensionAppLaunchManager);

chrome/browser/apps/launch_service/launch_manager.cc

@@ -3,9 +3,18 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/apps/launch_service/launch_manager.h"
+
 #include "base/command_line.h"
 #include "base/files/file_path.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_navigator.h"
+#include "chrome/browser/ui/browser_navigator_params.h"
+#include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/webui_url_constants.h"
+#include "ui/base/page_transition_types.h"
+#include "url/gurl.h"
 
 namespace apps {
 
@@ -33,4 +42,19 @@ std::vector<base::FilePath> LaunchManager::GetLaunchFilesFromCommandLine(
   return launch_files;
 }
 
+Browser* LaunchManager::CreateNewTabBrowser() {
+  Browser::CreateParams create_params =
+      Browser::CreateParams(profile_, /*user_gesture=*/false);
+  Browser* browser = new Browser(create_params);
+
+  NavigateParams params(browser, GURL(chrome::kChromeUINewTabURL),
+                        ui::PAGE_TRANSITION_AUTO_TOPLEVEL);
+  params.disposition = WindowOpenDisposition::NEW_FOREGROUND_TAB;
+  params.tabstrip_add_types = TabStripModel::ADD_ACTIVE;
+  Navigate(&params);
+
+  browser->window()->Show();
+  return browser;
+}
+
 }  // namespace apps

chrome/browser/apps/launch_service/launch_manager.h

@@ -8,8 +8,11 @@
 #include <string>
 #include <vector>
 
+#include "base/callback.h"
 #include "base/macros.h"
+#include "components/services/app_service/public/mojom/types.mojom.h"
 
+class Browser;
 class Profile;
 
 namespace base {
@@ -34,14 +37,18 @@ class LaunchManager {
   virtual content::WebContents* OpenApplication(
       const AppLaunchParams& params) = 0;
 
-  // Attempt to open |app_id| in a new window.
-  virtual bool OpenApplicationWindow(
+  // Attempt to open |app_id| in a new window or tab. Open an empty browser
+  // window if unsuccessful. The user's preferred launch container for the app
+  // (standalone window or browser tab) is used. |callback| will be called with
+  // the container type used to open the app, kLaunchContainerNone if an empty
+  // browser window was opened.
+  virtual void LaunchApplication(
       const std::string& app_id,
       const base::CommandLine& command_line,
-      const base::FilePath& current_directory) = 0;
-
-  // Attempt to open |app_id| in a new tab.
-  virtual bool OpenApplicationTab(const std::string& app_id) = 0;
+      const base::FilePath& current_directory,
+      base::OnceCallback<void(Browser* browser,
+                              apps::mojom::LaunchContainer container)>
+          callback) = 0;
 
   // Converts file arguments to an app on |command_line| into base::FilePaths.
   static std::vector<base::FilePath> GetLaunchFilesFromCommandLine(
@@ -51,6 +58,10 @@ class LaunchManager {
   explicit LaunchManager(Profile*);
   Profile* profile() { return profile_; }
 
+  // When a command line launch has an unknown app id, we open a browser
+  // with only the new tab page.
+  Browser* CreateNewTabBrowser();
+
  private:
   Profile* const profile_;
   DISALLOW_COPY_AND_ASSIGN(LaunchManager);

chrome/browser/apps/launch_service/launch_service.cc

@@ -58,16 +58,14 @@ content::WebContents* LaunchService::OpenApplication(
   return GetLaunchManagerForApp(params.app_id).OpenApplication(params);
 }
 
-bool LaunchService::OpenApplicationWindow(
+void LaunchService::LaunchApplication(
     const std::string& app_id,
     const base::CommandLine& command_line,
-    const base::FilePath& current_directory) {
-  return GetLaunchManagerForApp(app_id).OpenApplicationWindow(
-      app_id, command_line, current_directory);
-}
-
-bool LaunchService::OpenApplicationTab(const std::string& app_id) {
-  return GetLaunchManagerForApp(app_id).OpenApplicationTab(app_id);
+    const base::FilePath& current_directory,
+    base::OnceCallback<void(Browser* browser,
+                            apps::mojom::LaunchContainer container)> callback) {
+  GetLaunchManagerForApp(app_id).LaunchApplication(
+      app_id, command_line, current_directory, std::move(callback));
 }
 
 }  // namespace apps

chrome/browser/apps/launch_service/launch_service.h

@@ -8,9 +8,12 @@
 #include <memory>
 #include <string>
 
+#include "base/callback.h"
 #include "base/macros.h"
 #include "components/keyed_service/core/keyed_service.h"
+#include "components/services/app_service/public/mojom/types.mojom.h"
 
+class Browser;
 class Profile;
 
 namespace base {
@@ -44,13 +47,18 @@ class LaunchService : public KeyedService {
   // Open the application in a way specified by |params|.
   content::WebContents* OpenApplication(const AppLaunchParams& params);
 
-  // Attempt to open |app_id| in a new window.
-  bool OpenApplicationWindow(const std::string& app_id,
-                             const base::CommandLine& command_line,
-                             const base::FilePath& current_directory);
-
-  // Attempt to open |app_id| in a new tab.
-  bool OpenApplicationTab(const std::string& app_id);
+  // Attempt to open |app_id| in a new window or tab. Open an empty browser
+  // window if unsuccessful. The user's preferred launch container for the app
+  // (standalone window or browser tab) is used. |callback| will be called with
+  // the container type used to open the app, kLaunchContainerNone if an empty
+  // browser window was opened.
+  void LaunchApplication(
+      const std::string& app_id,
+      const base::CommandLine& command_line,
+      const base::FilePath& current_directory,
+      base::OnceCallback<void(Browser* browser,
+                              apps::mojom::LaunchContainer container)>
+          callback);
 
  private:
   LaunchManager& GetLaunchManagerForApp(const std::string& app_id);

chrome/browser/apps/platform_apps/platform_app_launch.cc

@@ -132,7 +132,8 @@ bool OpenExtensionApplicationWithReenablePrompt(
   return true;
 }
 
-bool OpenExtensionAppShortcutWindow(Profile* profile, const GURL& url) {
+content::WebContents* OpenExtensionAppShortcutWindow(Profile* profile,
+                                                     const GURL& url) {
   const extensions::Extension* app = extensions::ExtensionRegistry::Get(profile)
                                          ->enabled_extensions()
                                          .GetAppByURL(url);
@@ -144,8 +145,7 @@ bool OpenExtensionAppShortcutWindow(Profile* profile, const GURL& url) {
         extensions::Manifest::TYPE_HOSTED_APP);
   }
 
-  content::WebContents* app_tab = ::OpenAppShortcutWindow(profile, url);
-  return app_tab != nullptr;
+  return ::OpenAppShortcutWindow(profile, url);
 }
 
 void RecordExtensionAppLaunchOnTabRestored(Profile* profile, const GURL& url) {

chrome/browser/apps/platform_apps/platform_app_launch.h

@@ -15,6 +15,10 @@ class CommandLine;
 class FilePath;
 }  // namespace base
 
+namespace content {
+class WebContents;
+}  // namespace content
+
 namespace apps {
 
 // Tries to open an application window. If the app is specified to start in a
@@ -40,9 +44,10 @@ bool OpenExtensionApplicationWithReenablePrompt(
     const base::FilePath& current_directory);
 
 // Tries to open an application window by app's |url|.
-// Returns true if |url| was successfully opened in a window, and false
-// otherwise.
-bool OpenExtensionAppShortcutWindow(Profile* profile, const GURL& url);
+// Returns web contents if |url| was successfully opened in a window, and
+// nullptr otherwise.
+content::WebContents* OpenExtensionAppShortcutWindow(Profile* profile,
+                                                     const GURL& url);
 
 // Records the restored app launch for UMA.
 void RecordExtensionAppLaunchOnTabRestored(Profile* profile, const GURL& url);

chrome/browser/ui/browser_browsertest.cc

@@ -23,6 +23,7 @@
 #include "base/strings/string_split.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/system/sys_info.h"
+#include "base/test/metrics/histogram_tester.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "chrome/app/chrome_command_ids.h"
@@ -1275,6 +1276,7 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, TabClosingWhenRemovingExtension) {
 
 // Open with --app-id=<id>, and see that an application tab opens by default.
 IN_PROC_BROWSER_TEST_F(BrowserTest, AppIdSwitch) {
+  base::HistogramTester tester;
   ASSERT_TRUE(embedded_test_server()->Start());
 
   // There should be one tab to start with.
@@ -1293,11 +1295,19 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, AppIdSwitch) {
   StartupBrowserCreatorImpl launch(base::FilePath(), command_line, first_run);
 
   // The app should open as a tab.
-  EXPECT_FALSE(launch.OpenApplicationWindow(browser()->profile()));
-  EXPECT_TRUE(launch.OpenApplicationTab(browser()->profile()));
+  EXPECT_TRUE(launch.Launch(browser()->profile(), std::vector<GURL>(),
+                            /*process_startup=*/false));
 
-  // Check that a the number of browsers and tabs is correct.
-  unsigned int expected_browsers = 1;
+  {
+    // From startup_browser_creator_impl.cc:
+    constexpr char kLaunchModesHistogram[] = "Launch.Modes";
+    const base::HistogramBase::Sample LM_AS_WEBAPP_IN_TAB = 21;
+
+    tester.ExpectUniqueSample(kLaunchModesHistogram, LM_AS_WEBAPP_IN_TAB, 1);
+  }
+
+  // Check that the number of browsers and tabs is correct.
+  const unsigned int expected_browsers = 1;
   int expected_tabs = 1;
   expected_tabs++;
 

chrome/browser/ui/startup/startup_browser_creator_impl.cc

@@ -40,6 +40,7 @@
 #include "chrome/browser/profiles/profile_io_data.h"
 #include "chrome/browser/sessions/session_service.h"
 #include "chrome/browser/sessions/session_service_factory.h"
+#include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_list.h"
@@ -61,6 +62,7 @@
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "components/prefs/pref_service.h"
+#include "components/services/app_service/public/mojom/types.mojom.h"
 #include "content/public/browser/child_process_security_policy.h"
 #include "content/public/browser/dom_storage_context.h"
 #include "content/public/browser/storage_partition.h"
@@ -130,6 +132,7 @@ enum LaunchMode {
                                        // Credential Provider for Windows.
   LM_AS_WEBAPP_IN_TAB = 21,            // Launched as an installed web
                                        // application in a browser tab.
+  LM_UNKNOWN_WEBAPP = 22,  // The requested web application was not installed.
 };
 
 // Returns a LaunchMode value if one can be determined with low overhead, or
@@ -242,7 +245,7 @@ LaunchMode GetLaunchModeSlow() {
 // Log in a histogram the frequency of launching by the different methods. See
 // LaunchMode enum for the actual values of the buckets.
 void RecordLaunchModeHistogram(LaunchMode mode) {
-  static constexpr char kHistogramName[] = "Launch.Modes";
+  static constexpr char kLaunchModesHistogram[] = "Launch.Modes";
   if (mode == LM_TO_BE_DECIDED &&
       (mode = GetLaunchModeFast()) == LM_TO_BE_DECIDED) {
     // The mode couldn't be determined with a fast path. Perform a more
@@ -251,19 +254,53 @@ void RecordLaunchModeHistogram(LaunchMode mode) {
                    {base::ThreadPool(), base::TaskPriority::BEST_EFFORT,
                     base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
                    base::BindOnce([]() {
-                     base::UmaHistogramSparse(kHistogramName,
+                     base::UmaHistogramSparse(kLaunchModesHistogram,
                                               GetLaunchModeSlow());
                    }));
   } else {
-    base::UmaHistogramSparse(kHistogramName, mode);
+    base::UmaHistogramSparse(kLaunchModesHistogram, mode);
   }
 }
 
+void MaybeToggleFullscreen(Browser* browser) {
+  // In kiosk mode, we want to always be fullscreen.
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kKioskMode) ||
+      base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kStartFullscreen)) {
+    chrome::ToggleFullscreenMode(browser);
+  }
+}
+
+void FinalizeWebAppLaunch(Browser* browser,
+                          apps::mojom::LaunchContainer container) {
+  LaunchMode mode;
+  switch (container) {
+    case apps::mojom::LaunchContainer::kLaunchContainerWindow:
+      DCHECK(browser->is_type_app());
+      mode = LM_AS_WEBAPP_IN_WINDOW;
+      break;
+    case apps::mojom::LaunchContainer::kLaunchContainerTab:
+      DCHECK(!browser->is_type_app());
+      mode = LM_AS_WEBAPP_IN_TAB;
+      break;
+    case apps::mojom::LaunchContainer::kLaunchContainerPanelDeprecated:
+      NOTREACHED();
+      FALLTHROUGH;
+    case apps::mojom::LaunchContainer::kLaunchContainerNone:
+      DCHECK(!browser->is_type_app());
+      mode = LM_UNKNOWN_WEBAPP;
+      break;
+  }
+
+  RecordLaunchModeHistogram(mode);
+  MaybeToggleFullscreen(browser);
+}
+
 void UrlsToTabs(const std::vector<GURL>& urls, StartupTabs* tabs) {
-  for (size_t i = 0; i < urls.size(); ++i) {
+  for (const GURL& url : urls) {
     StartupTab tab;
     tab.is_pinned = false;
-    tab.url = urls[i];
+    tab.url = url;
     tabs->push_back(tab);
   }
 }
@@ -380,11 +417,7 @@ bool StartupBrowserCreatorImpl::Launch(Profile* profile,
   // should either open in an existing Chrome window for this profile, or spawn
   // a new Chrome window without any NTP if no window exists (see
   // crbug.com/528385).
-  if (OpenApplicationWindow(profile)) {
-    RecordLaunchModeHistogram(LM_AS_WEBAPP_IN_WINDOW);
-  } else if (OpenApplicationTab(profile)) {
-    RecordLaunchModeHistogram(LM_AS_WEBAPP_IN_TAB);
-  } else {
+  if (!MaybeLaunchApplication(profile)) {
     // Check the true process command line for --try-chrome-again=N rather than
     // the one parsed for startup URLs and such.
     if (!base::CommandLine::ForCurrentProcess()
@@ -410,18 +443,13 @@ bool StartupBrowserCreatorImpl::Launch(Profile* profile,
       KeystoneInfoBar::PromotionInfoBar(profile);
     }
 #endif
-  }
 
-  // In kiosk mode, we want to always be fullscreen, so switch to that now.
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kKioskMode) ||
-      base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kStartFullscreen)) {
     // It's possible for there to be no browser window, e.g. if someone
     // specified a non-sensical combination of options
     // ("--kiosk --no_startup_window"); do nothing in that case.
     Browser* browser = BrowserList::GetInstance()->GetLastActive();
     if (browser)
-      chrome::ToggleFullscreenMode(browser);
+      MaybeToggleFullscreen(browser);
   }
 
 #if defined(OS_WIN)
@@ -529,18 +557,16 @@ bool StartupBrowserCreatorImpl::IsAppLaunch(std::string* app_url,
   return false;
 }
 
-bool StartupBrowserCreatorImpl::OpenApplicationWindow(Profile* profile) {
+bool StartupBrowserCreatorImpl::MaybeLaunchApplication(Profile* profile) {
   std::string url_string, app_id;
   if (!IsAppLaunch(&url_string, &app_id))
     return false;
 
-  // This can fail if the app_id is invalid.  It can also fail if the
-  // extension is external, and has not yet been installed.
-  // TODO(skerner): Do something reasonable here. Pop up a warning panel?
-  // Open an URL to the gallery page of the extension id?
   if (!app_id.empty()) {
-    return apps::LaunchService::Get(profile)->OpenApplicationWindow(
-        app_id, command_line_, cur_dir_);
+    // Opens an empty browser window if the app_id is invalid.
+    apps::LaunchService::Get(profile)->LaunchApplication(
+        app_id, command_line_, cur_dir_, base::BindOnce(&FinalizeWebAppLaunch));
+    return true;
   }
 
   if (url_string.empty())
@@ -557,24 +583,19 @@ bool StartupBrowserCreatorImpl::OpenApplicationWindow(Profile* profile) {
         content::ChildProcessSecurityPolicy::GetInstance();
     if (policy->IsWebSafeScheme(url.scheme()) ||
         url.SchemeIs(url::kFileScheme)) {
-      return apps::OpenAppShortcutWindow(profile, url);
+      const content::WebContents* web_contents =
+          apps::OpenAppShortcutWindow(profile, url);
+      if (web_contents) {
+        FinalizeWebAppLaunch(
+            chrome::FindBrowserWithWebContents(web_contents),
+            apps::mojom::LaunchContainer::kLaunchContainerWindow);
+        return true;
+      }
     }
   }
   return false;
 }
 
-bool StartupBrowserCreatorImpl::OpenApplicationTab(Profile* profile) {
-  std::string app_id;
-  // App shortcuts to URLs always open in an app window.  Because this
-  // function will open an app that should be in a tab, there is no need
-  // to look at the app URL.  OpenApplicationWindow() will open app url
-  // shortcuts.
-  if (!IsAppLaunch(nullptr, &app_id) || app_id.empty())
-    return false;
-
-  return apps::LaunchService::Get(profile)->OpenApplicationTab(app_id);
-}
-
 void StartupBrowserCreatorImpl::DetermineURLsAndLaunch(
     bool process_startup,
     const std::vector<GURL>& cmd_line_urls) {

chrome/browser/ui/startup/startup_browser_creator_impl.h

@@ -127,14 +127,12 @@ class StartupBrowserCreatorImpl {
   // In this case |app_url| or |app_id| are populated if they're non-null.
   bool IsAppLaunch(std::string* app_url, std::string* app_id);
 
-  // If IsAppLaunch is true, tries to open an application window.
-  // If the app is specified to start in a tab, or IsAppLaunch is false,
-  // returns false to specify default processing.
-  bool OpenApplicationWindow(Profile* profile);
-
-  // If IsAppLaunch is true and the user set a pref indicating that the app
-  // should open in a tab, do so.
-  bool OpenApplicationTab(Profile* profile);
+  // Opens an application window or tab if the process was launched with the web
+  // application command line switches. Returns true if launch succeeded (or is
+  // proceeding asynchronously); otherwise, returns false to indicate that
+  // normal browser startup should resume. Desktop web applications launch
+  // asynchronously, and fall back to launching a browser window.
+  bool MaybeLaunchApplication(Profile* profile);
 
   // Determines the URLs to be shown at startup by way of various policies
   // (welcome, pinned tabs, etc.), determines whether a session restore

chrome/browser/ui/web_applications/web_app_engagement_browsertest.cc

@@ -5,12 +5,21 @@
 #include "chrome/browser/ui/web_applications/web_app_metrics.h"
 
 #include <bitset>
+#include <vector>
 
+#include "base/command_line.h"
+#include "base/files/file_path.h"
 #include "base/metrics/statistics_recorder.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "chrome/browser/engagement/site_engagement_service.h"
+#include "chrome/browser/first_run/first_run.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/ui/startup/startup_browser_creator_impl.h"
+#include "chrome/browser/ui/startup/startup_types.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/web_applications/test/web_app_browsertest_util.h"
 #include "chrome/browser/ui/web_applications/web_app_controller_browsertest.h"
 #include "chrome/browser/ui/web_applications/web_app_metrics.h"
@@ -19,11 +28,16 @@
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "chrome/browser/web_applications/web_app_provider.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/web_application_info.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_types.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/test_navigation_observer.h"
+#include "content/public/test/test_utils.h"
+#include "ui/views/widget/widget.h"
 #include "url/gurl.h"
 
 namespace {
@@ -489,6 +503,113 @@ IN_PROC_BROWSER_TEST_P(WebAppEngagementBrowserTest, RecordedForNonApps) {
   TestEngagementEventsAfterLaunch(histograms, browser());
 }
 
+IN_PROC_BROWSER_TEST_P(WebAppEngagementBrowserTest, CommandLineWindow) {
+  base::HistogramTester tester;
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  // There should be one browser to start with.
+  unsigned int expected_browsers = 1;
+  const int expected_tabs = 1;
+  EXPECT_EQ(expected_browsers, chrome::GetBrowserCount(browser()->profile()));
+  EXPECT_EQ(expected_tabs, browser()->tab_strip_model()->count());
+
+  const GURL example_url(
+      embedded_test_server()->GetURL("/banners/manifest_test_page.html"));
+
+  auto result_code = PendingAppManagerInstall(
+      browser()->profile(), CreateInstallOptions(example_url));
+  ASSERT_EQ(InstallResultCode::kSuccessNewInstall, result_code);
+  base::Optional<AppId> app_id = FindAppWithUrlInScope(example_url);
+  ASSERT_TRUE(app_id);
+  content::WindowedNotificationObserver app_loaded_observer(
+      content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
+      content::NotificationService::AllSources());
+
+  base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
+  command_line.AppendSwitchASCII(switches::kAppId, *app_id);
+
+  chrome::startup::IsFirstRun first_run =
+      first_run::IsChromeFirstRun() ? chrome::startup::IS_FIRST_RUN
+                                    : chrome::startup::IS_NOT_FIRST_RUN;
+  StartupBrowserCreatorImpl launch(base::FilePath(), command_line, first_run);
+
+  // The app should open as a window.
+  EXPECT_TRUE(launch.Launch(browser()->profile(), std::vector<GURL>(),
+                            /*process_startup=*/false));
+  app_loaded_observer.Wait();
+
+  Browser* const app_browser = BrowserList::GetInstance()->GetLastActive();
+  EXPECT_EQ(app_browser->type(), Browser::TYPE_APP);
+  EXPECT_TRUE(app_browser->app_controller());
+
+  {
+    // From startup_browser_creator_impl.cc:
+    constexpr char kLaunchModesHistogram[] = "Launch.Modes";
+    const base::HistogramBase::Sample LM_AS_WEBAPP_IN_WINDOW = 1;
+
+    tester.ExpectUniqueSample(kLaunchModesHistogram, LM_AS_WEBAPP_IN_WINDOW, 1);
+  }
+
+  // Check that the number of browsers and tabs is correct.
+  expected_browsers++;
+
+  EXPECT_EQ(expected_browsers, chrome::GetBrowserCount(browser()->profile()));
+  EXPECT_EQ(expected_tabs, browser()->tab_strip_model()->count());
+  EXPECT_EQ(expected_tabs, app_browser->tab_strip_model()->count());
+}
+
+IN_PROC_BROWSER_TEST_P(WebAppEngagementBrowserTest, CommandLineTab) {
+  base::HistogramTester tester;
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  // There should be one browser to start with.
+  const unsigned int expected_browsers = 1;
+  int expected_tabs = 1;
+  EXPECT_EQ(expected_browsers, chrome::GetBrowserCount(browser()->profile()));
+  EXPECT_EQ(expected_tabs, browser()->tab_strip_model()->count());
+
+  const GURL example_url(
+      embedded_test_server()->GetURL("/banners/manifest_test_page.html"));
+
+  ExternalInstallOptions install_options = CreateInstallOptions(example_url);
+  install_options.user_display_mode = DisplayMode::kBrowser;
+  auto result_code =
+      PendingAppManagerInstall(browser()->profile(), install_options);
+  ASSERT_EQ(InstallResultCode::kSuccessNewInstall, result_code);
+  base::Optional<AppId> app_id = FindAppWithUrlInScope(example_url);
+  ASSERT_TRUE(app_id);
+  content::WindowedNotificationObserver app_loaded_observer(
+      content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
+      content::NotificationService::AllSources());
+
+  base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
+  command_line.AppendSwitchASCII(switches::kAppId, *app_id);
+
+  chrome::startup::IsFirstRun first_run =
+      first_run::IsChromeFirstRun() ? chrome::startup::IS_FIRST_RUN
+                                    : chrome::startup::IS_NOT_FIRST_RUN;
+  StartupBrowserCreatorImpl launch(base::FilePath(), command_line, first_run);
+
+  // The app should open as a tab.
+  EXPECT_TRUE(launch.Launch(browser()->profile(), std::vector<GURL>(),
+                            /*process_startup=*/false));
+  app_loaded_observer.Wait();
+
+  {
+    // From startup_browser_creator_impl.cc:
+    constexpr char kLaunchModesHistogram[] = "Launch.Modes";
+    const base::HistogramBase::Sample LM_AS_WEBAPP_IN_TAB = 21;
+
+    tester.ExpectUniqueSample(kLaunchModesHistogram, LM_AS_WEBAPP_IN_TAB, 1);
+  }
+
+  // Check that the number of browsers and tabs is correct.
+  expected_tabs++;
+
+  EXPECT_EQ(expected_browsers, chrome::GetBrowserCount(browser()->profile()));
+  EXPECT_EQ(expected_tabs, browser()->tab_strip_model()->count());
+}
+
 INSTANTIATE_TEST_SUITE_P(
     All,
     WebAppEngagementBrowserTest,

chrome/browser/ui/web_applications/web_app_launch_manager.cc

@@ -10,7 +10,11 @@
 #include "chrome/browser/banners/app_banner_settings_helper.h"
 #include "chrome/browser/engagement/site_engagement_service.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/browser_navigator.h"
+#include "chrome/browser/ui/browser_navigator_params.h"
 #include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/web_applications/system_web_app_ui_utils.h"
 #include "chrome/browser/web_applications/components/file_handler_manager.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
@@ -22,10 +26,16 @@
 #include "chrome/browser/web_applications/web_app_registrar.h"
 #include "chrome/browser/web_applications/web_app_tab_helper.h"
 #include "chrome/browser/web_launch/web_launch_files_helper.h"
+#include "content/public/browser/page_navigator.h"
 #include "content/public/browser/render_view_host.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/common/referrer.h"
 #include "extensions/common/constants.h"
 #include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/mojom/renderer_preferences.mojom.h"
+#include "ui/base/page_transition_types.h"
+#include "ui/base/window_open_disposition.h"
+#include "url/gurl.h"
 
 namespace web_app {
 
@@ -38,6 +48,16 @@ ui::WindowShowState DetermineWindowShowState() {
   return ui::SHOW_STATE_DEFAULT;
 }
 
+void SetTabHelperAppId(content::WebContents* web_contents,
+                       const std::string& app_id) {
+  // TODO(https://crbug.com/1032443):
+  // Eventually move this to browser_navigator.cc: CreateTargetContents().
+  WebAppTabHelperBase* tab_helper =
+      WebAppTabHelperBase::FromWebContents(web_contents);
+  DCHECK(tab_helper);
+  tab_helper->SetAppId(app_id);
+}
+
 }  // namespace
 
 Browser* CreateWebApplicationWindow(Profile* profile,
@@ -63,13 +83,7 @@ content::WebContents* NavigateWebApplicationWindow(
   content::WebContents* web_contents =
       nav_params.navigated_or_inserted_contents;
 
-  // TODO(https://crbug.com/1032443):
-  // Eventually move this to browser_navigator.cc: CreateTargetContents().
-  WebAppTabHelperBase* tab_helper =
-      WebAppTabHelperBase::FromWebContents(web_contents);
-  DCHECK(tab_helper);
-  tab_helper->SetAppId(app_id);
-
+  SetTabHelperAppId(web_contents, app_id);
   return web_contents;
 }
 
@@ -105,10 +119,47 @@ content::WebContents* WebAppLaunchManager::OpenApplication(
     return browser->tab_strip_model()->GetActiveWebContents();
   }
 
-  Browser* browser = CreateWebApplicationWindow(profile(), params.app_id);
+  Browser* browser;
+  WindowOpenDisposition disposition = WindowOpenDisposition::NEW_FOREGROUND_TAB;
+  if (params.container == apps::mojom::LaunchContainer::kLaunchContainerTab) {
+    browser = chrome::FindTabbedBrowser(
+        profile(), /*match_original_profiles=*/false, params.display_id);
+    if (browser) {
+      // For existing browser, ensure its window is activated.
+      browser->window()->Activate();
+      disposition = params.disposition;
+    } else {
+      browser =
+          new Browser(Browser::CreateParams(Browser::TYPE_NORMAL, profile(),
+                                            /*user_gesture=*/true));
+    }
+  } else {
+    browser = CreateWebApplicationWindow(profile(), params.app_id);
+  }
 
-  content::WebContents* web_contents = NavigateWebApplicationWindow(
-      browser, params.app_id, url, WindowOpenDisposition::NEW_FOREGROUND_TAB);
+  content::WebContents* web_contents;
+  if (disposition == WindowOpenDisposition::CURRENT_TAB) {
+    TabStripModel* const model = browser->tab_strip_model();
+    content::WebContents* existing_tab = model->GetActiveWebContents();
+    const int tab_index = model->GetIndexOfWebContents(existing_tab);
+
+    existing_tab->OpenURL(content::OpenURLParams(
+        url,
+        content::Referrer::SanitizeForRequest(
+            url, content::Referrer(existing_tab->GetURL(),
+                                   network::mojom::ReferrerPolicy::kDefault)),
+        disposition, ui::PAGE_TRANSITION_AUTO_BOOKMARK,
+        /*is_renderer_initiated=*/false));
+
+    // Reset existing_tab as OpenURL() may have clobbered it.
+    existing_tab = browser->tab_strip_model()->GetActiveWebContents();
+    model->ActivateTabAt(tab_index, {TabStripModel::GestureType::kOther});
+    web_contents = existing_tab;
+    SetTabHelperAppId(web_contents, params.app_id);
+  } else {
+    web_contents = NavigateWebApplicationWindow(
+        browser, params.app_id, url, WindowOpenDisposition::NEW_FOREGROUND_TAB);
+  }
 
   if (file_handler_manager.IsFileHandlingAPIAvailable(params.app_id)) {
     web_launch::WebLaunchFilesHelper::SetLaunchPaths(web_contents, url,
@@ -143,12 +194,14 @@ content::WebContents* WebAppLaunchManager::OpenApplication(
   return web_contents;
 }
 
-bool WebAppLaunchManager::OpenApplicationWindow(
+void WebAppLaunchManager::LaunchApplication(
     const std::string& app_id,
     const base::CommandLine& command_line,
-    const base::FilePath& current_directory) {
+    const base::FilePath& current_directory,
+    base::OnceCallback<void(Browser* browser,
+                            apps::mojom::LaunchContainer container)> callback) {
   if (!provider_)
-    return false;
+    return;
 
   apps::AppLaunchParams params(
       app_id, apps::mojom::LaunchContainer::kLaunchContainerWindow,
@@ -159,34 +212,36 @@ bool WebAppLaunchManager::OpenApplicationWindow(
   params.launch_files =
       apps::LaunchManager::GetLaunchFilesFromCommandLine(command_line);
 
-  provider_->on_registry_ready().Post(
-      FROM_HERE, base::BindOnce(&WebAppLaunchManager::OpenWebApplication,
-                                weak_ptr_factory_.GetWeakPtr(), params));
-
-  return true;
-}
-
-bool WebAppLaunchManager::OpenApplicationTab(const std::string& app_id) {
-  if (!provider_)
-    return false;
-
-  apps::AppLaunchParams params(
-      app_id, apps::mojom::LaunchContainer::kLaunchContainerTab,
-      WindowOpenDisposition::NEW_FOREGROUND_TAB,
-      apps::mojom::AppLaunchSource::kSourceCommandLine);
-
   // Wait for the web applications database to load.
   // If the profile and WebAppLaunchManager are destroyed,
   // on_registry_ready will not fire.
   provider_->on_registry_ready().Post(
-      FROM_HERE, base::BindOnce(&WebAppLaunchManager::OpenWebApplication,
-                                weak_ptr_factory_.GetWeakPtr(), params));
-  return true;
+      FROM_HERE, base::BindOnce(&WebAppLaunchManager::LaunchWebApplication,
+                                weak_ptr_factory_.GetWeakPtr(), params,
+                                std::move(callback)));
 }
 
-void WebAppLaunchManager::OpenWebApplication(
-    const apps::AppLaunchParams& params) {
-  OpenApplication(params);
+void WebAppLaunchManager::LaunchWebApplication(
+    apps::AppLaunchParams params,
+    base::OnceCallback<void(Browser* browser,
+                            apps::mojom::LaunchContainer container)> callback) {
+  Browser* browser;
+  if (provider_->registrar().IsInstalled(params.app_id)) {
+    if (provider_->registrar().GetAppEffectiveDisplayMode(params.app_id) ==
+        blink::mojom::DisplayMode::kBrowser) {
+      params.container = apps::mojom::LaunchContainer::kLaunchContainerTab;
+      params.disposition = WindowOpenDisposition::NEW_FOREGROUND_TAB;
+    }
+
+    const content::WebContents* web_contents = OpenApplication(params);
+    browser = chrome::FindBrowserWithWebContents(web_contents);
+    DCHECK(browser);
+  } else {
+    // Open an empty browser window as the app_id is invalid.
+    browser = CreateNewTabBrowser();
+    params.container = apps::mojom::LaunchContainer::kLaunchContainerNone;
+  }
+  std::move(callback).Run(browser, params.container);
 }
 
 void RecordAppWindowLaunch(Profile* profile, const std::string& app_id) {

chrome/browser/ui/web_applications/web_app_launch_manager.h

@@ -36,14 +36,20 @@ class WebAppLaunchManager : public apps::LaunchManager {
   content::WebContents* OpenApplication(
       const apps::AppLaunchParams& params) override;
 
-  bool OpenApplicationWindow(const std::string& app_id,
-                             const base::CommandLine& command_line,
-                             const base::FilePath& current_directory) override;
-
-  bool OpenApplicationTab(const std::string& app_id) override;
+  void LaunchApplication(
+      const std::string& app_id,
+      const base::CommandLine& command_line,
+      const base::FilePath& current_directory,
+      base::OnceCallback<void(Browser* browser,
+                              apps::mojom::LaunchContainer container)> callback)
+      override;
 
  private:
-  void OpenWebApplication(const apps::AppLaunchParams& params);
+  void LaunchWebApplication(
+      apps::AppLaunchParams params,
+      base::OnceCallback<void(Browser* browser,
+                              apps::mojom::LaunchContainer container)>
+          callback);
 
   WebAppProvider* const provider_;
 

tools/metrics/histograms/enums.xml

@@ -35948,6 +35948,9 @@ Called by update_gpu_driver_bug_workaround_entries.py.-->
   <int value="21" label="LM_AS_WEBAPP_TAB">
     Launched as an installed web application in a browser tab
   </int>
+  <int value="22" label="LM_UNKNOWN_WEBAPP">
+    The requested web application was not installed
+  </int>
 </enum>
 
 <enum name="LaunchType">