Reland "Fix session restore after launching with PWA."

This is a reland of f7713274

Original change's description:
> 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/+/1984710
> Reviewed-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}

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

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) {
@@ -1612,9 +1613,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

@@ -1949,7 +1949,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();
@@ -1964,7 +1964,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,184 @@ IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorTest,
   ASSERT_EQ(1u, chrome::GetBrowserCount(profile2));
 }
 
+// An observer that returns back to test code after a new browser is added to
+// the BrowserList.
+class BrowserAddedObserver : public BrowserListObserver {
+ public:
+  BrowserAddedObserver() { BrowserList::AddObserver(this); }
+
+  ~BrowserAddedObserver() override { BrowserList::RemoveObserver(this); }
+
+  Browser* Wait() {
+    run_loop_.Run();
+    return browser_;
+  }
+
+ protected:
+  // BrowserListObserver:
+  void OnBrowserAdded(Browser* browser) override {
+    browser_ = browser;
+    run_loop_.Quit();
+  }
+
+ private:
+  Browser* browser_ = nullptr;
+  base::RunLoop run_loop_;
+};
+
+class StartupBrowserWithWebAppTest : public StartupBrowserCreatorTest {
+ protected:
+  StartupBrowserWithWebAppTest() = default;
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    StartupBrowserCreatorTest::SetUpCommandLine(command_line);
+    if (GetTestPreCount() == 1) {
+      // 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 web app that we will launch from the command line in
+  // the PRE test.
+  web_app::WebAppProviderBase* const provider =
+      web_app::WebAppProviderBase::GetProviderBase(browser()->profile());
+  web_app::InstallFinalizer& web_app_finalizer = provider->install_finalizer();
+
+  web_app::InstallFinalizer::FinalizeOptions options;
+  options.install_source = WebappInstallSource::OMNIBOX_INSTALL_ICON;
+
+  // Install web app set to open as a tab.
+  {
+    web_app_finalizer.RemoveLegacyInstallFinalizerForTesting();
+
+    base::RunLoop run_loop;
+    WebApplicationInfo info;
+    info.start_url = GURL(kStartUrl);
+    info.title = base::UTF8ToUTF16(kAppName);
+    info.open_as_window = true;
+    web_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();
+
+    EXPECT_EQ(provider->registrar().GetAppUserDisplayMode(kAppId),
+              blink::mojom::DisplayMode::kStandalone);
+  }
+}
+
+IN_PROC_BROWSER_TEST_F(StartupBrowserWithWebAppTest,
+                       PRE_LastUsedProfilesWithWebApp) {
+  BrowserAddedObserver added_observer;
+  content::RunAllTasksUntilIdle();
+  // Launching with an app opens the app window via a task, so the test
+  // might start before SelectFirstBrowser is called.
+  if (!browser()) {
+    added_observer.Wait();
+    SelectFirstBrowser();
+  }
+  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);
+  }
+
+  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(1u, 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(1u, 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/browser/web_applications/web_app_migration_user_display_mode_clean_up_browsertest.cc

@@ -27,18 +27,6 @@ const char kAppId[] = "dofnemchnjfeendjmdhaldenaiabpiad";
 const char kAppName[] = "Test App";
 const char kStartUrl[] = "https://test.com";
 
-size_t GetTestPreCount() {
-  constexpr base::StringPiece kPreTestPrefix = "PRE_";
-  base::StringPiece test_name =
-      testing::UnitTest::GetInstance()->current_test_info()->name();
-  size_t count = 0;
-  while (test_name.find(kPreTestPrefix, kPreTestPrefix.size() * count) ==
-         kPreTestPrefix.size() * count) {
-    ++count;
-  }
-  return count;
-}
-
 }  // namespace
 
 namespace web_app {

chrome/common/pref_names.cc

@@ -1605,7 +1605,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

chrome/test/base/in_process_browser_test.cc

@@ -377,6 +377,19 @@ void InProcessBrowserTest::TearDown() {
 #endif
 }
 
+// static
+size_t InProcessBrowserTest::GetTestPreCount() {
+  constexpr base::StringPiece kPreTestPrefix = "PRE_";
+  base::StringPiece test_name =
+      testing::UnitTest::GetInstance()->current_test_info()->name();
+  size_t count = 0;
+  while (base::StartsWith(test_name, kPreTestPrefix)) {
+    ++count;
+    test_name = test_name.substr(kPreTestPrefix.size());
+  }
+  return count;
+}
+
 void InProcessBrowserTest::SelectFirstBrowser() {
   const BrowserList* browser_list = BrowserList::GetInstance();
   if (!browser_list->empty())

chrome/test/base/in_process_browser_test.h

@@ -150,6 +150,11 @@ class InProcessBrowserTest : public content::BrowserTestBase {
     global_browser_set_up_function_ = set_up_function;
   }
 
+  // Counts the number of "PRE_" prefixes in the test name. This is used to
+  // differentiate between different PRE tests in browser test constructors
+  // and setup functions.
+  static size_t GetTestPreCount();
+
   // Returns the browser created by BrowserMain().
   // If no browser is created in BrowserMain(), this will return nullptr unless
   // another browser instance is created at a later time and