Fix session restore after launching with PWA.

The underlying issue is that the kProfilesLastActive pref gets
cleared when Chrome is launched with a specific profile, which
prevents Chrome from iterating over the last active profiles
and calling session restore on each of them. This CL stops
clearing the pref when launched with --app-id, and avoids
restoring sessions for the last active profiles
if launched with --app-id. Also, ignore browser opens and
closes if the browser window is for a web app, in order to prevent
kProfilesLastActive pref from getting overwritten.

Bug: 1022795
Change-Id: I350a54eaaf167dabb297b9bda9061fe965f0750e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1984710Reviewed-by: Greg Thompson <grt@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: David Bienvenu <davidbienvenu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827876}

chrome/browser/chrome_browser_main.cc

@@ -343,10 +343,9 @@ Profile* CreatePrimaryProfile(const content::MainFunctionParams& parameters,
                               const base::FilePath& user_data_dir,
                               const base::CommandLine& parsed_command_line) {
   TRACE_EVENT0("startup", "ChromeBrowserMainParts::CreateProfile")
-
   base::Time start = base::Time::Now();
 
-  bool set_last_used_profile = false;
+  bool last_used_profile_set = false;
 
 // If the browser is launched due to activation on Windows native
 // notification, the profile id encoded in the notification launch id should
@@ -356,22 +355,24 @@ Profile* CreatePrimaryProfile(const content::MainFunctionParams& parameters,
       NotificationLaunchId::GetNotificationLaunchProfileId(parsed_command_line);
   if (!last_used_profile_id.empty()) {
     profiles::SetLastUsedProfile(last_used_profile_id);
-    set_last_used_profile = true;
+    last_used_profile_set = true;
   }
 #endif  // defined(OS_WIN)
 
   bool profile_dir_specified =
       profiles::IsMultipleProfilesEnabled() &&
       parsed_command_line.HasSwitch(switches::kProfileDirectory);
-  if (!set_last_used_profile && profile_dir_specified) {
+  if (!last_used_profile_set && profile_dir_specified) {
     profiles::SetLastUsedProfile(
         parsed_command_line.GetSwitchValueASCII(switches::kProfileDirectory));
-    set_last_used_profile = true;
+    last_used_profile_set = true;
   }
 
-  if (set_last_used_profile) {
+  if (last_used_profile_set &&
+      !parsed_command_line.HasSwitch(switches::kAppId)) {
     // Clear kProfilesLastActive since the user only wants to launch a specific
-    // profile.
+    // profile. Don't clear it if the user launched a web app, in order to not
+    // break any subsequent multi-profile session restore.
     ListPrefUpdate update(g_browser_process->local_state(),
                           prefs::kProfilesLastActive);
     base::ListValue* profile_list = update.Get();
@@ -395,7 +396,7 @@ Profile* CreatePrimaryProfile(const content::MainFunctionParams& parameters,
 #else
   profile = GetStartupProfile(user_data_dir, parsed_command_line);
 
-  if (!profile && !set_last_used_profile)
+  if (!profile && !last_used_profile_set)
     profile = GetFallbackStartupProfile();
 
   if (!profile) {
@@ -1607,9 +1608,12 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() {
 #if !defined(OS_CHROMEOS)
   // On ChromeOS multiple profiles doesn't apply, and will break if we load
   // them this early as the cryptohome hasn't yet been mounted (which happens
-  // only once we log in).
-  last_opened_profiles =
-      g_browser_process->profile_manager()->GetLastOpenedProfiles();
+  // only once we log in). And if we're launching a web app, we don't want to
+  // restore the last opened profiles.
+  if (!parsed_command_line().HasSwitch(switches::kAppId)) {
+    last_opened_profiles =
+        g_browser_process->profile_manager()->GetLastOpenedProfiles();
+  }
 #endif  // defined(OS_CHROMEOS)
 
   // This step is costly and is already measured in

chrome/browser/chrome_browser_main_win.cc

@@ -681,7 +681,8 @@ void ChromeBrowserMainPartsWin::PostBrowserStart() {
   // complete run of the Chrome Cleanup tool. If post-cleanup settings reset is
   // enabled, we delay checks for settings reset prompt until the scheduled
   // reset is finished.
-  if (safe_browsing::PostCleanupSettingsResetter::IsEnabled()) {
+  if (safe_browsing::PostCleanupSettingsResetter::IsEnabled() &&
+      !parsed_command_line().HasSwitch(switches::kAppId)) {
     // Using last opened profiles, because we want to find reset the profile
     // that was open in the last Chrome run, which may not be open yet in
     // the current run.

chrome/browser/profiles/profile_manager.cc

@@ -1947,7 +1947,7 @@ void ProfileManager::OnBrowserOpened(Browser* browser) {
   DCHECK(profile);
   bool is_ephemeral =
       profile->GetPrefs()->GetBoolean(prefs::kForceEphemeralProfiles);
-  if (!profile->IsOffTheRecord() && !is_ephemeral &&
+  if (!profile->IsOffTheRecord() && !is_ephemeral && !browser->is_type_app() &&
       ++browser_counts_[profile] == 1) {
     active_profiles_.push_back(profile);
     SaveActiveProfiles();
@@ -1962,7 +1962,8 @@ void ProfileManager::OnBrowserOpened(Browser* browser) {
 void ProfileManager::OnBrowserClosed(Browser* browser) {
   Profile* profile = browser->profile();
   DCHECK(profile);
-  if (!profile->IsOffTheRecord() && --browser_counts_[profile] == 0) {
+  if (!profile->IsOffTheRecord() && !browser->is_type_app() &&
+      --browser_counts_[profile] == 0) {
     active_profiles_.erase(
         std::find(active_profiles_.begin(), active_profiles_.end(), profile));
     if (!closing_all_browsers_)

chrome/browser/ui/startup/startup_browser_creator.h

@@ -133,6 +133,8 @@ class StartupBrowserCreator {
                            CommandLineTab);
   FRIEND_TEST_ALL_PREFIXES(web_app::WebAppEngagementBrowserTest,
                            CommandLineWindow);
+  FRIEND_TEST_ALL_PREFIXES(StartupBrowserCreatorTest,
+                           LastUsedProfilesWithWebApp);
 
   bool ProcessCmdLineImpl(const base::CommandLine& command_line,
                           const base::FilePath& cur_dir,

chrome/browser/ui/startup/startup_browser_creator_browsertest.cc

@@ -8,6 +8,7 @@
 #include <memory>
 #include <string>
 
+#include "base/base_switches.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
@@ -15,6 +16,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/path_service.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/bind.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/threading/thread_restrictions.h"
 #include "build/branding_buildflags.h"
@@ -27,7 +29,9 @@
 #include "chrome/browser/extensions/launch_util.h"
 #include "chrome/browser/first_run/first_run.h"
 #include "chrome/browser/infobars/infobar_service.h"
+#include "chrome/browser/installable/installable_metrics.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
+#include "chrome/browser/prefs/chrome_pref_service_factory.h"
 #include "chrome/browser/prefs/session_startup_pref.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_attributes_entry.h"
@@ -50,8 +54,13 @@
 #include "chrome/browser/ui/startup/startup_tab_provider.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/ui_features.h"
+#include "chrome/browser/web_applications/components/install_finalizer.h"
+#include "chrome/browser/web_applications/components/install_manager.h"
+#include "chrome/browser/web_applications/components/web_application_info.h"
+#include "chrome/browser/web_applications/web_app_provider.h"
 #include "chrome/common/buildflags.h"
 #include "chrome/common/chrome_constants.h"
+#include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/pref_names.h"
@@ -71,6 +80,7 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/browser_test.h"
+#include "content/public/test/test_launcher.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "content/public/test/test_utils.h"
 #include "extensions/browser/extension_registry.h"
@@ -113,6 +123,11 @@ using extensions::Extension;
 namespace {
 
 #if !defined(OS_CHROMEOS)
+
+const char kAppId[] = "dofnemchnjfeendjmdhaldenaiabpiad";
+const char kAppName[] = "Test App";
+const char kStartUrl[] = "https://test.com";
+
 // Check that there are two browsers. Find the one that is not |browser|.
 Browser* FindOneOtherBrowser(Browser* browser) {
   // There should only be one other browser.
@@ -1174,6 +1189,164 @@ IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorTest,
   ASSERT_EQ(1u, chrome::GetBrowserCount(profile2));
 }
 
+class StartupBrowserWithWebAppTest : public StartupBrowserCreatorTest {
+ protected:
+  StartupBrowserWithWebAppTest() = default;
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    StartupBrowserCreatorTest::SetUpCommandLine(command_line);
+    if (content::IsPreTest()) {
+      // Load an app with launch.container = 'window'.
+      command_line->AppendSwitchASCII(switches::kAppId, kAppId);
+      command_line->AppendSwitchASCII(switches::kProfileDirectory, "Default");
+    }
+  }
+  web_app::WebAppProvider& provider() {
+    return *web_app::WebAppProvider::Get(profile());
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(StartupBrowserWithWebAppTest,
+                       PRE_PRE_LastUsedProfilesWithWebApp) {
+  // Simulate a browser restart by creating the profiles in the PRE_PRE part.
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  // Create two profiles.
+  base::FilePath dest_path = profile_manager->user_data_dir();
+
+  Profile* profile1 = nullptr;
+  Profile* profile2 = nullptr;
+  {
+    base::ScopedAllowBlockingForTesting allow_blocking;
+    profile1 = profile_manager->GetProfile(
+        dest_path.Append(FILE_PATH_LITERAL("New Profile 1")));
+    ASSERT_TRUE(profile1);
+
+    profile2 = profile_manager->GetProfile(
+        dest_path.Append(FILE_PATH_LITERAL("New Profile 2")));
+    ASSERT_TRUE(profile2);
+  }
+  DisableWelcomePages({profile1, profile2});
+
+  // Open some urls with the browsers, and close them.
+  Browser* browser1 = Browser::Create({Browser::TYPE_NORMAL, profile1, true});
+  chrome::NewTab(browser1);
+  ui_test_utils::NavigateToURL(browser1,
+                               embedded_test_server()->GetURL("/title1.html"));
+
+  Browser* browser2 = Browser::Create({Browser::TYPE_NORMAL, profile2, true});
+  chrome::NewTab(browser2);
+  ui_test_utils::NavigateToURL(browser2,
+                               embedded_test_server()->GetURL("/title2.html"));
+
+  // Set startup preferences for the 2 profiles to restore last session.
+  SessionStartupPref pref1(SessionStartupPref::LAST);
+  SessionStartupPref::SetStartupPref(profile1, pref1);
+  SessionStartupPref pref2(SessionStartupPref::LAST);
+  SessionStartupPref::SetStartupPref(profile2, pref2);
+
+  profile1->GetPrefs()->CommitPendingWrite();
+  profile2->GetPrefs()->CommitPendingWrite();
+
+  // Install a bookmark app that we will launch from the command line in
+  // subsequent tests.
+  web_app::InstallFinalizer& web_app_finalizer = provider().install_finalizer();
+  web_app::InstallFinalizer* bookmark_app_finalizer =
+      web_app_finalizer.legacy_finalizer_for_testing();
+  ASSERT_TRUE(bookmark_app_finalizer);
+
+  web_app::InstallFinalizer::FinalizeOptions options;
+  options.install_source = WebappInstallSource::OMNIBOX_INSTALL_ICON;
+
+  // Install bookmark app set to open as window.
+  {
+    base::RunLoop run_loop;
+    WebApplicationInfo info;
+    info.start_url = GURL(kStartUrl);
+    info.title = base::UTF8ToUTF16(kAppName);
+    info.open_as_window = true;
+    bookmark_app_finalizer->FinalizeInstall(
+        info, options,
+        base::BindLambdaForTesting(
+            [&](const web_app::AppId& app_id, web_app::InstallResultCode code) {
+              EXPECT_EQ(app_id, kAppId);
+              EXPECT_EQ(code, web_app::InstallResultCode::kSuccessNewInstall);
+              run_loop.Quit();
+            }));
+    run_loop.Run();
+
+    const extensions::Extension* extension =
+        extensions::ExtensionRegistry::Get(profile())
+            ->enabled_extensions()
+            .GetByID(kAppId);
+    ASSERT_TRUE(extension);
+    EXPECT_EQ(extensions::GetLaunchContainer(
+                  extensions::ExtensionPrefs::Get(profile()), extension),
+              extensions::LaunchContainer::kLaunchContainerWindow);
+  }
+}
+
+IN_PROC_BROWSER_TEST_F(StartupBrowserWithWebAppTest,
+                       PRE_LastUsedProfilesWithWebApp) {
+  ASSERT_EQ(1u, chrome::GetBrowserCount(browser()->profile()));
+
+  // An app window should have been launched.
+  EXPECT_TRUE(browser()->is_type_app());
+  CloseBrowserAsynchronously(browser());
+}
+
+IN_PROC_BROWSER_TEST_F(StartupBrowserWithWebAppTest,
+                       LastUsedProfilesWithWebApp) {
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+
+  base::FilePath dest_path = profile_manager->user_data_dir();
+
+  Profile* profile1 = nullptr;
+  Profile* profile2 = nullptr;
+  {
+    base::ScopedAllowBlockingForTesting allow_blocking;
+    profile1 = profile_manager->GetProfile(
+        dest_path.Append(FILE_PATH_LITERAL("New Profile 1")));
+    ASSERT_TRUE(profile1);
+
+    profile2 = profile_manager->GetProfile(
+        dest_path.Append(FILE_PATH_LITERAL("New Profile 2")));
+    ASSERT_TRUE(profile2);
+  }
+
+  // Simulate a browser launch with last opened profiles.
+  base::CommandLine dummy(base::CommandLine::NO_PROGRAM);
+  StartupBrowserCreator browser_creator;
+  std::vector<Profile*> last_opened_profiles =
+      profile_manager->GetLastOpenedProfiles();
+  browser_creator.Start(dummy, profile_manager->user_data_dir(), profile1,
+                        last_opened_profiles);
+
+  while (SessionRestore::IsRestoring(profile1) ||
+         SessionRestore::IsRestoring(profile2)) {
+    base::RunLoop().RunUntilIdle();
+  }
+
+  // The last open sessions should be restored.
+  EXPECT_TRUE(profile1->restored_last_session());
+  EXPECT_TRUE(profile2->restored_last_session());
+
+  Browser* new_browser = nullptr;
+  ASSERT_EQ(2u, chrome::GetBrowserCount(profile1));
+  new_browser = FindOneOtherBrowserForProfile(profile1, nullptr);
+  ASSERT_TRUE(new_browser);
+  TabStripModel* tab_strip = new_browser->tab_strip_model();
+  EXPECT_EQ("/title1.html", tab_strip->GetWebContentsAt(0)->GetURL().path());
+
+  ASSERT_EQ(2u, chrome::GetBrowserCount(profile2));
+  new_browser = FindOneOtherBrowserForProfile(profile2, nullptr);
+  ASSERT_TRUE(new_browser);
+  tab_strip = new_browser->tab_strip_model();
+  EXPECT_EQ("/title2.html", tab_strip->GetWebContentsAt(0)->GetURL().path());
+}
+
 #endif  // !defined(OS_CHROMEOS)
 
 class StartupBrowserCreatorExtensionsCheckupExperimentTest

chrome/common/pref_names.cc

@@ -1602,7 +1602,13 @@ const char kNotificationPermissionActions[] =
 // Directory of the last profile used.
 const char kProfileLastUsed[] = "profile.last_used";
 
-// List of directories of the profiles last active.
+// List of directories of the profiles last active in browser windows. It does
+// not include profiles active in app windows. When a browser window is opened,
+// if it's the only browser window open in the profile, its profile is added to
+// this list. When a browser window is closed, and there are no other browser
+// windows open in the profile, its profile is removed from this list. When
+// Chrome is launched with --session-restore, each of the profiles in this list
+// have their sessions restored.
 const char kProfilesLastActive[] = "profile.last_active_profiles";
 
 // Total number of profiles created for this Chrome build. Used to tag profile