Fix the System Profile to not load extensions. It could otherwise be forced to by GPO.

A few releases ago I introduced a System Profile to back the User Manager (https://codereview.chromium.org/847733005). This profile, however, is causing some problems. I'm trying to reduce those by making clearer the contract of what it's for: A profile that doesn't take a browser window, has no extensions, writes little to disk, etc.

This certainly addresses issue #4 raised within the bug, and address the other issues as well (I can't repro them). This CL generally hardens Chrome for the System Profile in a lot of the same ways we do the Guest Profile.

BUG=482176
[email protected]

Review URL: https://codereview.chromium.org/1129293002

Cr-Commit-Position: refs/heads/master@{#331276}
(cherry picked from commit 7831f57)

Review URL: https://codereview.chromium.org/1157273006

Cr-Commit-Position: refs/branch-heads/2403@{crosswalk-project#116}
Cr-Branched-From: f54b809-refs/heads/master@{#330231}

Author: Mike Lerman

chrome/browser/app_controller_mac.mm

@@ -1032,9 +1032,10 @@ - (void)commandDispatch:(id)sender {
   NSInteger tag = [sender tag];
 
   // If there are no browser windows, and we are trying to open a browser
-  // for a locked profile, we have to show the User Manager instead as the
-  // locked profile needs authentication.
-  if (IsProfileSignedOut(lastProfile)) {
+  // for a locked profile or the system profile, we have to show the User
+  // Manager instead as the locked profile needs authentication and the system
+  // profile cannot have a browser.
+  if (IsProfileSignedOut(lastProfile) || lastProfile->IsSystemProfile()) {
     UserManager::Show(base::FilePath(),
                       profiles::USER_MANAGER_NO_TUTORIAL,
                       profiles::USER_MANAGER_SELECT_PROFILE_NO_ACTION);
@@ -1246,11 +1247,12 @@ - (BOOL)applicationShouldHandleReopen:(NSApplication*)theApplication
 
   // Otherwise open a new window.
   // If the last profile was locked, we have to open the User Manager, as the
-  // profile requires authentication. Similarly, because guest mode is
-  // implemented as forced incognito, we can't open a new guest browser either,
-  // so we have to show the User Manager as well.
+  // profile requires authentication. Similarly, because guest mode and system
+  // profile are implemented as forced incognito, we can't open a new guest
+  // browser either, so we have to show the User Manager as well.
   Profile* lastProfile = [self lastProfile];
-  if (lastProfile->IsGuestSession() || IsProfileSignedOut(lastProfile)) {
+  if (lastProfile->IsGuestSession() || IsProfileSignedOut(lastProfile) ||
+      lastProfile->IsSystemProfile()) {
     UserManager::Show(base::FilePath(),
                       profiles::USER_MANAGER_NO_TUTORIAL,
                       profiles::USER_MANAGER_SELECT_PROFILE_NO_ACTION);

chrome/browser/extensions/chrome_process_manager_delegate.cc

@@ -41,7 +41,8 @@ bool ChromeProcessManagerDelegate::IsBackgroundPageAllowed(
     content::BrowserContext* context) const {
   Profile* profile = static_cast<Profile*>(context);
 
-  bool is_normal_session = !profile->IsGuestSession();
+  bool is_normal_session = !profile->IsGuestSession() &&
+                           !profile->IsSystemProfile();
 #if defined(OS_CHROMEOS)
   is_normal_session = is_normal_session &&
                       user_manager::UserManager::Get()->IsUserLoggedIn();

chrome/browser/extensions/extension_system_impl.cc

@@ -310,7 +310,8 @@ void ExtensionSystemImpl::Shared::Init(bool extensions_enabled) {
   // ExtensionService depends on RuntimeData.
   runtime_data_.reset(new RuntimeData(ExtensionRegistry::Get(profile_)));
 
-  bool autoupdate_enabled = !profile_->IsGuestSession();
+  bool autoupdate_enabled = !profile_->IsGuestSession() &&
+                            !profile_->IsSystemProfile();
 #if defined(OS_CHROMEOS)
   if (!extensions_enabled)
     autoupdate_enabled = false;

chrome/browser/extensions/tab_helper.cc

@@ -144,6 +144,7 @@ bool TabHelper::CanCreateApplicationShortcuts() const {
 
 bool TabHelper::CanCreateBookmarkApp() const {
   return !profile_->IsGuestSession() &&
+         !profile_->IsSystemProfile() &&
          IsValidBookmarkAppUrl(web_contents()->GetURL());
 }
 

chrome/browser/profiles/profile_manager.cc

@@ -304,6 +304,7 @@ Profile* ProfileManager::GetLastUsedProfile() {
 Profile* ProfileManager::GetLastUsedProfileAllowedByPolicy() {
   Profile* profile = GetLastUsedProfile();
   if (profile->IsGuestSession() ||
+      profile->IsSystemProfile() ||
       IncognitoModePrefs::GetAvailability(profile->GetPrefs()) ==
       IncognitoModePrefs::FORCED) {
     return profile->GetOffTheRecordProfile();
@@ -437,8 +438,10 @@ void ProfileManager::CreateProfileAsync(
     if (iter != profiles_info_.end() && info->created) {
       Profile* profile = info->profile.get();
       // If this was the guest profile, apply settings and go OffTheRecord.
-      if (profile->GetPath() == ProfileManager::GetGuestProfilePath()) {
-        SetGuestProfilePrefs(profile);
+      // The system profile also needs characteristics of being off the record,
+      // such as having no extensions, not writing to disk, etc.
+      if (profile->IsGuestSession() || profile->IsSystemProfile()) {
+        SetNonPersonalProfilePrefs(profile);
         profile = profile->GetOffTheRecordProfile();
       }
       // Profile has already been created. Run callback immediately.
@@ -1002,9 +1005,10 @@ void ProfileManager::OnProfileCreated(Profile* profile,
   }
 
   if (profile) {
-    // If this was the guest profile, finish setting its special status.
-    if (profile->GetPath() == ProfileManager::GetGuestProfilePath())
-      SetGuestProfilePrefs(profile);
+    // If this was the guest or system profile, finish setting its special
+    // status.
+    if (profile->IsGuestSession() || profile->IsSystemProfile())
+      SetNonPersonalProfilePrefs(profile);
 
     // Invoke CREATED callback for incognito profiles.
     if (go_off_the_record)
@@ -1323,16 +1327,12 @@ void ProfileManager::AddProfileToCache(Profile* profile) {
   }
 }
 
-void ProfileManager::SetGuestProfilePrefs(Profile* profile) {
+void ProfileManager::SetNonPersonalProfilePrefs(Profile* profile) {
   PrefService* prefs = profile->GetPrefs();
   prefs->SetBoolean(prefs::kSigninAllowed, false);
   prefs->SetBoolean(bookmarks::prefs::kEditBookmarksEnabled, false);
   prefs->SetBoolean(bookmarks::prefs::kShowBookmarkBar, false);
   prefs->ClearPref(DefaultSearchManager::kDefaultSearchProviderDataPrefName);
-  // This can be removed in the future but needs to be present through
-  // a release (or two) so that any existing installs get switched to
-  // the new state and away from the previous "forced" state.
-  IncognitoModePrefs::SetAvailability(prefs, IncognitoModePrefs::ENABLED);
 }
 
 bool ProfileManager::ShouldGoOffTheRecord(Profile* profile) {
@@ -1341,7 +1341,7 @@ bool ProfileManager::ShouldGoOffTheRecord(Profile* profile) {
     return true;
   }
 #endif
-  return profile->IsGuestSession();
+  return profile->IsGuestSession() || profile->IsSystemProfile();
 }
 
 void ProfileManager::RunCallbacks(const std::vector<CreateCallback>& callbacks,
@@ -1367,7 +1367,9 @@ void ProfileManager::UpdateLastUser(Profile* last_active) {
   PrefService* local_state = g_browser_process->local_state();
   DCHECK(local_state);
   // Only keep track of profiles that we are managing; tests may create others.
-  if (profiles_info_.find(last_active->GetPath()) != profiles_info_.end()) {
+  // Also never consider the SystemProfile as "active".
+  if (profiles_info_.find(last_active->GetPath()) != profiles_info_.end() &&
+      !last_active->IsSystemProfile()) {
     std::string profile_path_base =
         last_active->GetPath().BaseName().MaybeAsASCII();
     if (profile_path_base != GetLastUsedProfileName())

chrome/browser/profiles/profile_manager.h

@@ -286,8 +286,9 @@ class ProfileManager : public base::NonThreadSafe,
   // Adds |profile| to the profile info cache if it hasn't been added yet.
   void AddProfileToCache(Profile* profile);
 
-  // Apply settings for (desktop) Guest User profile.
-  void SetGuestProfilePrefs(Profile* profile);
+  // Apply settings for profiles created by the system rather than users: The
+  // (desktop) Guest User profile and (desktop) System Profile.
+  void SetNonPersonalProfilePrefs(Profile* profile);
 
   // For ChromeOS, determines if profile should be otr.
   bool ShouldGoOffTheRecord(Profile* profile);

chrome/browser/profiles/profile_window.cc

@@ -383,7 +383,7 @@ bool IsLockAvailable(Profile* profile) {
   if (!switches::IsNewProfileManagement())
     return false;
 
-  if (profile->IsGuestSession())
+  if (profile->IsGuestSession() || profile->IsSystemProfile())
     return false;
 
   std::string hosted_domain = profile->GetPrefs()->

chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc

@@ -374,9 +374,9 @@ ChromeLauncherController::ChromeLauncherController(Profile* profile,
     // Use the original profile as on chromeos we may get a temporary off the
     // record profile, unless in guest session (where off the record profile is
     // the right one).
-    Profile* active_profile = ProfileManager::GetActiveUserProfile();
-    profile_ = active_profile->IsGuestSession() ? active_profile :
-        active_profile->GetOriginalProfile();
+    profile_ = ProfileManager::GetActiveUserProfile();
+    if (!profile_->IsGuestSession() && !profile_->IsSystemProfile())
+      profile_ = profile_->GetOriginalProfile();
 
     app_sync_ui_state_ = AppSyncUIState::Get(profile_);
     if (app_sync_ui_state_)
@@ -2052,7 +2052,8 @@ bool ChromeLauncherController::IsIncognito(
     const content::WebContents* web_contents) const {
   const Profile* profile =
       Profile::FromBrowserContext(web_contents->GetBrowserContext());
-  return profile->IsOffTheRecord() && !profile->IsGuestSession();
+  return profile->IsOffTheRecord() && !profile->IsGuestSession() &&
+         !profile->IsSystemProfile();
 }
 
 void ChromeLauncherController::CloseWindowedAppsFromRemovedExtension(

chrome/browser/ui/browser.cc

@@ -365,6 +365,12 @@ Browser::Browser(const CreateParams& params)
   CHECK(IncognitoModePrefs::CanOpenBrowser(profile_));
   CHECK(!profile_->IsGuestSession() || profile_->IsOffTheRecord())
       << "Only off the record browser may be opened in guest mode";
+  DCHECK(!profile_->IsSystemProfile())
+      << "The system profile should never have a real browser.";
+  // TODO(mlerman): After this hits stable channel, see if there are counts
+  // for this metric. If not, change the DCHECK above to a CHECK.
+  if (profile_->IsSystemProfile())
+    content::RecordAction(base::UserMetricsAction("BrowserForSystemProfile"));
 
   // TODO(jeremy): Move to initializer list once flag is removed.
   if (IsFastTabUnloadEnabled())

chrome/browser/ui/browser_command_controller.cc

@@ -954,7 +954,10 @@ void BrowserCommandController::InitCommandState() {
   command_updater_.UpdateCommandEnabled(IDC_ZOOM_MINUS, true);
 
   // Show various bits of UI
-  const bool guest_session = profile()->IsGuestSession();
+  const bool guest_session = profile()->IsGuestSession() ||
+                             profile()->IsSystemProfile();
+  DCHECK(!profile()->IsSystemProfile())
+      << "Ought to never have browser for the system profile.";
   const bool normal_window = browser_->is_type_tabbed();
   UpdateOpenFileState(&command_updater_);
   command_updater_.UpdateCommandEnabled(IDC_CREATE_SHORTCUTS, false);
@@ -1193,6 +1196,7 @@ void BrowserCommandController::UpdateCommandsForBookmarkBar() {
   command_updater_.UpdateCommandEnabled(
       IDC_SHOW_BOOKMARK_BAR,
       browser_defaults::bookmarks_enabled && !profile()->IsGuestSession() &&
+          !profile()->IsSystemProfile() &&
           !profile()->GetPrefs()->IsManagedPreference(
               bookmarks::prefs::kShowBookmarkBar) &&
           IsShowingMainUI());

chrome/browser/ui/startup/startup_browser_creator.cc

@@ -682,15 +682,16 @@ bool StartupBrowserCreator::ProcessCmdLineImpl(
       bool signin_required = profile_index != std::string::npos &&
           profile_info.ProfileIsSigninRequiredAtIndex(profile_index);
 
-      // Guest or locked profiles cannot be re-opened on startup. The only
-      // exception is if there's already a Guest window open in a separate
+      // Guest, system or locked profiles cannot be re-opened on startup. The
+      // only exception is if there's already a Guest window open in a separate
       // process (for example, launching a new browser after clicking on a
       // downloaded file in Guest mode).
-      bool has_guest_browsers = last_used_profile->IsGuestSession() &&
+      bool guest_or_system = last_used_profile->IsGuestSession() ||
+                             last_used_profile->IsSystemProfile();
+      bool has_guest_browsers = guest_or_system &&
           chrome::GetTotalBrowserCountForProfile(
               last_used_profile->GetOffTheRecordProfile()) > 0;
-      if (signin_required ||
-          (last_used_profile->IsGuestSession() && !has_guest_browsers)) {
+      if (signin_required || (guest_or_system && !has_guest_browsers)) {
         profiles::UserManagerProfileSelected action =
             command_line.HasSwitch(switches::kShowAppList) ?
                 profiles::USER_MANAGER_SELECT_PROFILE_APP_LAUNCHER :

chrome/test/base/testing_profile_manager.cc

@@ -130,7 +130,7 @@ TestingProfile* TestingProfileManager::CreateGuestProfile() {
   TestingProfile::Builder().BuildIncognito(profile);
 
   profile_manager_->AddProfile(profile);  // Takes ownership.
-  profile_manager_->SetGuestProfilePrefs(profile);
+  profile_manager_->SetNonPersonalProfilePrefs(profile);
 
   testing_profiles_.insert(std::make_pair(kGuestProfileName, profile));
 

tools/metrics/actions/actions.xml

@@ -1781,6 +1781,15 @@ should be able to be added at any place in this file.
   <description>Please enter the description of this user action.</description>
 </action>
 
+<action name="BrowserForSystemProfile">
+  <owner>[email protected]</owner>
+  <description>
+    A browser opened with the System Profile. Tsk, tsk. Any non-zero amount
+    means something is using the system profile for a browser when it likely
+    shouldn't be.
+  </description>
+</action>
+
 <action name="BrowserPlugin.Guest.AbnormalDeath">
   <owner>Please list the metric's owners. Add more owner tags as needed.</owner>
   <description>Please enter the description of this user action.</description>