Update ephemeral Guest Profile handling when policy is set.

When |ForceEphemeralProfiles| is set to false, ProfileManager cannot
overwrite the pref for ephemeral Guest profiles.
To overcome the issue, a new |IsEphemeral| function is added to the
ProfileManager which not only checks for the pref value, but also
consider profile type.

Patchset-0 was previously reviewed and approved in crrev.com/c/2584025,
but reverted due to test timeout on Windows.

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

chrome/browser/profiles/profile_manager.cc

@@ -363,6 +363,11 @@ bool IsLoggedIn() {
 }
 #endif
 
+bool IsEphemeral(Profile* profile) {
+  return profile->GetPrefs()->GetBoolean(prefs::kForceEphemeralProfiles) ||
+         profile->IsEphemeralGuestProfile();
+}
+
 }  // namespace
 
 ProfileManager::ProfileManager(const base::FilePath& user_data_dir)
@@ -1560,8 +1565,7 @@ void ProfileManager::RemoveProfile(const base::FilePath& profile_dir) {
   DCHECK(base::Contains(profiles_info_, profile_dir));
 
   Profile* profile = GetProfileByPath(profile_dir);
-  bool ephemeral =
-      profile->GetPrefs()->GetBoolean(prefs::kForceEphemeralProfiles);
+  bool ephemeral = IsEphemeral(profile);
 
   // Remove from |profiles_info_|, eventually causing the Profile object's
   // destruction.
@@ -1910,12 +1914,10 @@ void ProfileManager::AddProfileToStorage(Profile* profile) {
       storage.GetProfileAttributesWithPath(profile->GetPath(), &entry);
   DCHECK(has_entry);
 
-  if (profile->IsEphemeralGuestProfile()) {
-    profile->GetPrefs()->SetBoolean(prefs::kForceEphemeralProfiles, true);
+  if (profile->IsEphemeralGuestProfile())
     entry->SetIsGuest(true);
-  }
 
-  if (profile->GetPrefs()->GetBoolean(prefs::kForceEphemeralProfiles))
+  if (IsEphemeral(profile))
     entry->SetIsEphemeral(true);
 
   entry->SetSignedInWithCredentialProvider(
@@ -1972,7 +1974,7 @@ void ProfileManager::SaveActiveProfiles() {
     // Some profiles might become ephemeral after they are created.
     // Don't persist the System Profile as one of the last actives, it should
     // never get a browser.
-    if (!(*it)->GetPrefs()->GetBoolean(prefs::kForceEphemeralProfiles) &&
+    if (!IsEphemeral(*it) &&
         profile_paths.find(profile_path) == profile_paths.end() &&
         profile_path !=
             base::FilePath(chrome::kSystemProfileDir).AsUTF8Unsafe()) {
@@ -1987,10 +1989,8 @@ void ProfileManager::OnBrowserOpened(Browser* browser) {
   DCHECK(browser);
   Profile* profile = browser->profile();
   DCHECK(profile);
-  bool is_ephemeral =
-      profile->GetPrefs()->GetBoolean(prefs::kForceEphemeralProfiles);
-  if (!profile->IsOffTheRecord() && !is_ephemeral && !browser->is_type_app() &&
-      ++browser_counts_[profile] == 1) {
+  if (!profile->IsOffTheRecord() && !IsEphemeral(profile) &&
+      !browser->is_type_app() && ++browser_counts_[profile] == 1) {
     active_profiles_.push_back(profile);
     SaveActiveProfiles();
   }
@@ -2037,7 +2037,7 @@ void ProfileManager::OnBrowserClosed(Browser* browser) {
   base::FilePath path = profile->GetPath();
   if (IsProfileDirectoryMarkedForDeletion(path)) {
     // Do nothing if the profile is already being deleted.
-  } else if (profile->GetPrefs()->GetBoolean(prefs::kForceEphemeralProfiles)) {
+  } else if (IsEphemeral(profile)) {
     // Avoid scheduling deletion if it's a testing profile that is not
     // registered with profile manager.
     if (profile->AsTestingProfile() &&
@@ -2117,7 +2117,7 @@ void ProfileManager::BrowserListObserver::OnBrowserSetLastActive(
 
   // Don't remember ephemeral profiles as last because they are not going to
   // persist after restart.
-  if (last_active->GetPrefs()->GetBoolean(prefs::kForceEphemeralProfiles))
+  if (IsEphemeral(last_active))
     return;
 
   profile_manager_->UpdateLastUser(last_active);

chrome/browser/profiles/profile_manager_browsertest.cc

@@ -17,6 +17,7 @@
 #include "build/chromeos_buildflags.h"
 #include "chrome/browser/apps/platform_apps/shortcut_manager.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
+#include "chrome/browser/policy/policy_test_utils.h"
 #include "chrome/browser/profiles/profile_attributes_entry.h"
 #include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ -25,6 +26,7 @@
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_window.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/testing_browser_process.h"
@@ -33,6 +35,8 @@
 #include "components/password_manager/core/browser/password_form.h"
 #include "components/password_manager/core/browser/password_store.h"
 #include "components/password_manager/core/browser/password_store_consumer.h"
+#include "components/policy/core/common/policy_map.h"
+#include "components/policy/policy_constants.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/browsing_data_remover.h"
 #include "content/public/test/browser_test.h"
@@ -748,3 +752,54 @@ IN_PROC_BROWSER_TEST_F(ProfileManagerBrowserTest, IncognitoProfile) {
                    ->GetFilePath(prefs::kSaveFileDefaultDirectory)
                    .empty());
 }
+
+#if !defined(OS_ANDROID) && !BUILDFLAG(IS_CHROMEOS_ASH)
+class EphemeralGuestProfilePolicyTest
+    : public policy::PolicyTest,
+      public ::testing::WithParamInterface<bool> {
+ public:
+  EphemeralGuestProfilePolicyTest() {
+    scoped_feature_list_.InitAndEnableFeature(
+        features::kEnableEphemeralGuestProfilesOnDesktop);
+  }
+
+ protected:
+  void SetUp() override {
+    // Shortcut deletion delays tests shutdown on Win-7 and results in time out.
+    // See crbug.com/1073451.
+#if defined(OS_WIN)
+    AppShortcutManager::SuppressShortcutsForTesting();
+#endif
+    InProcessBrowserTest::SetUp();
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+// TODO(https://crbug.com/1125474): Remove this comment!
+// If this test times out on Windows7 (or flaky on Windows 10), please disable
+// it and assign the bug to rhalavati@.
+IN_PROC_BROWSER_TEST_P(EphemeralGuestProfilePolicyTest,
+                       TestsForceEphemeralProfilesPolicy) {
+  policy::PolicyMap policies;
+  SetPolicy(&policies, policy::key::kForceEphemeralProfiles,
+            base::Value(GetParam()));
+  UpdateProviderPolicy(policies);
+
+  Profile* guest = CreateGuestBrowser()->profile();
+  EXPECT_TRUE(guest->IsEphemeralGuestProfile());
+
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+  ProfileAttributesEntry* entry;
+  EXPECT_TRUE(profile_manager->GetProfileAttributesStorage()
+                  .GetProfileAttributesWithPath(guest->GetPath(), &entry));
+  EXPECT_TRUE(entry->IsGuest());
+  EXPECT_TRUE(entry->IsEphemeral());
+}
+
+INSTANTIATE_TEST_SUITE_P(AllGuestProfileTypes,
+                         EphemeralGuestProfilePolicyTest,
+                         /*policy_is_enforced=*/testing::Bool());
+
+#endif  // !defined(OS_ANDROID) && !BUILDFLAG(IS_CHROMEOS_ASH)