Update Browser unittests for ephemeral Guest profiles.

Updates browser unittest and Browser Close unittest to cover ephemeral
Guest profiles.

Please see go/ephemeral-guest-profiles for more context.

Bug: 1125474
Change-Id: I62ab9f41abac7eedff1ff3b61e140997d95a12cf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2502369Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824413}

chrome/browser/profiles/profile_manager.cc

@@ -333,8 +333,8 @@ void OnProfileLoaded(ProfileManager::ProfileLoadedCallback client_callback,
 
 #if !defined(OS_ANDROID)
 // Helper function for ScheduleForcedEphemeralProfileForDeletion.
-bool IsProfileEphemeral(ProfileAttributesStorage* storage,
-                        const base::FilePath& profile_dir) {
+bool IsRegisteredAsEphemeral(ProfileAttributesStorage* storage,
+                             const base::FilePath& profile_dir) {
   ProfileAttributesEntry* entry = nullptr;
   return storage->GetProfileAttributesWithPath(profile_dir, &entry) &&
          entry->IsEphemeral();
@@ -1988,6 +1988,12 @@ void ProfileManager::OnBrowserClosed(Browser* browser) {
   if (IsProfileDirectoryMarkedForDeletion(path)) {
     // Do nothing if the profile is already being deleted.
   } else if (profile->GetPrefs()->GetBoolean(prefs::kForceEphemeralProfiles)) {
+    // Avoid scheduling deletion if it's a testing profile that is not
+    // registered with profile manager.
+    if (profile->AsTestingProfile() &&
+        !IsRegisteredAsEphemeral(&GetProfileAttributesStorage(), path)) {
+      return;
+    }
     // Delete if the profile is an ephemeral profile.
     ScheduleForcedEphemeralProfileForDeletion(path);
   }
@@ -2085,7 +2091,7 @@ void ProfileManager::OnNewActiveProfileLoaded(
 void ProfileManager::ScheduleForcedEphemeralProfileForDeletion(
     const base::FilePath& profile_dir) {
   DCHECK_EQ(0u, chrome::GetBrowserCount(GetProfileByPath(profile_dir)));
-  DCHECK(IsProfileEphemeral(&GetProfileAttributesStorage(), profile_dir));
+  DCHECK(IsRegisteredAsEphemeral(&GetProfileAttributesStorage(), profile_dir));
 
   base::Optional<base::FilePath> new_active_profile_dir =
       FindLastActiveProfile(base::BindRepeating(

chrome/browser/ui/browser.cc

@@ -2759,9 +2759,9 @@ void Browser::SyncHistoryWithTabs(int index) {
 
 bool Browser::CanCloseWithInProgressDownloads() {
 #if defined(OS_MAC) || defined(OS_CHROMEOS)
-  // On Mac and ChromeOS, non-incognito download can still continue after window
-  // is closed.
-  if (!profile_->IsOffTheRecord())
+  // On Mac and ChromeOS, non-incognito and non-Guest downloads can still
+  // continue after window is closed.
+  if (!profile_->IsOffTheRecord() && !profile_->IsEphemeralGuestProfile())
     return true;
 #endif
 

chrome/browser/ui/browser.h

@@ -696,7 +696,6 @@ class Browser : public TabStripModelObserver,
   FRIEND_TEST_ALL_PREFIXES(AppModeTest, EnableAppModeTest);
   FRIEND_TEST_ALL_PREFIXES(BrowserCommandControllerTest,
                            IsReservedCommandOrKeyIsApp);
-  FRIEND_TEST_ALL_PREFIXES(BrowserCloseTest, LastGuest);
   FRIEND_TEST_ALL_PREFIXES(BrowserCloseTest, LastIncognito);
   FRIEND_TEST_ALL_PREFIXES(BrowserCloseTest, LastRegular);
   FRIEND_TEST_ALL_PREFIXES(BrowserCommandControllerTest, AppFullScreen);
@@ -706,6 +705,7 @@ class Browser : public TabStripModelObserver,
                            DenyExitsFullscreen);
   FRIEND_TEST_ALL_PREFIXES(ExclusiveAccessTest,
                            TabEntersPresentationModeFromWindowed);
+  FRIEND_TEST_ALL_PREFIXES(GuestBrowserCloseTest, LastGuest);
   FRIEND_TEST_ALL_PREFIXES(StartupBrowserCreatorTest, OpenAppShortcutNoPref);
   FRIEND_TEST_ALL_PREFIXES(StartupBrowserCreatorTest,
                            OpenAppShortcutWindowPref);

chrome/browser/ui/browser_close_unittest.cc

@@ -130,14 +130,6 @@ class BrowserCloseTest : public testing::Test {
     return otr_profile;
   }
 
-  Profile* CreateGuestProfile(int windows, int downloads) {
-    TestingProfile* profile = profile_manager_.CreateGuestProfile();
-    Profile* incognito_profile = profile->GetPrimaryOTRProfile();
-    ConfigureCreatedProfile(incognito_profile, windows, downloads);
-
-    return incognito_profile;
-  }
-
   Browser* GetProfileBrowser(Profile* profile, int index) {
     CHECK(browsers_.end() != browsers_.find(profile));
     CHECK_GT(browsers_[profile].size(), static_cast<size_t>(index));
@@ -145,7 +137,9 @@ class BrowserCloseTest : public testing::Test {
     return browsers_[profile][index];
   }
 
- private:
+  TestingProfileManager* profile_manager() { return &profile_manager_; }
+
+ protected:
   void ConfigureCreatedProfile(Profile* profile,
                                int num_windows,
                                int num_downloads) {
@@ -177,6 +171,7 @@ class BrowserCloseTest : public testing::Test {
     browsers_[profile] = browsers;
   }
 
+ private:
   // Note that the vector elements are all owned by this class and must be
   // cleaned up.
   std::map<Profile*, std::vector<TestBrowserWindow*>> browser_windows_;
@@ -364,8 +359,32 @@ TEST_F(BrowserCloseTest, PluralIncognito) {
   EXPECT_EQ(2, num_downloads_blocking);
 }
 
+class GuestBrowserCloseTest : public BrowserCloseTest,
+                              public testing::WithParamInterface<bool> {
+ public:
+  GuestBrowserCloseTest() : is_ephemeral_(GetParam()) {
+    // Change the value if Ephemeral is not supported.
+    is_ephemeral_ &=
+        TestingProfile::SetScopedFeatureListForEphemeralGuestProfiles(
+            scoped_feature_list_, is_ephemeral_);
+  }
+
+  Profile* CreateGuestProfile(int windows, int downloads) {
+    TestingProfile* profile = profile_manager()->CreateGuestProfile();
+    Profile* guest_profile =
+        is_ephemeral_ ? profile : profile->GetPrimaryOTRProfile();
+    ConfigureCreatedProfile(guest_profile, windows, downloads);
+
+    return guest_profile;
+  }
+
+ private:
+  bool is_ephemeral_;
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
 // Last window close (guest window) will trigger warning.
-TEST_F(BrowserCloseTest, LastWindowGuest) {
+TEST_P(GuestBrowserCloseTest, LastWindowGuest) {
   Profile* guest_profile = CreateGuestProfile(1, 1);
   Browser* browser = GetProfileBrowser(guest_profile, 0);
 
@@ -376,7 +395,7 @@ TEST_F(BrowserCloseTest, LastWindowGuest) {
 }
 
 // Last guest window close triggers download warning.
-TEST_F(BrowserCloseTest, LastGuest) {
+TEST_P(GuestBrowserCloseTest, LastGuest) {
   CreateProfile(1, 0);
   Profile* profile = CreateGuestProfile(1, 1);
   Browser* browser(GetProfileBrowser(profile, 0));
@@ -390,7 +409,7 @@ TEST_F(BrowserCloseTest, LastGuest) {
 }
 
 // Last guest window close with no downloads => no warning.
-TEST_F(BrowserCloseTest, LastGuestNoDownloads) {
+TEST_P(GuestBrowserCloseTest, LastGuestNoDownloads) {
   Profile* profile = CreateGuestProfile(1, 0);
   Browser* browser = GetProfileBrowser(profile, 0);
 
@@ -400,7 +419,7 @@ TEST_F(BrowserCloseTest, LastGuestNoDownloads) {
 }
 
 // Non-last guest window => no warning.
-TEST_F(BrowserCloseTest, NonLastGuest) {
+TEST_P(GuestBrowserCloseTest, NonLastGuest) {
   Profile* profile = CreateGuestProfile(2, 1);
   Browser* browser = GetProfileBrowser(profile, 0);
 
@@ -408,3 +427,7 @@ TEST_F(BrowserCloseTest, NonLastGuest) {
   EXPECT_EQ(Browser::DownloadCloseType::kOk,
             browser->OkToCloseWithInProgressDownloads(&num_downloads_blocking));
 }
+
+INSTANTIATE_TEST_SUITE_P(AllGuestTypes,
+                         GuestBrowserCloseTest,
+                         /*is_ephemeral=*/testing::Bool());

chrome/browser/ui/browser_unittest.cc

@@ -6,6 +6,7 @@
 
 #include "base/macros.h"
 #include "chrome/app/chrome_command_ids.h"
+#include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
 #include "chrome/browser/prefs/incognito_mode_prefs.h"
 #include "chrome/browser/ui/browser_command_controller.h"
 #include "chrome/browser/ui/browser_commands.h"
@@ -13,10 +14,9 @@
 #include "chrome/test/base/browser_with_test_window_test.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/session_manager/core/session_manager.h"
-#include "components/user_manager/user_names.h"
-#include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
 #include "components/user_manager/fake_user_manager.h"
 #include "components/user_manager/scoped_user_manager.h"
+#include "components/user_manager/user_names.h"
 #include "components/zoom/zoom_controller.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/site_instance.h"
@@ -162,30 +162,6 @@ TEST_F(BrowserUnitTest, DisableZoomOnCrashedTab) {
   EXPECT_FALSE(chrome::CanZoomOut(raw_contents));
 }
 
-// Tests that Browser::Create creates a guest session browser for OTR profile
-// only.
-TEST_F(BrowserUnitTest, CreateGuestSessionBrowser) {
-  TestingProfile::Builder profile_builder;
-  profile_builder.SetGuestSession();
-  std::unique_ptr<TestingProfile> test_profile = profile_builder.Build();
-  TestingProfile::Builder otr_profile_builder;
-  otr_profile_builder.SetGuestSession();
-  otr_profile_builder.BuildIncognito(test_profile.get());
-
-  // Try creating a browser in original guest profile - it should fail.
-  EXPECT_EQ(Browser::BrowserCreationStatus::kErrorProfileUnsuitable,
-            Browser::GetBrowserCreationStatusForProfile(test_profile.get()));
-
-  // Creating a browser in OTR guest profile should succeed.
-  Browser::CreateParams off_the_record_create_params(
-      test_profile->GetPrimaryOTRProfile(), false);
-  std::unique_ptr<BrowserWindow> test_window(CreateBrowserWindow());
-  off_the_record_create_params.window = test_window.get();
-  std::unique_ptr<Browser> otr_browser(
-      Browser::Create(off_the_record_create_params));
-  EXPECT_TRUE(otr_browser);
-}
-
 TEST_F(BrowserUnitTest, CreateBrowserFailsIfProfileDisallowsBrowserWindows) {
   TestingProfile::Builder profile_builder;
   profile_builder.DisallowBrowserWindows();
@@ -410,3 +386,58 @@ TEST_F(BrowserBookmarkBarTest, StateOnActiveTabChanged) {
   EXPECT_EQ(BookmarkBar::SHOW, browser()->bookmark_bar_state());
   EXPECT_EQ(BookmarkBar::SHOW, window_bookmark_bar_state());
 }
+
+class GuestBrowserUnitTest : public BrowserUnitTest,
+                             public testing::WithParamInterface<bool> {
+ public:
+  GuestBrowserUnitTest() : is_ephemeral_(GetParam()) {
+    // Change the value if Ephemeral is not supported.
+    is_ephemeral_ &=
+        TestingProfile::SetScopedFeatureListForEphemeralGuestProfiles(
+            scoped_feature_list_, is_ephemeral_);
+  }
+
+  bool is_ephemeral() const { return is_ephemeral_; }
+
+ private:
+  bool is_ephemeral_;
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+// Tests that Browser::Create creates a guest session browser for appropriate
+// OTR/non-OTR type for old and ephemeral Guest profiles.
+TEST_P(GuestBrowserUnitTest, CreateGuestSessionBrowser) {
+  TestingProfile::Builder profile_builder;
+  profile_builder.SetGuestSession();
+  std::unique_ptr<TestingProfile> test_profile = profile_builder.Build();
+  TestingProfile::Builder otr_profile_builder;
+  otr_profile_builder.SetGuestSession();
+  Profile* guest_profile = nullptr;
+
+  if (is_ephemeral()) {
+    // Try creating an OTR profile for ephemeral Guest profile, it should fail.
+    EXPECT_FALSE(otr_profile_builder.BuildIncognito(test_profile.get()));
+    guest_profile = test_profile.get();
+  } else {
+    // Try creating a browser in original guest profile - it should fail.
+    EXPECT_EQ(Browser::BrowserCreationStatus::kErrorProfileUnsuitable,
+              Browser::GetBrowserCreationStatusForProfile(test_profile.get()));
+
+    // Create OTR profile for the Guest profile.
+    EXPECT_TRUE(otr_profile_builder.BuildIncognito(test_profile.get()));
+    guest_profile = test_profile->GetPrimaryOTRProfile();
+  }
+
+  // Creating a browser should succeed.
+  Browser::CreateParams create_params =
+      Browser::CreateParams(guest_profile, false);
+  std::unique_ptr<BrowserWindow> test_window = CreateBrowserWindow();
+  create_params.window = test_window.get();
+  std::unique_ptr<Browser> browser =
+      std::unique_ptr<Browser>(Browser::Create(create_params));
+  EXPECT_TRUE(browser);
+}
+
+INSTANTIATE_TEST_SUITE_P(AllGuestTypes,
+                         GuestBrowserUnitTest,
+                         /*is_ephemeral=*/testing::Bool());

chrome/test/base/testing_profile.cc

@@ -377,6 +377,9 @@ void TestingProfile::Init() {
   else
     CreateTestingPrefService();
 
+  if (guest_session_ && IsEphemeralGuestProfileEnabled())
+    GetPrefs()->SetBoolean(prefs::kForceEphemeralProfiles, true);
+
   key_->SetPrefs(prefs_.get());
   SimpleKeyMap::GetInstance()->Associate(this, key_.get());
 
@@ -1118,6 +1121,11 @@ TestingProfile* TestingProfile::Builder::BuildOffTheRecord(
   DCHECK(original_profile);
   build_called_ = true;
 
+  // Ephemeral guest profiles do not support Incognito.
+  if (original_profile->IsEphemeralGuestProfile() &&
+      otr_profile_id == OTRProfileID::PrimaryID())
+    return nullptr;
+
   // Note: Owned by |original_profile|.
   return new TestingProfile(
       path_, delegate_,

chrome/test/base/testing_profile_manager.cc

@@ -161,9 +161,11 @@ TestingProfile* TestingProfileManager::CreateGuestProfile() {
   profile_ptr->set_profile_name(kGuestProfileName);
 
   // Set up a profile with an off the record profile.
-  TestingProfile::Builder off_the_record_builder;
-  off_the_record_builder.SetGuestSession();
-  off_the_record_builder.BuildIncognito(profile_ptr);
+  if (!TestingProfile::IsEphemeralGuestProfileEnabled()) {
+    TestingProfile::Builder off_the_record_builder;
+    off_the_record_builder.SetGuestSession();
+    off_the_record_builder.BuildIncognito(profile_ptr);
+  }
 
   profile_manager_->AddProfile(std::move(profile));
   profile_manager_->SetNonPersonalProfilePrefs(profile_ptr);