Fix login visual hitch on chromebook

chrome/browser/chromeos/login/wallpaper_manager.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/login/wallpaper_manager.h"
 
 #include <vector>
 
 #include "ash/shell.h"
 #include "base/command_line.h"
@@ skipping 208 matching lines @@
   }
 
   bool disable_new_oobe = CommandLine::ForCurrentProcess()->
       HasSwitch(switches::kDisableNewOobe);
 
   if (!user_manager->IsUserLoggedIn()) {
     if (!disable_new_oobe) {
       if (!WizardController::IsDeviceRegistered()) {
         SetDefaultWallpaper();
       } else {
+        CrosSettings::Get()->AddSettingsObserver(
+            kAccountsPrefShowUserNamesOnSignIn, this);
         InitializeRegisteredDeviceWallpaper();
       }
     }
     return;
   }
   SetUserWallpaper(user_manager->GetLoggedInUser()->email());
 }
 
 void WallpaperManager::Observe(int type,
                                const content::NotificationSource& source,
@@ skipping 21 matching lines @@
       if (should_cache_wallpaper_) {
         BrowserThread::PostDelayedTask(
             BrowserThread::UI, FROM_HERE,
             base::Bind(&WallpaperManager::CacheUsersWallpapers,
                        weak_factory_.GetWeakPtr()),
             base::TimeDelta::FromMilliseconds(kCacheWallpaperDelayMs));
         should_cache_wallpaper_ = false;
       }
       break;
     }
+    case chrome::NOTIFICATION_SYSTEM_SETTING_CHANGED: {
+      if (*content::Details<const std::string>(details).ptr() ==
+          kAccountsPrefShowUserNamesOnSignIn)

[pastarmovj, 2013/01/21 14:55:24]

Multi-line conditions needs { } around the body.

[bshe, 2013/01/21 22:53:07]

Done.

+        InitializeRegisteredDeviceWallpaper();
+      break;
+    }
     default:
       NOTREACHED() << "Unexpected notification " << type;
   }
 }
 
 void WallpaperManager::RemoveUserWallpaperInfo(const std::string& email) {
   PrefService* prefs = g_browser_process->local_state();
   DictionaryPrefUpdate prefs_wallpapers_info_update(prefs,
       prefs::kUsersWallpaperInfo);
   prefs_wallpapers_info_update->RemoveWithoutPathExpansion(email, NULL);
@@ skipping 270 matching lines @@
   current_wallpaper_path_.clear();
   SetUserWallpaper(last_selected_user_);
 }
 
 // WallpaperManager, private: --------------------------------------------------
 
 WallpaperManager::~WallpaperManager() {
   ClearObsoleteWallpaperPrefs();
   DBusThreadManager::Get()->GetPowerManagerClient()->RemoveObserver(this);
   system::TimezoneSettings::GetInstance()->RemoveObserver(this);
+  CrosSettings::Get()->RemoveSettingsObserver(
+      kAccountsPrefShowUserNamesOnSignIn, this);
+
   weak_factory_.InvalidateWeakPtrs();
 }
 
 void WallpaperManager::BatchUpdateWallpaper() {
   NOTIMPLEMENTED();
 }
 
 void WallpaperManager::CacheUsersWallpapers() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   UserList users = UserManager::Get()->GetUsers();
@@ skipping 73 matching lines @@
 
   base::WorkerPool::PostTask(
       FROM_HERE,
       base::Bind(&WallpaperManager::DeleteWallpaperInList,
                  base::Unretained(this),
                  file_to_remove),
       false);
 }
 
 void WallpaperManager::InitializeRegisteredDeviceWallpaper() {
-  if (CrosSettingsProvider::TEMPORARILY_UNTRUSTED ==
-      CrosSettings::Get()->PrepareTrustedValues(
-          base::Bind(&WallpaperManager::InitializeRegisteredDeviceWallpaper,
-                     base::Unretained(this)))) {
+  if (UserManager::Get()->IsUserLoggedIn())
     return;
-  }
+
   bool disable_boot_animation = CommandLine::ForCurrentProcess()->
       HasSwitch(switches::kDisableBootAnimation);
   bool show_users = true;
   bool result = CrosSettings::Get()->GetBoolean(

[pastarmovj, 2013/01/21 14:55:24]

Without the condition you removed on |PrepareTrustedValues| you can't not be
sure whether the value you get here is actually coming from the trusted policy
store or from a possibly expired cache. If this logic only controls which image
to show as a background it is not critical enough to care but if you make any
more grave decisions than that please consider reinstating the logic you had
here before.

[Nikita (slow), 2013/01/23 09:53:34]

Julian, were're fine here with using cached value. That were one of the sources
of slowdown comes from.

The rest of login screen UI behaves the same:
* Uses cached values on boot
* Subscribes for updates when they are changed
* Never proceeds with a real action like login if values are  not trusted

Yes, we only decide here which wallpaper to show on boot.
We want great boot UX so any slowdown here has to be eliminated.

Cases when cached value is different are rare. In this case it is when "show
user pods" option is changed. In that case we're fine to show wallpaper of the
user who logged in last time on boot and then change that to the default
wallpaper (that is shown when on sign in UI is shown).

[pastarmovj, 2013/01/23 10:05:59]

Cool, thanks for explaining! I just wanted to make sure this concern doesn't go
unnoticed. My main reason to insist on this was to make sure that the code would
handle correctly the case where there is a cached value saying we are showing
usernames but the actual policy has changed and then some portions of the UI
might see one and another portions different value. It is not so critical it is
just important the UI handles that gracefully.

[bshe, 2013/01/23 14:54:05]

Sorry pastarmovj, I missed your comment. Thanks Nikita for explansion. 

On 2013/01/23 10:05:59, pastarmovj wrote:

       kAccountsPrefShowUserNamesOnSignIn, &show_users);
   DCHECK(result) << "Unable to fetch setting "
                  << kAccountsPrefShowUserNamesOnSignIn;
   const chromeos::UserList& users = UserManager::Get()->GetUsers();
   if (!show_users || users.empty()) {
     // Boot into sign in form, preload default wallpaper.
     SetDefaultWallpaper();
     return;
   }
 
@@ skipping 240 matching lines @@
                           FROM_HERE,
                           base::Bind(&WallpaperManager::StartLoad,
                                      base::Unretained(this),
                                      email,
                                      info,
                                      update_wallpaper,
                                      valid_path));
 }
 
 }  // chromeos