diff options
27 files changed, 1227 insertions, 205 deletions
diff --git a/chrome/browser/chromeos/login/login_manager_test.cc b/chrome/browser/chromeos/login/login_manager_test.cc index 338b292..511839b 100644 --- a/chrome/browser/chromeos/login/login_manager_test.cc +++ b/chrome/browser/chromeos/login/login_manager_test.cc @@ -37,7 +37,6 @@ void LoginManagerTest::SetUpCommandLine(CommandLine* command_line) { command_line->AppendSwitch(chromeos::switches::kLoginManager); command_line->AppendSwitch(chromeos::switches::kForceLoginManagerInTests); command_line->AppendSwitch(::switches::kMultiProfiles); - InProcessBrowserTest::SetUpCommandLine(command_line); } void LoginManagerTest::SetUpInProcessBrowserTestFixture() { @@ -78,7 +77,10 @@ bool LoginManagerTest::AddUserTosession(const std::string& username, const std::string& password) { ExistingUserController* controller = ExistingUserController::current_controller(); - EXPECT_TRUE(controller != NULL); + if (!controller) { + ADD_FAILURE(); + return false; + } controller->Login(UserContext(username, password, std::string())); content::WindowedNotificationObserver( chrome::NOTIFICATION_SESSION_STARTED, diff --git a/chrome/browser/chromeos/login/screens/user_image_screen.cc b/chrome/browser/chromeos/login/screens/user_image_screen.cc index dd66720..ebe2fe1 100644 --- a/chrome/browser/chromeos/login/screens/user_image_screen.cc +++ b/chrome/browser/chromeos/login/screens/user_image_screen.cc @@ -4,9 +4,17 @@ #include "chrome/browser/chromeos/login/screens/user_image_screen.h" +#include <string> + +#include "base/bind.h" +#include "base/bind_helpers.h" #include "base/compiler_specific.h" +#include "base/location.h" +#include "base/logging.h" +#include "base/message_loop/message_loop_proxy.h" #include "base/metrics/histogram.h" #include "base/timer/timer.h" +#include "base/values.h" #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/chromeos/accessibility/accessibility_manager.h" #include "chrome/browser/chromeos/camera_detector.h" @@ -17,11 +25,18 @@ #include "chrome/browser/chromeos/login/user_image_manager.h" #include "chrome/browser/chromeos/login/user_manager.h" #include "chrome/browser/chromeos/login/wizard_controller.h" +#include "chrome/browser/policy/policy_service.h" +#include "chrome/browser/policy/profile_policy_connector.h" +#include "chrome/browser/policy/profile_policy_connector_factory.h" +#include "chrome/browser/profiles/profile.h" #include "chrome/common/url_constants.h" +#include "components/policy/core/common/policy_map.h" +#include "components/policy/core/common/policy_namespace.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/notification_service.h" #include "grit/generated_resources.h" #include "grit/theme_resources.h" +#include "policy/policy_constants.h" #include "third_party/skia/include/core/SkBitmap.h" #include "ui/base/l10n/l10n_util.h" #include "ui/base/resource/resource_bundle.h" @@ -57,8 +72,9 @@ UserImageScreen::UserImageScreen(ScreenObserver* screen_observer, user_has_selected_image_(false) { actor_->SetDelegate(this); SetProfilePictureEnabled(true); - registrar_.Add(this, chrome::NOTIFICATION_LOGIN_USER_IMAGE_CHANGED, - content::NotificationService::AllSources()); + notification_registrar_.Add(this, + chrome::NOTIFICATION_LOGIN_USER_IMAGE_CHANGED, + content::NotificationService::AllSources()); } UserImageScreen::~UserImageScreen() { @@ -104,7 +120,6 @@ void UserImageScreen::HideCurtain() { actor_->HideCurtain(); } - void UserImageScreen::OnImageDecoded(const ImageDecoder* decoder, const SkBitmap& decoded_image) { DCHECK_EQ(image_decoder_.get(), decoder); @@ -118,17 +133,16 @@ void UserImageScreen::OnDecodeImageFailed(const ImageDecoder* decoder) { } void UserImageScreen::OnInitialSync(bool local_image_updated) { - DCHECK(sync_timer_.get()); - sync_timer_->Stop(); - sync_timer_.reset(); - UserManager::Get()->GetUserImageManager()->GetSyncObserver()-> - RemoveObserver(this); + DCHECK(sync_timer_); if (!local_image_updated) { + sync_timer_.reset(); + UserManager::Get()->GetUserImageManager()->GetSyncObserver()-> + RemoveObserver(this); if (is_screen_ready_) HideCurtain(); return; } - get_screen_observer()->OnExit(ScreenObserver::USER_IMAGE_SELECTED); + ExitScreen(); } void UserImageScreen::OnSyncTimeout() { @@ -143,6 +157,15 @@ bool UserImageScreen::IsWaitingForSync() const { return sync_timer_.get() && sync_timer_->IsRunning(); } +void UserImageScreen::OnUserImagePolicyChanged(const base::Value* previous, + const base::Value* current) { + if (current) { + base::MessageLoopProxy::current()->DeleteSoon(FROM_HERE, + policy_registrar_.release()); + ExitScreen(); + } +} + void UserImageScreen::OnImageSelected(const std::string& image_type, const std::string& image_url, bool is_user_selection) { @@ -195,7 +218,7 @@ void UserImageScreen::OnImageAccepted() { uma_index, kHistogramImagesCount); } - get_screen_observer()->OnExit(ScreenObserver::USER_IMAGE_SELECTED); + ExitScreen(); } @@ -204,14 +227,20 @@ void UserImageScreen::SetProfilePictureEnabled(bool profile_picture_enabled) { return; profile_picture_enabled_ = profile_picture_enabled; if (profile_picture_enabled) { - registrar_.Add(this, chrome::NOTIFICATION_PROFILE_IMAGE_UPDATED, - content::NotificationService::AllSources()); - registrar_.Add(this, chrome::NOTIFICATION_PROFILE_IMAGE_UPDATE_FAILED, + notification_registrar_.Add(this, + chrome::NOTIFICATION_PROFILE_IMAGE_UPDATED, + content::NotificationService::AllSources()); + notification_registrar_.Add( + this, + chrome::NOTIFICATION_PROFILE_IMAGE_UPDATE_FAILED, content::NotificationService::AllSources()); } else { - registrar_.Remove(this, chrome::NOTIFICATION_PROFILE_IMAGE_UPDATED, + notification_registrar_.Remove(this, + chrome::NOTIFICATION_PROFILE_IMAGE_UPDATED, content::NotificationService::AllSources()); - registrar_.Remove(this, chrome::NOTIFICATION_PROFILE_IMAGE_UPDATE_FAILED, + notification_registrar_.Remove( + this, + chrome::NOTIFICATION_PROFILE_IMAGE_UPDATE_FAILED, content::NotificationService::AllSources()); } if (actor_) @@ -239,12 +268,42 @@ const User* UserImageScreen::GetUser() { void UserImageScreen::Show() { if (!actor_) return; + + DCHECK(!policy_registrar_); + Profile* profile = UserManager::Get()->GetProfileByUser(GetUser()); + if (profile) { + policy::PolicyService* policy_service = + policy::ProfilePolicyConnectorFactory::GetForProfile(profile)-> + policy_service(); + if (policy_service->GetPolicies( + policy::PolicyNamespace(policy::POLICY_DOMAIN_CHROME, + std::string())) + .Get(policy::key::kUserAvatarImage)) { + // If the user image is managed by policy, skip the screen because the + // user is not allowed to override a policy-set image. + ExitScreen(); + return; + } + + // Listen for policy changes. If at any point, the user image becomes + // managed by policy, the screen will close. + policy_registrar_.reset(new policy::PolicyChangeRegistrar( + policy_service, + policy::PolicyNamespace(policy::POLICY_DOMAIN_CHROME, std::string()))); + policy_registrar_->Observe( + policy::key::kUserAvatarImage, + base::Bind(&UserImageScreen::OnUserImagePolicyChanged, + base::Unretained(this))); + } else { + NOTREACHED(); + } + if (GetUser()->CanSyncImage()) { if (UserImageSyncObserver* sync_observer = UserManager::Get()->GetUserImageManager()->GetSyncObserver()) { // We have synced image already. if (sync_observer->is_synced()) { - get_screen_observer()->OnExit(ScreenObserver::USER_IMAGE_SELECTED); + ExitScreen(); return; } sync_observer->AddObserver(this); @@ -325,4 +384,14 @@ std::string UserImageScreen::profile_picture_data_url() { return profile_picture_data_url_; } +void UserImageScreen::ExitScreen() { + policy_registrar_.reset(); + sync_timer_.reset(); + UserImageSyncObserver* sync_observer = + UserManager::Get()->GetUserImageManager()->GetSyncObserver(); + if (sync_observer) + sync_observer->RemoveObserver(this); + get_screen_observer()->OnExit(ScreenObserver::USER_IMAGE_SELECTED); +} + } // namespace chromeos diff --git a/chrome/browser/chromeos/login/screens/user_image_screen.h b/chrome/browser/chromeos/login/screens/user_image_screen.h index 8c5f6dc..c07ad26 100644 --- a/chrome/browser/chromeos/login/screens/user_image_screen.h +++ b/chrome/browser/chromeos/login/screens/user_image_screen.h @@ -6,6 +6,7 @@ #define CHROME_BROWSER_CHROMEOS_LOGIN_SCREENS_USER_IMAGE_SCREEN_H_ #include "base/compiler_specific.h" +#include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "chrome/browser/chromeos/login/screens/user_image_screen_actor.h" #include "chrome/browser/chromeos/login/screens/wizard_screen.h" @@ -17,8 +18,13 @@ namespace base { class Timer; +class Value; }; +namespace policy { +class PolicyChangeRegistrar; +} + namespace chromeos { class UserImageScreen: public WizardScreen, @@ -77,6 +83,12 @@ class UserImageScreen: public WizardScreen, bool IsWaitingForSync() const; + // Called when the policy::key::kUserAvatarImage policy changes while the + // screen is being shown. If the policy is set, closes the screen because the + // user is not allowed to override a policy-set image. + void OnUserImagePolicyChanged(const base::Value* previous, + const base::Value* current); + const User* GetUser(); // Called when the camera presence check has been completed. @@ -85,7 +97,12 @@ class UserImageScreen: public WizardScreen, // Called when it's decided not to skip the screen. void HideCurtain(); - content::NotificationRegistrar registrar_; + // Closes the screen. + void ExitScreen(); + + content::NotificationRegistrar notification_registrar_; + + scoped_ptr<policy::PolicyChangeRegistrar> policy_registrar_; UserImageScreenActor* actor_; diff --git a/chrome/browser/chromeos/login/user_image_manager_browsertest.cc b/chrome/browser/chromeos/login/user_image_manager_browsertest.cc index 05c1ee4..a1b8f1d 100644 --- a/chrome/browser/chromeos/login/user_image_manager_browsertest.cc +++ b/chrome/browser/chromeos/login/user_image_manager_browsertest.cc @@ -4,11 +4,13 @@ #include <map> #include <string> +#include <vector> #include "base/basictypes.h" #include "base/compiler_specific.h" #include "base/file_util.h" #include "base/files/file_path.h" +#include "base/json/json_writer.h" #include "base/memory/linked_ptr.h" #include "base/memory/ref_counted.h" #include "base/memory/ref_counted_memory.h" @@ -21,6 +23,7 @@ #include "base/run_loop.h" #include "base/time/time.h" #include "base/values.h" +#include "chrome/browser/browser_process.h" #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/chromeos/login/default_user_images.h" #include "chrome/browser/chromeos/login/login_manager_test.h" @@ -32,22 +35,39 @@ #include "chrome/browser/chromeos/login/user_image_manager_impl.h" #include "chrome/browser/chromeos/login/user_image_manager_test_util.h" #include "chrome/browser/chromeos/login/user_manager.h" +#include "chrome/browser/chromeos/policy/cloud_external_data_manager_base_test_util.h" +#include "chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h" +#include "chrome/browser/chromeos/policy/user_cloud_policy_manager_factory_chromeos.h" +#include "chrome/browser/policy/cloud/cloud_policy_core.h" +#include "chrome/browser/policy/cloud/cloud_policy_store.h" +#include "chrome/browser/policy/cloud/policy_builder.h" +#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile_downloader.h" #include "chrome/common/chrome_paths.h" #include "chrome/test/base/in_process_browser_test.h" #include "chrome/test/base/testing_browser_process.h" +#include "chromeos/chromeos_paths.h" +#include "chromeos/dbus/cryptohome_client.h" +#include "chromeos/dbus/dbus_thread_manager.h" +#include "chromeos/dbus/fake_dbus_thread_manager.h" +#include "chromeos/dbus/fake_session_manager_client.h" +#include "chromeos/dbus/session_manager_client.h" #include "content/public/browser/notification_service.h" #include "content/public/browser/notification_source.h" #include "content/public/test/test_utils.h" +#include "crypto/rsa_private_key.h" #include "google_apis/gaia/oauth2_token_service.h" +#include "net/test/embedded_test_server/embedded_test_server.h" #include "net/url_request/test_url_fetcher_factory.h" #include "net/url_request/url_fetcher_delegate.h" #include "net/url_request/url_request_status.h" +#include "policy/proto/cloud_policy.pb.h" #include "testing/gtest/include/gtest/gtest.h" #include "third_party/skia/include/core/SkBitmap.h" #include "ui/base/layout.h" #include "ui/base/resource/resource_bundle.h" #include "ui/gfx/image/image_skia.h" +#include "url/gurl.h" namespace chromeos { @@ -56,6 +76,21 @@ namespace { const char kTestUser1[] = "test-user@example.com"; const char kTestUser2[] = "test-user2@example.com"; +policy::CloudPolicyStore* GetStoreForUser(const User* user) { + Profile* profile = UserManager::Get()->GetProfileByUser(user); + if (!profile) { + ADD_FAILURE(); + return NULL; + } + policy::UserCloudPolicyManagerChromeOS* policy_manager = + policy::UserCloudPolicyManagerFactoryChromeOS::GetForProfile(profile); + if (!policy_manager) { + ADD_FAILURE(); + return NULL; + } + return policy_manager->core()->store(); +} + } // namespace class UserImageManagerTest : public LoginManagerTest, @@ -65,6 +100,13 @@ class UserImageManagerTest : public LoginManagerTest, } // LoginManagerTest overrides: + virtual void SetUpInProcessBrowserTestFixture() OVERRIDE { + LoginManagerTest::SetUpInProcessBrowserTestFixture(); + + ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir_)); + ASSERT_TRUE(PathService::Get(chrome::DIR_USER_DATA, &user_data_dir_)); + } + virtual void SetUpOnMainThread() OVERRIDE { LoginManagerTest::SetUpOnMainThread(); local_state_ = g_browser_process->local_state(); @@ -168,19 +210,7 @@ class UserImageManagerTest : public LoginManagerTest, // Returns the image path for user |username| with specified |extension|. base::FilePath GetUserImagePath(const std::string& username, const std::string& extension) { - base::FilePath user_data_dir; - PathService::Get(chrome::DIR_USER_DATA, &user_data_dir); - return user_data_dir.Append(username).AddExtension(extension); - } - - // Returns the path to a test image that can be set as the user image. - base::FilePath GetTestImagePath() { - base::FilePath test_data_dir; - if (!PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir)) { - ADD_FAILURE(); - return base::FilePath(); - } - return test_data_dir.Append("chromeos").Append("avatar1.jpg"); + return user_data_dir_.Append(username).AddExtension(extension); } // Completes the download of all non-image profile data for the currently @@ -251,6 +281,9 @@ class UserImageManagerTest : public LoginManagerTest, } } + base::FilePath test_data_dir_; + base::FilePath user_data_dir_; + PrefService* local_state_; scoped_ptr<gfx::ImageSkia> decoded_image_; @@ -425,7 +458,8 @@ IN_PROC_BROWSER_TEST_F(UserImageManagerTest, SaveUserImageFromFile) { const User* user = UserManager::Get()->FindUser(kTestUser1); ASSERT_TRUE(user); - const base::FilePath custom_image_path = GetTestImagePath(); + const base::FilePath custom_image_path = + test_data_dir_.Append(test::kUserAvatarImage1RelativePath); const scoped_ptr<gfx::ImageSkia> custom_image = test::ImageLoader(custom_image_path).Load(); ASSERT_TRUE(custom_image); @@ -538,4 +572,298 @@ IN_PROC_BROWSER_TEST_F(UserImageManagerTest, ExpectNewUserImageInfo(kTestUser1, kFirstDefaultImageIndex, base::FilePath()); } +class UserImageManagerPolicyTest : public UserImageManagerTest, + public policy::CloudPolicyStore::Observer { + protected: + UserImageManagerPolicyTest() + : fake_dbus_thread_manager_(new chromeos::FakeDBusThreadManager), + fake_session_manager_client_(new chromeos::FakeSessionManagerClient) { + fake_dbus_thread_manager_->SetFakeClients(); + fake_dbus_thread_manager_->SetSessionManagerClient( + scoped_ptr<SessionManagerClient>(fake_session_manager_client_)); + } + + // UserImageManagerTest overrides: + virtual void SetUpInProcessBrowserTestFixture() OVERRIDE { + DBusThreadManager::SetInstanceForTesting(fake_dbus_thread_manager_); + UserImageManagerTest::SetUpInProcessBrowserTestFixture(); + } + + virtual void SetUpOnMainThread() OVERRIDE { + UserImageManagerTest::SetUpOnMainThread(); + + base::FilePath user_keys_dir; + ASSERT_TRUE(PathService::Get(chromeos::DIR_USER_POLICY_KEYS, + &user_keys_dir)); + const std::string sanitized_username = + chromeos::CryptohomeClient::GetStubSanitizedUsername(kTestUser1); + const base::FilePath user_key_file = + user_keys_dir.AppendASCII(sanitized_username) + .AppendASCII("policy.pub"); + std::vector<uint8> user_key_bits; + ASSERT_TRUE(user_policy_.GetSigningKey()->ExportPublicKey(&user_key_bits)); + ASSERT_TRUE(base::CreateDirectory(user_key_file.DirName())); + ASSERT_EQ(file_util::WriteFile( + user_key_file, + reinterpret_cast<const char*>(user_key_bits.data()), + user_key_bits.size()), + static_cast<int>(user_key_bits.size())); + user_policy_.policy_data().set_username(kTestUser1); + + ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady()); + + policy_image_ = test::ImageLoader(test_data_dir_.Append( + test::kUserAvatarImage2RelativePath)).Load(); + ASSERT_TRUE(policy_image_); + } + + // policy::CloudPolicyStore::Observer overrides: + virtual void OnStoreLoaded(policy::CloudPolicyStore* store) OVERRIDE { + if (run_loop_) + run_loop_->Quit(); + } + + virtual void OnStoreError(policy::CloudPolicyStore* store) OVERRIDE { + if (run_loop_) + run_loop_->Quit(); + } + + std::string ConstructPolicy(const std::string& relative_path) { + std::string image_data; + if (!base::ReadFileToString(test_data_dir_.Append(relative_path), + &image_data)) { + ADD_FAILURE(); + } + std::string policy; + base::JSONWriter::Write(policy::test::ConstructExternalDataReference( + embedded_test_server()->GetURL(std::string("/") + relative_path).spec(), + image_data).get(), + &policy); + return policy; + } + + policy::UserPolicyBuilder user_policy_; + FakeDBusThreadManager* fake_dbus_thread_manager_; + FakeSessionManagerClient* fake_session_manager_client_; + + scoped_ptr<gfx::ImageSkia> policy_image_; + + private: + DISALLOW_COPY_AND_ASSIGN(UserImageManagerPolicyTest); +}; + +IN_PROC_BROWSER_TEST_F(UserImageManagerPolicyTest, PRE_SetAndClear) { + RegisterUser(kTestUser1); + chromeos::StartupUtils::MarkOobeCompleted(); +} + +// Verifies that the user image can be set through policy. Also verifies that +// after the policy has been cleared, the user is able to choose a different +// image. +IN_PROC_BROWSER_TEST_F(UserImageManagerPolicyTest, SetAndClear) { + const User* user = UserManager::Get()->FindUser(kTestUser1); + ASSERT_TRUE(user); + + LoginUser(kTestUser1); + base::RunLoop().RunUntilIdle(); + + policy::CloudPolicyStore* store = GetStoreForUser(user); + ASSERT_TRUE(store); + + // Set policy. Verify that the policy-provided user image is downloaded, set + // and persisted. + user_policy_.payload().mutable_useravatarimage()->set_value( + ConstructPolicy(test::kUserAvatarImage2RelativePath)); + user_policy_.Build(); + fake_session_manager_client_->set_user_policy(kTestUser1, + user_policy_.GetBlob()); + run_loop_.reset(new base::RunLoop); + store->Load(); + run_loop_->Run(); + + EXPECT_FALSE(user->HasDefaultImage()); + EXPECT_EQ(User::kExternalImageIndex, user->image_index()); + EXPECT_TRUE(test::AreImagesEqual(*policy_image_, user->image())); + ExpectNewUserImageInfo(kTestUser1, + User::kExternalImageIndex, + GetUserImagePath(kTestUser1, "jpg")); + + scoped_ptr<gfx::ImageSkia> saved_image = + test::ImageLoader(GetUserImagePath(kTestUser1, "jpg")).Load(); + ASSERT_TRUE(saved_image); + + // Check image dimensions. Images can't be compared since JPEG is lossy. + EXPECT_EQ(policy_image_->width(), saved_image->width()); + EXPECT_EQ(policy_image_->height(), saved_image->height()); + + // Clear policy. Verify that the policy-provided user image remains set as no + // different user image has been chosen yet. + user_policy_.payload().Clear(); + user_policy_.Build(); + fake_session_manager_client_->set_user_policy(kTestUser1, + user_policy_.GetBlob()); + run_loop_.reset(new base::RunLoop); + store->AddObserver(this); + store->Load(); + run_loop_->Run(); + store->RemoveObserver(this); + base::RunLoop().RunUntilIdle(); + + EXPECT_FALSE(user->HasDefaultImage()); + EXPECT_EQ(User::kExternalImageIndex, user->image_index()); + EXPECT_TRUE(test::AreImagesEqual(*policy_image_, user->image())); + ExpectNewUserImageInfo(kTestUser1, + User::kExternalImageIndex, + GetUserImagePath(kTestUser1, "jpg")); + + saved_image = test::ImageLoader(GetUserImagePath(kTestUser1, "jpg")).Load(); + ASSERT_TRUE(saved_image); + + // Check image dimensions. Images can't be compared since JPEG is lossy. + EXPECT_EQ(policy_image_->width(), saved_image->width()); + EXPECT_EQ(policy_image_->height(), saved_image->height()); + + // Choose a different user image. Verify that the chosen user image is set and + // persisted. + const gfx::ImageSkia& default_image = + GetDefaultImage(kFirstDefaultImageIndex); + + UserImageManager* user_image_manager = + UserManager::Get()->GetUserImageManager(); + user_image_manager->SaveUserDefaultImageIndex(kTestUser1, + kFirstDefaultImageIndex); + + EXPECT_TRUE(user->HasDefaultImage()); + EXPECT_EQ(kFirstDefaultImageIndex, user->image_index()); + EXPECT_TRUE(test::AreImagesEqual(default_image, user->image())); + ExpectNewUserImageInfo(kTestUser1, kFirstDefaultImageIndex, base::FilePath()); +} + +IN_PROC_BROWSER_TEST_F(UserImageManagerPolicyTest, PRE_PolicyOverridesUser) { + RegisterUser(kTestUser1); + chromeos::StartupUtils::MarkOobeCompleted(); +} + +// Verifies that when the user chooses a user image and a different image is +// then set through policy, the policy takes precedence, overriding the +// previously chosen image. +IN_PROC_BROWSER_TEST_F(UserImageManagerPolicyTest, PolicyOverridesUser) { + const User* user = UserManager::Get()->FindUser(kTestUser1); + ASSERT_TRUE(user); + + LoginUser(kTestUser1); + base::RunLoop().RunUntilIdle(); + + policy::CloudPolicyStore* store = GetStoreForUser(user); + ASSERT_TRUE(store); + + // Choose a user image. Verify that the chosen user image is set and + // persisted. + const gfx::ImageSkia& default_image = + GetDefaultImage(kFirstDefaultImageIndex); + + UserImageManager* user_image_manager = + UserManager::Get()->GetUserImageManager(); + user_image_manager->SaveUserDefaultImageIndex(kTestUser1, + kFirstDefaultImageIndex); + + EXPECT_TRUE(user->HasDefaultImage()); + EXPECT_EQ(kFirstDefaultImageIndex, user->image_index()); + EXPECT_TRUE(test::AreImagesEqual(default_image, user->image())); + ExpectNewUserImageInfo(kTestUser1, kFirstDefaultImageIndex, base::FilePath()); + + // Set policy. Verify that the policy-provided user image is downloaded, set + // and persisted, overriding the previously set image. + user_policy_.payload().mutable_useravatarimage()->set_value( + ConstructPolicy(test::kUserAvatarImage2RelativePath)); + user_policy_.Build(); + fake_session_manager_client_->set_user_policy(kTestUser1, + user_policy_.GetBlob()); + run_loop_.reset(new base::RunLoop); + store->Load(); + run_loop_->Run(); + + EXPECT_FALSE(user->HasDefaultImage()); + EXPECT_EQ(User::kExternalImageIndex, user->image_index()); + EXPECT_TRUE(test::AreImagesEqual(*policy_image_, user->image())); + ExpectNewUserImageInfo(kTestUser1, + User::kExternalImageIndex, + GetUserImagePath(kTestUser1, "jpg")); + + scoped_ptr<gfx::ImageSkia> saved_image = + test::ImageLoader(GetUserImagePath(kTestUser1, "jpg")).Load(); + ASSERT_TRUE(saved_image); + + // Check image dimensions. Images can't be compared since JPEG is lossy. + EXPECT_EQ(policy_image_->width(), saved_image->width()); + EXPECT_EQ(policy_image_->height(), saved_image->height()); +} + +IN_PROC_BROWSER_TEST_F(UserImageManagerPolicyTest, + PRE_UserDoesNotOverridePolicy) { + RegisterUser(kTestUser1); + chromeos::StartupUtils::MarkOobeCompleted(); +} + +// Verifies that when the user image has been set through policy and the user +// chooses a different image, the policy takes precedence, preventing the user +// from overriding the previously chosen image. +IN_PROC_BROWSER_TEST_F(UserImageManagerPolicyTest, UserDoesNotOverridePolicy) { + const User* user = UserManager::Get()->FindUser(kTestUser1); + ASSERT_TRUE(user); + + LoginUser(kTestUser1); + base::RunLoop().RunUntilIdle(); + + policy::CloudPolicyStore* store = GetStoreForUser(user); + ASSERT_TRUE(store); + + // Set policy. Verify that the policy-provided user image is downloaded, set + // and persisted. + user_policy_.payload().mutable_useravatarimage()->set_value( + ConstructPolicy(test::kUserAvatarImage2RelativePath)); + user_policy_.Build(); + fake_session_manager_client_->set_user_policy(kTestUser1, + user_policy_.GetBlob()); + run_loop_.reset(new base::RunLoop); + store->Load(); + run_loop_->Run(); + + EXPECT_FALSE(user->HasDefaultImage()); + EXPECT_EQ(User::kExternalImageIndex, user->image_index()); + EXPECT_TRUE(test::AreImagesEqual(*policy_image_, user->image())); + ExpectNewUserImageInfo(kTestUser1, + User::kExternalImageIndex, + GetUserImagePath(kTestUser1, "jpg")); + + scoped_ptr<gfx::ImageSkia> saved_image = + test::ImageLoader(GetUserImagePath(kTestUser1, "jpg")).Load(); + ASSERT_TRUE(saved_image); + + // Check image dimensions. Images can't be compared since JPEG is lossy. + EXPECT_EQ(policy_image_->width(), saved_image->width()); + EXPECT_EQ(policy_image_->height(), saved_image->height()); + + // Choose a different user image. Verify that the user image does not change + // as policy takes precedence. + UserImageManager* user_image_manager = + UserManager::Get()->GetUserImageManager(); + user_image_manager->SaveUserDefaultImageIndex(kTestUser1, + kFirstDefaultImageIndex); + + EXPECT_FALSE(user->HasDefaultImage()); + EXPECT_EQ(User::kExternalImageIndex, user->image_index()); + EXPECT_TRUE(test::AreImagesEqual(*policy_image_, user->image())); + ExpectNewUserImageInfo(kTestUser1, + User::kExternalImageIndex, + GetUserImagePath(kTestUser1, "jpg")); + + saved_image = test::ImageLoader(GetUserImagePath(kTestUser1, "jpg")).Load(); + ASSERT_TRUE(saved_image); + + // Check image dimensions. Images can't be compared since JPEG is lossy. + EXPECT_EQ(policy_image_->width(), saved_image->width()); + EXPECT_EQ(policy_image_->height(), saved_image->height()); +} + } // namespace chromeos diff --git a/chrome/browser/chromeos/login/user_image_manager_impl.cc b/chrome/browser/chromeos/login/user_image_manager_impl.cc index 8b1a53b..37f351d 100644 --- a/chrome/browser/chromeos/login/user_image_manager_impl.cc +++ b/chrome/browser/chromeos/login/user_image_manager_impl.cc @@ -18,6 +18,7 @@ #include "base/prefs/scoped_user_pref_update.h" #include "base/rand_util.h" #include "base/sequenced_task_runner.h" +#include "base/stl_util.h" #include "base/task_runner_util.h" #include "base/threading/sequenced_worker_pool.h" #include "base/time/time.h" @@ -28,13 +29,16 @@ #include "chrome/browser/chromeos/login/user_image.h" #include "chrome/browser/chromeos/login/user_image_sync_observer.h" #include "chrome/browser/chromeos/login/user_manager.h" +#include "chrome/browser/chromeos/policy/device_local_account_policy_service.h" +#include "chrome/browser/chromeos/settings/cros_settings.h" +#include "chrome/browser/policy/browser_policy_connector.h" #include "chrome/browser/profiles/profile_downloader.h" #include "chrome/browser/profiles/profile_manager.h" #include "chrome/common/chrome_paths.h" -#include "chrome/common/chrome_switches.h" #include "chromeos/chromeos_switches.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/notification_service.h" +#include "policy/policy_constants.h" #include "ui/gfx/image/image_skia.h" namespace chromeos { @@ -218,11 +222,15 @@ class UserImageManagerImpl::Job { void SetToImage(int image_index, const UserImage& user_image); - // Loads the the image at |path|, transcodes it to JPEG format, saves the - // image to disk and sets the user image for |user_id_| in local state to that - // image. If |resize| is true, the image is cropped and resized before - // transcoding. Also updates the |user| object for |user_id_| with the new - // image. + // Decodes the JPEG image |data|, crops and resizes the image, saves it to + // disk and sets the user image for |user_id_| in local state to that image. + // Also updates the |user| object for |user_id_| with the new image. + void SetToImageData(scoped_ptr<std::string> data); + + // Loads the image at |path|, transcodes it to JPEG format, saves the image to + // disk and sets the user image for |user_id_| in local state to that image. + // If |resize| is true, the image is cropped and resized before transcoding. + // Also updates the |user| object for |user_id_| with the new image. void SetToPath(const base::FilePath& path, int image_index, const GURL& image_url, @@ -342,6 +350,29 @@ void UserImageManagerImpl::Job::SetToImage(int image_index, SaveImageAndUpdateLocalState(); } +void UserImageManagerImpl::Job::SetToImageData(scoped_ptr<std::string> data) { + DCHECK(!run_); + run_ = true; + + image_index_ = User::kExternalImageIndex; + + // This method uses the image_loader_, not the unsafe_image_loader_: + // * This is necessary because the method is used to update the user image + // whenever the policy for a user is set. In the case of device-local + // accounts, policy may change at any time, even if the user is not + // currently logged in (and thus, the unsafe_image_loader_ may not be used). + // * This is possible because only JPEG |data| is accepted. No support for + // other image file formats is needed. + // * This is safe because the image_loader_ employs a hardened JPEG decoder + // that protects against malicious invalid image data being used to attack + // the login screen or another user session currently in progress. + parent_->image_loader_->Start(data.Pass(), + login::kMaxUserImageSize, + base::Bind(&Job::OnLoadImageDone, + weak_factory_.GetWeakPtr(), + true)); +} + void UserImageManagerImpl::Job::SetToPath(const base::FilePath& path, int image_index, const GURL& image_url, @@ -371,7 +402,7 @@ void UserImageManagerImpl::Job::OnLoadImageDone(bool save, } void UserImageManagerImpl::Job::UpdateUser() { - User* user = UserManager::Get()->FindUserAndModify(user_id_); + User* user = parent_->user_manager_->FindUserAndModify(user_id_); if (!user) return; @@ -405,7 +436,7 @@ void UserImageManagerImpl::Job::OnSaveImageDone(bool success) { void UserImageManagerImpl::Job::UpdateLocalState() { // Ignore if data stored or cached outside the user's cryptohome is to be // treated as ephemeral. - if (UserManager::Get()->IsUserNonCryptohomeDataEphemeral(user_id_)) + if (parent_->user_manager_->IsUserNonCryptohomeDataEphemeral(user_id_)) return; scoped_ptr<base::DictionaryValue> entry(new base::DictionaryValue); @@ -417,15 +448,17 @@ void UserImageManagerImpl::Job::UpdateLocalState() { kUserImageProperties); update->SetWithoutPathExpansion(user_id_, entry.release()); - UserManager::Get()->NotifyLocalStateChanged(); + parent_->user_manager_->NotifyLocalStateChanged(); } void UserImageManagerImpl::Job::NotifyJobDone() { parent_->OnJobDone(user_id_); } -UserImageManagerImpl::UserImageManagerImpl() - : downloading_profile_image_(false), +UserImageManagerImpl::UserImageManagerImpl(CrosSettings* cros_settings, + UserManager* user_manager) + : user_manager_(user_manager), + downloading_profile_image_(false), profile_image_requested_(false), weak_factory_(this) { base::SequencedWorkerPool* blocking_pool = @@ -438,6 +471,14 @@ UserImageManagerImpl::UserImageManagerImpl() background_task_runner_); unsafe_image_loader_ = new UserImageLoader(ImageDecoder::DEFAULT_CODEC, background_task_runner_); + policy_observer_.reset(new policy::CloudExternalDataPolicyObserver( + cros_settings, + user_manager, + g_browser_process->browser_policy_connector()-> + GetDeviceLocalAccountPolicyService(), + policy::key::kUserAvatarImage, + this)); + policy_observer_->Init(); } UserImageManagerImpl::~UserImageManagerImpl() { @@ -472,6 +513,12 @@ void UserImageManagerImpl::LoadUserImages(const UserList& users) { &image_properties); } + // If the user image for |user_id| is managed by policy and the policy-set + // image is being loaded and persisted right now, let that job continue. It + // will update the user image when done. + if (IsUserImageManaged(user_id) && ContainsKey(jobs_, user_id)) + continue; + if (!image_properties) { SetInitialUserImage(user_id); continue; @@ -517,7 +564,7 @@ void UserImageManagerImpl::LoadUserImages(const UserList& users) { void UserImageManagerImpl::UserLoggedIn(const std::string& user_id, bool user_is_new, bool user_is_local) { - User* user = UserManager::Get()->GetLoggedInUser(); + User* user = user_manager_->GetLoggedInUser(); if (user_is_new) { if (!user_is_local) SetInitialUserImage(user_id); @@ -526,7 +573,8 @@ void UserImageManagerImpl::UserLoggedIn(const std::string& user_id, ImageIndexToHistogramIndex(user->image_index()), kHistogramImagesCount); - if (users_to_migrate_.find(user_id) != users_to_migrate_.end()) { + if (!IsUserImageManaged(user_id) && + ContainsKey(users_to_migrate_, user_id)) { const DictionaryValue* prefs_images_unsafe = g_browser_process->local_state()->GetDictionary(kUserImages); const base::DictionaryValue* image_properties = NULL; @@ -554,7 +602,7 @@ void UserImageManagerImpl::UserLoggedIn(const std::string& user_id, profile_image_url_ = GURL(); profile_image_requested_ = false; - if (UserManager::Get()->IsLoggedInAsRegularUser()) { + if (user_manager_->IsLoggedInAsRegularUser()) { TryToInitDownloadedProfileImage(); // Schedule an initial download of the profile data (full name and @@ -579,22 +627,14 @@ void UserImageManagerImpl::UserLoggedIn(const std::string& user_id, profile_download_periodic_timer_.Stop(); } - const CommandLine* command_line = CommandLine::ForCurrentProcess(); - if (user_image_sync_observer_.get() && - !command_line->HasSwitch(::switches::kMultiProfiles)) { - NOTREACHED() << "User logged in more than once."; - } - - if (user->CanSyncImage() && - !command_line->HasSwitch(chromeos::switches::kDisableUserImageSync)) { - user_image_sync_observer_.reset(new UserImageSyncObserver(user)); - } else { - user_image_sync_observer_.reset(); - } + user_image_sync_observer_.reset(); + TryToCreateImageSyncObserver(); } void UserImageManagerImpl::SaveUserDefaultImageIndex(const std::string& user_id, int default_image_index) { + if (IsUserImageManaged(user_id)) + return; linked_ptr<Job>& job = jobs_[user_id]; job.reset(new Job(this, user_id)); job->SetToDefaultImage(default_image_index); @@ -602,6 +642,8 @@ void UserImageManagerImpl::SaveUserDefaultImageIndex(const std::string& user_id, void UserImageManagerImpl::SaveUserImage(const std::string& user_id, const UserImage& user_image) { + if (IsUserImageManaged(user_id)) + return; linked_ptr<Job>& job = jobs_[user_id]; job.reset(new Job(this, user_id)); job->SetToImage(User::kExternalImageIndex, user_image); @@ -609,6 +651,8 @@ void UserImageManagerImpl::SaveUserImage(const std::string& user_id, void UserImageManagerImpl::SaveUserImageFromFile(const std::string& user_id, const base::FilePath& path) { + if (IsUserImageManaged(user_id)) + return; linked_ptr<Job>& job = jobs_[user_id]; job.reset(new Job(this, user_id)); job->SetToPath(path, User::kExternalImageIndex, GURL(), true); @@ -616,6 +660,8 @@ void UserImageManagerImpl::SaveUserImageFromFile(const std::string& user_id, void UserImageManagerImpl::SaveUserImageFromProfileImage( const std::string& user_id) { + if (IsUserImageManaged(user_id)) + return; // Use the profile image if it has been downloaded already. Otherwise, use a // stub image (gray avatar). linked_ptr<Job>& job = jobs_[user_id]; @@ -641,6 +687,10 @@ void UserImageManagerImpl::DownloadProfileImage(const std::string& reason) { DownloadProfileData(reason); } +const gfx::ImageSkia& UserImageManagerImpl::DownloadedProfileImage() const { + return downloaded_profile_image_; +} + UserImageSyncObserver* UserImageManagerImpl::GetSyncObserver() const { return user_image_sync_observer_.get(); } @@ -648,59 +698,42 @@ UserImageSyncObserver* UserImageManagerImpl::GetSyncObserver() const { void UserImageManagerImpl::Shutdown() { profile_downloader_.reset(); user_image_sync_observer_.reset(); + policy_observer_.reset(); } -const gfx::ImageSkia& UserImageManagerImpl::DownloadedProfileImage() const { - return downloaded_profile_image_; -} +void UserImageManagerImpl::OnExternalDataSet(const std::string& policy, + const std::string& user_id) { + DCHECK_EQ(policy::key::kUserAvatarImage, policy); + if (IsUserImageManaged(user_id)) + return; + users_with_managed_images_.insert(user_id); -void UserImageManagerImpl::SetInitialUserImage(const std::string& user_id) { - // Choose a random default image. - SaveUserDefaultImageIndex(user_id, - base::RandInt(kFirstDefaultImageIndex, - kDefaultImagesCount - 1)); -} + jobs_.erase(user_id); -void UserImageManagerImpl::TryToInitDownloadedProfileImage() { - const User* user = UserManager::Get()->GetLoggedInUser(); - if (user->image_index() == User::kProfileImageIndex && - downloaded_profile_image_.isNull() && - !user->image_is_stub()) { - // Initialize the |downloaded_profile_image_| for the currently logged-in - // user if it has not been initialized already, the user image is the - // profile image and the user image has been loaded successfully. - VLOG(1) << "Profile image initialized from disk."; - downloaded_profile_image_ = user->image(); - profile_image_url_ = user->image_url(); - } + const User* logged_in_user = user_manager_->GetLoggedInUser(); + // If the user image for the currently logged-in user became managed, stop the + // sync observer so that the policy-set image does not get synced out. + if (logged_in_user && logged_in_user->email() == user_id) + user_image_sync_observer_.reset(); } -bool UserImageManagerImpl::NeedProfileImage() const { - return UserManager::Get()->IsLoggedInAsRegularUser() && - (UserManager::Get()->GetLoggedInUser()->image_index() == - User::kProfileImageIndex || - profile_image_requested_); +void UserImageManagerImpl::OnExternalDataCleared(const std::string& policy, + const std::string& user_id) { + DCHECK_EQ(policy::key::kUserAvatarImage, policy); + users_with_managed_images_.erase(user_id); + TryToCreateImageSyncObserver(); } -void UserImageManagerImpl::DownloadProfileData(const std::string& reason) { - // GAIA profiles exist for regular users only. - if (!UserManager::Get()->IsLoggedInAsRegularUser()) - return; - - // If a download is already in progress, allow it to continue, with one - // exception: If the current download does not include the profile image but - // the image has since become necessary, start a new download that includes - // the profile image. - if (profile_downloader_ && - (downloading_profile_image_ || !NeedProfileImage())) { - return; +void UserImageManagerImpl::OnExternalDataFetched(const std::string& policy, + const std::string& user_id, + scoped_ptr<std::string> data) { + DCHECK_EQ(policy::key::kUserAvatarImage, policy); + DCHECK(IsUserImageManaged(user_id)); + if (data) { + linked_ptr<Job>& job = jobs_[user_id]; + job.reset(new Job(this, user_id)); + job->SetToImageData(data.Pass()); } - - downloading_profile_image_ = NeedProfileImage(); - profile_image_download_reason_ = reason; - profile_image_load_start_time_ = base::TimeTicks::Now(); - profile_downloader_.reset(new ProfileDownloader(this)); - profile_downloader_->Start(); } // static @@ -708,6 +741,10 @@ void UserImageManagerImpl::IgnoreProfileDataDownloadDelayForTesting() { g_ignore_profile_data_download_delay_ = true; } +void UserImageManagerImpl::StopPolicyObserverForTesting() { + policy_observer_.reset(); +} + bool UserImageManagerImpl::NeedsProfilePicture() const { return downloading_profile_image_; } @@ -716,14 +753,14 @@ int UserImageManagerImpl::GetDesiredImageSideLength() const { return GetCurrentUserImageSize(); } -std::string UserImageManagerImpl::GetCachedPictureURL() const { - return profile_image_url_.spec(); -} - Profile* UserImageManagerImpl::GetBrowserProfile() { return ProfileManager::GetDefaultProfile(); } +std::string UserImageManagerImpl::GetCachedPictureURL() const { + return profile_image_url_.spec(); +} + void UserImageManagerImpl::OnProfileDownloadSuccess( ProfileDownloader* downloader) { // Ensure that the |profile_downloader_| is deleted when this method returns. @@ -731,15 +768,14 @@ void UserImageManagerImpl::OnProfileDownloadSuccess( profile_downloader_.release()); DCHECK_EQ(downloader, profile_downloader.get()); - UserManager* user_manager = UserManager::Get(); - const User* user = user_manager->GetLoggedInUser(); + const User* user = user_manager_->GetLoggedInUser(); const std::string& user_id = user->email(); - user_manager->UpdateUserAccountData(user_id, - UserManager::UserAccountData( - downloader->GetProfileFullName(), - downloader->GetProfileGivenName(), - downloader->GetProfileLocale())); + user_manager_->UpdateUserAccountData( + user_id, + UserManager::UserAccountData(downloader->GetProfileFullName(), + downloader->GetProfileGivenName(), + downloader->GetProfileLocale())); if (!downloading_profile_image_) return; @@ -836,6 +872,60 @@ void UserImageManagerImpl::OnProfileDownloadFailure( content::NotificationService::NoDetails()); } +bool UserImageManagerImpl::IsUserImageManaged( + const std::string& user_id) const { + return ContainsKey(users_with_managed_images_, user_id); +} + +void UserImageManagerImpl::SetInitialUserImage(const std::string& user_id) { + // Choose a random default image. + SaveUserDefaultImageIndex(user_id, + base::RandInt(kFirstDefaultImageIndex, + kDefaultImagesCount - 1)); +} + +void UserImageManagerImpl::TryToInitDownloadedProfileImage() { + const User* user = user_manager_->GetLoggedInUser(); + if (user->image_index() == User::kProfileImageIndex && + downloaded_profile_image_.isNull() && + !user->image_is_stub()) { + // Initialize the |downloaded_profile_image_| for the currently logged-in + // user if it has not been initialized already, the user image is the + // profile image and the user image has been loaded successfully. + VLOG(1) << "Profile image initialized from disk."; + downloaded_profile_image_ = user->image(); + profile_image_url_ = user->image_url(); + } +} + +bool UserImageManagerImpl::NeedProfileImage() const { + return user_manager_->IsLoggedInAsRegularUser() && + (user_manager_->GetLoggedInUser()->image_index() == + User::kProfileImageIndex || + profile_image_requested_); +} + +void UserImageManagerImpl::DownloadProfileData(const std::string& reason) { + // GAIA profiles exist for regular users only. + if (!user_manager_->IsLoggedInAsRegularUser()) + return; + + // If a download is already in progress, allow it to continue, with one + // exception: If the current download does not include the profile image but + // the image has since become necessary, start a new download that includes + // the profile image. + if (profile_downloader_ && + (downloading_profile_image_ || !NeedProfileImage())) { + return; + } + + downloading_profile_image_ = NeedProfileImage(); + profile_image_download_reason_ = reason; + profile_image_load_start_time_ = base::TimeTicks::Now(); + profile_downloader_.reset(new ProfileDownloader(this)); + profile_downloader_->Start(); +} + void UserImageManagerImpl::DeleteUserImageAndLocalStateEntry( const std::string& user_id, const char* prefs_dict_root) { @@ -858,7 +948,7 @@ void UserImageManagerImpl::DeleteUserImageAndLocalStateEntry( } void UserImageManagerImpl::OnJobChangedUserImage(const User* user) { - if (user == UserManager::Get()->GetLoggedInUser()) + if (user == user_manager_->GetLoggedInUser()) TryToInitDownloadedProfileImage(); content::NotificationService::current()->Notify( @@ -878,7 +968,7 @@ void UserImageManagerImpl::OnJobDone(const std::string& user_id) { NOTREACHED(); } - if (users_to_migrate_.find(user_id) == users_to_migrate_.end()) + if (!ContainsKey(users_to_migrate_, user_id)) return; // Migration completed for |user_id|. users_to_migrate_.erase(user_id); @@ -927,4 +1017,17 @@ void UserImageManagerImpl::UpdateLocalStateAfterMigration( update->RemoveWithoutPathExpansion(user_id, NULL); } +void UserImageManagerImpl::TryToCreateImageSyncObserver() { + const User* user = user_manager_->GetLoggedInUser(); + // If the currently logged-in user's user image is managed, the sync observer + // must not be started so that the policy-set image does not get synced out. + if (!user_image_sync_observer_ && + user && user->CanSyncImage() && + !IsUserImageManaged(user->email()) && + !CommandLine::ForCurrentProcess()->HasSwitch( + chromeos::switches::kDisableUserImageSync)) { + user_image_sync_observer_.reset(new UserImageSyncObserver(user)); + } +} + } // namespace chromeos diff --git a/chrome/browser/chromeos/login/user_image_manager_impl.h b/chrome/browser/chromeos/login/user_image_manager_impl.h index 68f2197..e587151 100644 --- a/chrome/browser/chromeos/login/user_image_manager_impl.h +++ b/chrome/browser/chromeos/login/user_image_manager_impl.h @@ -10,6 +10,7 @@ #include <string> #include "base/basictypes.h" +#include "base/compiler_specific.h" #include "base/memory/linked_ptr.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" @@ -20,6 +21,7 @@ #include "chrome/browser/chromeos/login/user.h" #include "chrome/browser/chromeos/login/user_image_loader.h" #include "chrome/browser/chromeos/login/user_image_manager.h" +#include "chrome/browser/chromeos/policy/cloud_external_data_policy_observer.h" #include "chrome/browser/profiles/profile_downloader_delegate.h" #include "ui/gfx/image/image_skia.h" @@ -32,15 +34,17 @@ class SequencedTaskRunner; } namespace chromeos { -class UserImageSyncObserver; -} -namespace chromeos { +class CrosSettings; +class UserImageSyncObserver; +class UserManager; -class UserImageManagerImpl : public UserImageManager, - public ProfileDownloaderDelegate { +class UserImageManagerImpl + : public UserImageManager, + public ProfileDownloaderDelegate, + public policy::CloudExternalDataPolicyObserver::Delegate { public: - UserImageManagerImpl(); + UserImageManagerImpl(CrosSettings* cros_settings, UserManager* user_manager); // UserImageManager: virtual ~UserImageManagerImpl(); @@ -62,7 +66,17 @@ class UserImageManagerImpl : public UserImageManager, virtual UserImageSyncObserver* GetSyncObserver() const OVERRIDE; virtual void Shutdown() OVERRIDE; + // policy::CloudExternalDataPolicyObserver::Delegate: + virtual void OnExternalDataSet(const std::string& policy, + const std::string& user_id) OVERRIDE; + virtual void OnExternalDataCleared(const std::string& policy, + const std::string& user_id) OVERRIDE; + virtual void OnExternalDataFetched(const std::string& policy, + const std::string& user_id, + scoped_ptr<std::string> data) OVERRIDE; + static void IgnoreProfileDataDownloadDelayForTesting(); + void StopPolicyObserverForTesting(); private: friend class UserImageManagerTest; @@ -90,6 +104,10 @@ class UserImageManagerImpl : public UserImageManager, ProfileDownloader* downloader, ProfileDownloaderDelegate::FailureReason reason) OVERRIDE; + // Returns true if the user image for |user_id| is managed by policy and the + // user is not allowed to change it. + bool IsUserImageManaged(const std::string& user_id) const; + // Randomly chooses one of the default images for the specified user, sends a // LOGIN_USER_IMAGE_CHANGED notification and updates local state. void SetInitialUserImage(const std::string& user_id); @@ -130,6 +148,13 @@ class UserImageManagerImpl : public UserImageManager, // Completes migration by removing |user_id| from the old prefs dictionary. void UpdateLocalStateAfterMigration(const std::string& user_id); + // Create a sync observer if a user is logged in, the user's user image is + // allowed to be synced and no sync observer exists yet. + void TryToCreateImageSyncObserver(); + + // The user manager. + UserManager* user_manager_; + // Loader for JPEG user images. scoped_refptr<UserImageLoader> image_loader_; @@ -186,6 +211,9 @@ class UserImageManagerImpl : public UserImageManager, // Sync observer for the currently logged-in user. scoped_ptr<UserImageSyncObserver> user_image_sync_observer_; + // Observer for the policy that can be used to manage user images. + scoped_ptr<policy::CloudExternalDataPolicyObserver> policy_observer_; + // Background task runner on which Jobs perform file I/O and the image // decoders run. scoped_refptr<base::SequencedTaskRunner> background_task_runner_; @@ -193,6 +221,9 @@ class UserImageManagerImpl : public UserImageManager, // The currently running jobs. std::map<std::string, linked_ptr<Job> > jobs_; + // List of user_ids whose user image is managed by policy. + std::set<std::string> users_with_managed_images_; + base::WeakPtrFactory<UserImageManagerImpl> weak_factory_; DISALLOW_COPY_AND_ASSIGN(UserImageManagerImpl); diff --git a/chrome/browser/chromeos/login/user_image_manager_test_util.cc b/chrome/browser/chromeos/login/user_image_manager_test_util.cc index 01e9163..ce64d20 100644 --- a/chrome/browser/chromeos/login/user_image_manager_test_util.cc +++ b/chrome/browser/chromeos/login/user_image_manager_test_util.cc @@ -16,6 +16,9 @@ namespace chromeos { namespace test { +const char kUserAvatarImage1RelativePath[] = "chromeos/avatar1.jpg"; +const char kUserAvatarImage2RelativePath[] = "chromeos/avatar2.jpg"; + bool AreImagesEqual(const gfx::ImageSkia& first, const gfx::ImageSkia& second) { if (first.width() != second.width() || first.height() != second.height()) return false; diff --git a/chrome/browser/chromeos/login/user_image_manager_test_util.h b/chrome/browser/chromeos/login/user_image_manager_test_util.h index 04b6f39..dc08c8c 100644 --- a/chrome/browser/chromeos/login/user_image_manager_test_util.h +++ b/chrome/browser/chromeos/login/user_image_manager_test_util.h @@ -25,6 +25,9 @@ class ImageSkia; namespace chromeos { namespace test { +extern const char kUserAvatarImage1RelativePath[]; +extern const char kUserAvatarImage2RelativePath[]; + // Returns |true| if the two given images are pixel-for-pixel identical. bool AreImagesEqual(const gfx::ImageSkia& first, const gfx::ImageSkia& second); diff --git a/chrome/browser/chromeos/login/user_manager_impl.cc b/chrome/browser/chromeos/login/user_manager_impl.cc index 1aecef1..f7af2d3 100644 --- a/chrome/browser/chromeos/login/user_manager_impl.cc +++ b/chrome/browser/chromeos/login/user_manager_impl.cc @@ -191,7 +191,7 @@ UserManagerImpl::UserManagerImpl() is_current_user_new_(false), is_current_user_ephemeral_regular_user_(false), ephemeral_users_enabled_(false), - user_image_manager_(new UserImageManagerImpl), + user_image_manager_(new UserImageManagerImpl(cros_settings_, this)), supervised_user_manager_(new SupervisedUserManagerImpl(this)), manager_creation_time_(base::TimeTicks::Now()), multi_profile_first_run_notification_( diff --git a/chrome/browser/chromeos/policy/cloud_external_data_policy_observer.cc b/chrome/browser/chromeos/policy/cloud_external_data_policy_observer.cc index 9977b5b..12b974c 100644 --- a/chrome/browser/chromeos/policy/cloud_external_data_policy_observer.cc +++ b/chrome/browser/chromeos/policy/cloud_external_data_policy_observer.cc @@ -206,7 +206,12 @@ void CloudExternalDataPolicyObserver::OnPolicyUpdated( DeviceLocalAccountPolicyBroker* broker = device_local_account_policy_service_->GetBrokerForUser(user_id); if (!broker) { - NOTREACHED(); + // The order in which |this| and the |device_local_account_policy_service_| + // find out that a new device-local account has been added is undefined. If + // no |broker| exists yet, the |device_local_account_policy_service_| must + // not have seen the new |user_id| yet. OnPolicyUpdated() will be invoked + // again by the |device_local_account_policy_service_| in this case when it + // finds out about |user_id| and creates a |broker| for it. return; } @@ -306,7 +311,6 @@ void CloudExternalDataPolicyObserver::OnExternalDataFetched( FetchWeakPtrMap::iterator it = fetch_weak_ptrs_.find(user_id); DCHECK(it != fetch_weak_ptrs_.end()); fetch_weak_ptrs_.erase(it); - DCHECK(data); delegate_->OnExternalDataFetched(policy_, user_id, data.Pass()); } diff --git a/chrome/browser/chromeos/policy/cloud_external_data_policy_observer.h b/chrome/browser/chromeos/policy/cloud_external_data_policy_observer.h index 7f523fb..00f2882 100644 --- a/chrome/browser/chromeos/policy/cloud_external_data_policy_observer.h +++ b/chrome/browser/chromeos/policy/cloud_external_data_policy_observer.h @@ -76,7 +76,7 @@ class CloudExternalDataPolicyObserver const content::NotificationSource& source, const content::NotificationDetails& details) OVERRIDE; - // DeviceLocalAccountPolicyService: + // DeviceLocalAccountPolicyService::Observer: virtual void OnPolicyUpdated(const std::string& user_id) OVERRIDE; virtual void OnDeviceLocalAccountsChanged() OVERRIDE; diff --git a/chrome/browser/chromeos/policy/cloud_external_data_policy_observer_unittest.cc b/chrome/browser/chromeos/policy/cloud_external_data_policy_observer_unittest.cc index fed62e4..c7edbde 100644 --- a/chrome/browser/chromeos/policy/cloud_external_data_policy_observer_unittest.cc +++ b/chrome/browser/chromeos/policy/cloud_external_data_policy_observer_unittest.cc @@ -15,11 +15,10 @@ #include "base/message_loop/message_loop_proxy.h" #include "base/path_service.h" #include "base/run_loop.h" -#include "base/sha1.h" -#include "base/strings/string_number_conversions.h" #include "base/values.h" #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/chromeos/login/fake_user_manager.h" +#include "chrome/browser/chromeos/policy/cloud_external_data_manager_base_test_util.h" #include "chrome/browser/chromeos/policy/device_local_account.h" #include "chrome/browser/chromeos/policy/device_local_account_external_data_manager.h" #include "chrome/browser/chromeos/policy/device_local_account_policy_provider.h" @@ -80,12 +79,9 @@ void ConstructAvatarPolicy(const std::string& file_name, ASSERT_TRUE(base::ReadFileToString( test_data_dir.Append("chromeos").Append(file_name), policy_data)); - const std::string sha1 = base::SHA1HashString(*policy_data); - - base::DictionaryValue dict; - dict.SetString("url", url); - dict.SetString("hash", base::HexEncode(sha1.data(), sha1.size())); - base::JSONWriter::Write(&dict, policy); + base::JSONWriter::Write( + test::ConstructExternalDataReference(url, *policy_data).get(), + policy); } } // namespace diff --git a/chrome/browser/chromeos/policy/configuration_policy_handler_chromeos.cc b/chrome/browser/chromeos/policy/configuration_policy_handler_chromeos.cc index b9043e1..611ea7a 100644 --- a/chrome/browser/chromeos/policy/configuration_policy_handler_chromeos.cc +++ b/chrome/browser/chromeos/policy/configuration_policy_handler_chromeos.cc @@ -5,13 +5,17 @@ #include "chrome/browser/chromeos/policy/configuration_policy_handler_chromeos.h" #include <string> +#include <vector> #include "ash/magnifier/magnifier_constants.h" #include "base/callback.h" #include "base/json/json_reader.h" #include "base/json/json_writer.h" +#include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "base/prefs/pref_value_map.h" +#include "base/sha1.h" +#include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" #include "base/values.h" #include "chrome/browser/chromeos/policy/login_screen_power_management_policy.h" @@ -27,9 +31,90 @@ #include "components/policy/core/common/policy_map.h" #include "grit/component_strings.h" #include "policy/policy_constants.h" +#include "url/gurl.h" namespace policy { +namespace { + +const char kSubkeyURL[] = "url"; +const char kSubkeyHash[] = "hash"; + +bool GetSubkeyString(const base::DictionaryValue& dict, + policy::PolicyErrorMap* errors, + const std::string& policy, + const std::string& subkey, + std::string* value) { + const base::Value* raw_value = NULL; + if (!dict.GetWithoutPathExpansion(subkey, &raw_value)) { + errors->AddError(policy, subkey, IDS_POLICY_NOT_SPECIFIED_ERROR); + return false; + } + std::string string_value; + if (!raw_value->GetAsString(&string_value)) { + errors->AddError(policy, subkey, IDS_POLICY_TYPE_ERROR, "string"); + return false; + } + if (string_value.empty()) { + errors->AddError(policy, subkey, IDS_POLICY_NOT_SPECIFIED_ERROR); + return false; + } + *value = string_value; + return true; +} + +} // namespace + +ExternalDataPolicyHandler::ExternalDataPolicyHandler(const char* policy_name) + : TypeCheckingPolicyHandler(policy_name, Value::TYPE_DICTIONARY) { +} + +ExternalDataPolicyHandler::~ExternalDataPolicyHandler() { +} + +bool ExternalDataPolicyHandler::CheckPolicySettings(const PolicyMap& policies, + PolicyErrorMap* errors) { + if (!TypeCheckingPolicyHandler::CheckPolicySettings(policies, errors)) + return false; + + const std::string policy = policy_name(); + const base::Value* value = policies.GetValue(policy); + if (!value) + return true; + + const DictionaryValue* dict = NULL; + value->GetAsDictionary(&dict); + if (!dict) { + NOTREACHED(); + return false; + } + std::string url_string; + std::string hash_string; + if (!GetSubkeyString(*dict, errors, policy, kSubkeyURL, &url_string) || + !GetSubkeyString(*dict, errors, policy, kSubkeyHash, &hash_string)) { + return false; + } + + const GURL url(url_string); + if (!url.is_valid()) { + errors->AddError(policy, kSubkeyURL, IDS_POLICY_VALUE_FORMAT_ERROR); + return false; + } + + std::vector<uint8> hash; + if (!base::HexStringToBytes(hash_string, &hash) || + hash.size() != base::kSHA1Length) { + errors->AddError(policy, kSubkeyHash, IDS_POLICY_VALUE_FORMAT_ERROR); + return false; + } + + return true; +} + +void ExternalDataPolicyHandler::ApplyPolicySettings(const PolicyMap& policies, + PrefValueMap* prefs) { +} + // static NetworkConfigurationPolicyHandler* NetworkConfigurationPolicyHandler::CreateForUserPolicy() { diff --git a/chrome/browser/chromeos/policy/configuration_policy_handler_chromeos.h b/chrome/browser/chromeos/policy/configuration_policy_handler_chromeos.h index d1b751d..a756a83 100644 --- a/chrome/browser/chromeos/policy/configuration_policy_handler_chromeos.h +++ b/chrome/browser/chromeos/policy/configuration_policy_handler_chromeos.h @@ -5,6 +5,8 @@ #ifndef CHROME_BROWSER_CHROMEOS_POLICY_CONFIGURATION_POLICY_HANDLER_CHROMEOS_H_ #define CHROME_BROWSER_CHROMEOS_POLICY_CONFIGURATION_POLICY_HANDLER_CHROMEOS_H_ +#include "base/basictypes.h" +#include "base/compiler_specific.h" #include "chrome/browser/extensions/policy_handlers.h" #include "chrome/browser/policy/configuration_policy_handler.h" #include "chromeos/network/network_ui_data.h" @@ -17,6 +19,23 @@ class Value; namespace policy { +// ConfigurationPolicyHandler for policies referencing external data. +class ExternalDataPolicyHandler : public TypeCheckingPolicyHandler { + public: + explicit ExternalDataPolicyHandler(const char* policy_name); + virtual ~ExternalDataPolicyHandler(); + + // TypeCheckingPolicyHandler: + virtual bool CheckPolicySettings(const PolicyMap& policies, + PolicyErrorMap* errors) OVERRIDE; + + virtual void ApplyPolicySettings(const PolicyMap& policies, + PrefValueMap* prefs) OVERRIDE; + + private: + DISALLOW_COPY_AND_ASSIGN(ExternalDataPolicyHandler); +}; + // ConfigurationPolicyHandler for validation of the network configuration // policies. These actually don't set any preferences, but the handler just // generates error messages. diff --git a/chrome/browser/chromeos/policy/configuration_policy_handler_chromeos_unittest.cc b/chrome/browser/chromeos/policy/configuration_policy_handler_chromeos_unittest.cc index 21cd13d..f33c850 100644 --- a/chrome/browser/chromeos/policy/configuration_policy_handler_chromeos_unittest.cc +++ b/chrome/browser/chromeos/policy/configuration_policy_handler_chromeos_unittest.cc @@ -5,6 +5,7 @@ #include "chrome/browser/chromeos/policy/configuration_policy_handler_chromeos.h" #include "base/callback.h" +#include "base/memory/scoped_ptr.h" #include "base/prefs/pref_value_map.h" #include "base/values.h" #include "chrome/browser/ui/ash/chrome_launcher_prefs.h" @@ -71,6 +72,104 @@ TEST_F(ScreenMagnifierPolicyHandlerTest, Enabled) { EXPECT_TRUE(base::FundamentalValue(1).Equals(type)); } +TEST(ExternalDataPolicyHandlerTest, Empty) { + PolicyErrorMap errors; + EXPECT_TRUE(ExternalDataPolicyHandler(key::kUserAvatarImage) + .CheckPolicySettings(PolicyMap(), &errors)); + EXPECT_TRUE(errors.GetErrors(key::kUserAvatarImage).empty()); +} + +TEST(ExternalDataPolicyHandlerTest, WrongType) { + PolicyMap policy_map; + policy_map.Set(key::kUserAvatarImage, + POLICY_LEVEL_MANDATORY, + POLICY_SCOPE_USER, + new base::FundamentalValue(false), + NULL); + PolicyErrorMap errors; + EXPECT_FALSE(ExternalDataPolicyHandler(key::kUserAvatarImage) + .CheckPolicySettings(policy_map, &errors)); + EXPECT_FALSE(errors.GetErrors(key::kUserAvatarImage).empty()); +} + +TEST(ExternalDataPolicyHandlerTest, MissingURL) { + scoped_ptr<base::DictionaryValue> dict(new base::DictionaryValue); + dict->SetString("hash", "1234567890123456789012345678901234567890"); + PolicyMap policy_map; + policy_map.Set(key::kUserAvatarImage, + POLICY_LEVEL_MANDATORY, + POLICY_SCOPE_USER, + dict.release(), + NULL); + PolicyErrorMap errors; + EXPECT_FALSE(ExternalDataPolicyHandler(key::kUserAvatarImage) + .CheckPolicySettings(policy_map, &errors)); + EXPECT_FALSE(errors.GetErrors(key::kUserAvatarImage).empty()); +} + +TEST(ExternalDataPolicyHandlerTest, InvalidURL) { + scoped_ptr<base::DictionaryValue> dict(new base::DictionaryValue); + dict->SetString("url", "http://"); + dict->SetString("hash", "1234567890123456789012345678901234567890"); + PolicyMap policy_map; + policy_map.Set(key::kUserAvatarImage, + POLICY_LEVEL_MANDATORY, + POLICY_SCOPE_USER, + dict.release(), + NULL); + PolicyErrorMap errors; + EXPECT_FALSE(ExternalDataPolicyHandler(key::kUserAvatarImage) + .CheckPolicySettings(policy_map, &errors)); + EXPECT_FALSE(errors.GetErrors(key::kUserAvatarImage).empty()); +} + +TEST(ExternalDataPolicyHandlerTest, MissingHash) { + scoped_ptr<base::DictionaryValue> dict(new base::DictionaryValue); + dict->SetString("url", "http://localhost/"); + PolicyMap policy_map; + policy_map.Set(key::kUserAvatarImage, + POLICY_LEVEL_MANDATORY, + POLICY_SCOPE_USER, + dict.release(), + NULL); + PolicyErrorMap errors; + EXPECT_FALSE(ExternalDataPolicyHandler(key::kUserAvatarImage) + .CheckPolicySettings(policy_map, &errors)); + EXPECT_FALSE(errors.GetErrors(key::kUserAvatarImage).empty()); +} + +TEST(ExternalDataPolicyHandlerTest, InvalidHash) { + scoped_ptr<base::DictionaryValue> dict(new base::DictionaryValue); + dict->SetString("url", "http://localhost/"); + dict->SetString("hash", "1234"); + PolicyMap policy_map; + policy_map.Set(key::kUserAvatarImage, + POLICY_LEVEL_MANDATORY, + POLICY_SCOPE_USER, + dict.release(), + NULL); + PolicyErrorMap errors; + EXPECT_FALSE(ExternalDataPolicyHandler(key::kUserAvatarImage) + .CheckPolicySettings(policy_map, &errors)); + EXPECT_FALSE(errors.GetErrors(key::kUserAvatarImage).empty()); +} + +TEST(ExternalDataPolicyHandlerTest, Valid) { + scoped_ptr<base::DictionaryValue> dict(new base::DictionaryValue); + dict->SetString("url", "http://localhost/"); + dict->SetString("hash", "1234567890123456789012345678901234567890"); + PolicyMap policy_map; + policy_map.Set(key::kUserAvatarImage, + POLICY_LEVEL_MANDATORY, + POLICY_SCOPE_USER, + dict.release(), + NULL); + PolicyErrorMap errors; + EXPECT_TRUE(ExternalDataPolicyHandler(key::kUserAvatarImage) + .CheckPolicySettings(policy_map, &errors)); + EXPECT_TRUE(errors.GetErrors(key::kUserAvatarImage).empty()); +} + const char kLoginScreenPowerManagementPolicy[] = "{" " \"AC\": {" diff --git a/chrome/browser/chromeos/policy/device_local_account_browsertest.cc b/chrome/browser/chromeos/policy/device_local_account_browsertest.cc index a505536..43d1758 100644 --- a/chrome/browser/chromeos/policy/device_local_account_browsertest.cc +++ b/chrome/browser/chromeos/policy/device_local_account_browsertest.cc @@ -21,6 +21,7 @@ #include "base/message_loop/message_loop.h" #include "base/message_loop/message_loop_proxy.h" #include "base/path_service.h" +#include "base/prefs/pref_service.h" #include "base/run_loop.h" #include "base/sequenced_task_runner.h" #include "base/strings/string_number_conversions.h" @@ -37,6 +38,9 @@ #include "chrome/browser/chromeos/login/mock_login_status_consumer.h" #include "chrome/browser/chromeos/login/screens/wizard_screen.h" #include "chrome/browser/chromeos/login/user.h" +#include "chrome/browser/chromeos/login/user_image_manager.h" +#include "chrome/browser/chromeos/login/user_image_manager_impl.h" +#include "chrome/browser/chromeos/login/user_image_manager_test_util.h" #include "chrome/browser/chromeos/login/user_manager.h" #include "chrome/browser/chromeos/login/webui_login_view.h" #include "chrome/browser/chromeos/login/wizard_controller.h" @@ -100,6 +104,7 @@ #include "testing/gmock/include/gmock/gmock.h" //#include "third_party/cros_system_api/dbus/service_constants.h" #include "ui/base/l10n/l10n_util.h" +#include "ui/gfx/image/image_skia.h" #include "url/gurl.h" namespace em = enterprise_management; @@ -265,7 +270,8 @@ scoped_ptr<net::FakeURLFetcher> RunCallbackAndReturnFakeURLFetcher( } // namespace -class DeviceLocalAccountTest : public DevicePolicyCrosBrowserTest { +class DeviceLocalAccountTest : public DevicePolicyCrosBrowserTest, + public chromeos::UserManager::Observer { protected: DeviceLocalAccountTest() : user_id_1_(GenerateDeviceLocalAccountUserId( @@ -329,6 +335,10 @@ class DeviceLocalAccountTest : public DevicePolicyCrosBrowserTest { base::RunLoop().RunUntilIdle(); } + virtual void LocalStateChanged(chromeos::UserManager* user_manager) OVERRIDE { + run_loop_->Quit(); + } + void InitializePolicy() { device_policy()->policy_data().set_public_key_version(1); em::ChromeDeviceSettingsProto& proto(device_policy()->payload()); @@ -403,6 +413,8 @@ class DeviceLocalAccountTest : public DevicePolicyCrosBrowserTest { UserPolicyBuilder device_local_account_policy_; LocalPolicyTestServer test_server_; + + scoped_ptr<base::RunLoop> run_loop_; }; static bool IsKnownUser(const std::string& account_id) { @@ -808,6 +820,15 @@ IN_PROC_BROWSER_TEST_F(DeviceLocalAccountTest, ExtensionsCached) { } IN_PROC_BROWSER_TEST_F(DeviceLocalAccountTest, ExternalData) { + // chromeos::UserImageManagerImpl requests an external data fetch whenever the + // key::kUserAvatarImage policy is set. Since this test wants to verify that + // the underlying policy subsystem will start a fetch without this request as + // well, the chromeos::UserImageManagerImpl must be prevented from seeing the + // policy change. + reinterpret_cast<chromeos::UserImageManagerImpl*>( + chromeos::UserManager::Get()->GetUserImageManager())-> + StopPolicyObserverForTesting(); + UploadDeviceLocalAccountPolicy(); AddPublicSessionToDevicePolicy(kAccountId1); @@ -924,6 +945,84 @@ IN_PROC_BROWSER_TEST_F(DeviceLocalAccountTest, ExternalData) { EXPECT_EQ(kExternalData, *fetched_external_data); } +IN_PROC_BROWSER_TEST_F(DeviceLocalAccountTest, UserAvatarImage) { + ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady()); + + UploadDeviceLocalAccountPolicy(); + AddPublicSessionToDevicePolicy(kAccountId1); + + // This observes the display name becoming available as this indicates + // device-local account policy is fully loaded. + content::WindowedNotificationObserver( + chrome::NOTIFICATION_USER_LIST_CHANGED, + base::Bind(&DisplayNameMatches, user_id_1_, kDisplayName)).Wait(); + + base::FilePath test_dir; + ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_dir)); + std::string image_data; + ASSERT_TRUE(base::ReadFileToString( + test_dir.Append(chromeos::test::kUserAvatarImage1RelativePath), + &image_data)); + + std::string policy; + base::JSONWriter::Write(test::ConstructExternalDataReference( + embedded_test_server()->GetURL(std::string("/") + + chromeos::test::kUserAvatarImage1RelativePath).spec(), + image_data).get(), + &policy); + device_local_account_policy_.payload().mutable_useravatarimage()->set_value( + policy); + UploadAndInstallDeviceLocalAccountPolicy(); + DeviceLocalAccountPolicyBroker* broker = + g_browser_process->browser_policy_connector()-> + GetDeviceLocalAccountPolicyService()->GetBrokerForUser(user_id_1_); + ASSERT_TRUE(broker); + + run_loop_.reset(new base::RunLoop); + chromeos::UserManager::Get()->AddObserver(this); + broker->core()->store()->Load(); + run_loop_->Run(); + chromeos::UserManager::Get()->RemoveObserver(this); + + scoped_ptr<gfx::ImageSkia> policy_image = chromeos::test::ImageLoader( + test_dir.Append(chromeos::test::kUserAvatarImage1RelativePath)).Load(); + ASSERT_TRUE(policy_image); + + const chromeos::User* user = + chromeos::UserManager::Get()->FindUser(user_id_1_); + ASSERT_TRUE(user); + + base::FilePath user_data_dir; + ASSERT_TRUE(PathService::Get(chrome::DIR_USER_DATA, &user_data_dir)); + const base::FilePath saved_image_path = + user_data_dir.Append(user_id_1_).AddExtension("jpg"); + + EXPECT_FALSE(user->HasDefaultImage()); + EXPECT_EQ(chromeos::User::kExternalImageIndex, user->image_index()); + EXPECT_TRUE(chromeos::test::AreImagesEqual(*policy_image, user->image())); + const base::DictionaryValue* images_pref = + g_browser_process->local_state()->GetDictionary("user_image_info"); + ASSERT_TRUE(images_pref); + const base::DictionaryValue* image_properties; + ASSERT_TRUE(images_pref->GetDictionaryWithoutPathExpansion( + user_id_1_, + &image_properties)); + int image_index; + std::string image_path; + ASSERT_TRUE(image_properties->GetInteger("index", &image_index)); + ASSERT_TRUE(image_properties->GetString("path", &image_path)); + EXPECT_EQ(chromeos::User::kExternalImageIndex, image_index); + EXPECT_EQ(saved_image_path.value(), image_path); + + scoped_ptr<gfx::ImageSkia> saved_image = + chromeos::test::ImageLoader(saved_image_path).Load(); + ASSERT_TRUE(saved_image); + + // Check image dimensions. Images can't be compared since JPEG is lossy. + EXPECT_EQ(policy_image->width(), saved_image->width()); + EXPECT_EQ(policy_image->height(), saved_image->height()); +} + class TermsOfServiceTest : public DeviceLocalAccountTest, public testing::WithParamInterface<bool> { }; diff --git a/chrome/browser/policy/configuration_policy_handler_list_factory.cc b/chrome/browser/policy/configuration_policy_handler_list_factory.cc index c88c01b..4b83931 100644 --- a/chrome/browser/policy/configuration_policy_handler_list_factory.cc +++ b/chrome/browser/policy/configuration_policy_handler_list_factory.cc @@ -643,6 +643,8 @@ scoped_ptr<ConfigurationPolicyHandlerList> BuildHandlerList() { 0, ash::MAGNIFIER_FULL, false))); + handlers->AddHandler(make_scoped_ptr<ConfigurationPolicyHandler>( + new ExternalDataPolicyHandler(key::kUserAvatarImage))); #endif // defined(OS_CHROMEOS) #if defined(OS_ANDROID) diff --git a/chrome/browser/policy/policy_prefs_browsertest.cc b/chrome/browser/policy/policy_prefs_browsertest.cc index 98241ce..2c4bfa2 100644 --- a/chrome/browser/policy/policy_prefs_browsertest.cc +++ b/chrome/browser/policy/policy_prefs_browsertest.cc @@ -16,6 +16,7 @@ #include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "base/memory/scoped_vector.h" +#include "base/memory/weak_ptr.h" #include "base/prefs/pref_service.h" #include "base/run_loop.h" #include "base/stl_util.h" @@ -30,7 +31,10 @@ #include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/test/base/in_process_browser_test.h" #include "chrome/test/base/ui_test_utils.h" +#include "components/policy/core/common/external_data_fetcher.h" +#include "components/policy/core/common/external_data_manager.h" #include "components/policy/core/common/mock_configuration_policy_provider.h" +#include "components/policy/core/common/policy_details.h" #include "components/policy/core/common/policy_map.h" #include "components/policy/core/common/schema.h" #include "content/public/browser/web_contents.h" @@ -126,10 +130,12 @@ class PolicyTestCase { public: PolicyTestCase(const std::string& name, bool is_official_only, - bool can_be_recommended) + bool can_be_recommended, + const std::string& indicator_selector) : name_(name), is_official_only_(is_official_only), - can_be_recommended_(can_be_recommended) {} + can_be_recommended_(can_be_recommended), + indicator_selector_(indicator_selector) {} ~PolicyTestCase() {} const std::string& name() const { return name_; } @@ -163,9 +169,10 @@ class PolicyTestCase { return IsOsSupported(); } - const PolicyMap& test_policy() const { return test_policy_; } - void SetTestPolicy(const PolicyMap& policy) { - test_policy_.CopyFrom(policy); + const base::DictionaryValue& test_policy() const { return test_policy_; } + void SetTestPolicy(const base::DictionaryValue& policy) { + test_policy_.Clear(); + test_policy_.MergeDictionary(&policy); } const ScopedVector<PrefMapping>& pref_mappings() const { @@ -175,13 +182,16 @@ class PolicyTestCase { pref_mappings_.push_back(pref_mapping); } + const std::string& indicator_selector() const { return indicator_selector_; } + private: std::string name_; bool is_official_only_; bool can_be_recommended_; std::vector<std::string> supported_os_; - PolicyMap test_policy_; + base::DictionaryValue test_policy_; ScopedVector<PrefMapping> pref_mappings_; + std::string indicator_selector_; DISALLOW_COPY_AND_ASSIGN(PolicyTestCase); }; @@ -246,8 +256,12 @@ class PolicyTestCases { policy_test_dict->GetBoolean("official_only", &is_official_only); bool can_be_recommended = false; policy_test_dict->GetBoolean("can_be_recommended", &can_be_recommended); - PolicyTestCase* policy_test_case = - new PolicyTestCase(name, is_official_only, can_be_recommended); + std::string indicator_selector; + policy_test_dict->GetString("indicator_selector", &indicator_selector); + PolicyTestCase* policy_test_case = new PolicyTestCase(name, + is_official_only, + can_be_recommended, + indicator_selector); const base::ListValue* os_list = NULL; if (policy_test_dict->GetList("os", &os_list)) { for (size_t i = 0; i < os_list->GetSize(); ++i) { @@ -256,12 +270,9 @@ class PolicyTestCases { policy_test_case->AddSupportedOs(os); } } - const base::DictionaryValue* policy_dict = NULL; - if (policy_test_dict->GetDictionary("test_policy", &policy_dict)) { - PolicyMap policy; - policy.LoadFrom(policy_dict, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER); - policy_test_case->SetTestPolicy(policy); - } + const base::DictionaryValue* policy = NULL; + if (policy_test_dict->GetDictionary("test_policy", &policy)) + policy_test_case->SetTestPolicy(*policy); const base::ListValue* pref_mappings = NULL; if (policy_test_dict->GetList("pref_mappings", &pref_mappings)) { for (size_t i = 0; i < pref_mappings->GetSize(); ++i) { @@ -449,8 +460,28 @@ class PolicyPrefsTest : public InProcessBrowserTest { TemplateURLServiceFactory::GetForProfile(browser()->profile())); } - void UpdateProviderPolicy(const PolicyMap& policy) { - provider_.UpdateChromePolicy(policy); + void ClearProviderPolicy() { + provider_.UpdateChromePolicy(PolicyMap()); + base::RunLoop().RunUntilIdle(); + } + + void SetProviderPolicy(const base::DictionaryValue& policies, + PolicyLevel level) { + PolicyMap policy_map; + for (DictionaryValue::Iterator it(policies); !it.IsAtEnd(); it.Advance()) { + const PolicyDetails* policy_details = GetChromePolicyDetails(it.key()); + ASSERT_TRUE(policy_details); + policy_map.Set( + it.key(), + level, + POLICY_SCOPE_USER, + it.value().DeepCopy(), + policy_details->max_external_data_size ? + new ExternalDataFetcher(base::WeakPtr<ExternalDataManager>(), + it.key()) : + NULL); + } + provider_.UpdateChromePolicy(policy_map); base::RunLoop().RunUntilIdle(); } @@ -490,14 +521,14 @@ IN_PROC_BROWSER_TEST_F(PolicyPrefsTest, PolicyToPrefsMapping) { ASSERT_TRUE(pref); // Verify that setting the policy overrides the pref. - UpdateProviderPolicy(PolicyMap()); + ClearProviderPolicy(); prefs->ClearPref((*pref_mapping)->pref().c_str()); EXPECT_TRUE(pref->IsDefaultValue()); EXPECT_TRUE(pref->IsUserModifiable()); EXPECT_FALSE(pref->IsUserControlled()); EXPECT_FALSE(pref->IsManaged()); - UpdateProviderPolicy(it->second->test_policy()); + SetProviderPolicy(it->second->test_policy(), POLICY_LEVEL_MANDATORY); EXPECT_FALSE(pref->IsDefaultValue()); EXPECT_FALSE(pref->IsUserModifiable()); EXPECT_FALSE(pref->IsUserControlled()); @@ -524,25 +555,47 @@ IN_PROC_BROWSER_TEST_P(PolicyPrefIndicatorTest, CheckPolicyIndicators) { it != GetParam().end(); ++it) { const PolicyTestCase* policy_test_case = test_cases.Get(*it); ASSERT_TRUE(policy_test_case) << "PolicyTestCase not found for " << *it; + if (!policy_test_case->IsSupported()) + continue; const ScopedVector<PrefMapping>& pref_mappings = policy_test_case->pref_mappings(); - if (!policy_test_case->IsSupported() || pref_mappings.empty()) - continue; - bool has_indicator_tests = false; - for (ScopedVector<PrefMapping>::const_iterator - pref_mapping = pref_mappings.begin(); - pref_mapping != pref_mappings.end(); - ++pref_mapping) { - if (!(*pref_mapping)->indicator_test_cases().empty()) { - has_indicator_tests = true; - break; + if (policy_test_case->indicator_selector().empty()) { + bool has_pref_indicator_tests = false; + for (ScopedVector<PrefMapping>::const_iterator + pref_mapping = pref_mappings.begin(); + pref_mapping != pref_mappings.end(); + ++pref_mapping) { + if (!(*pref_mapping)->indicator_test_cases().empty()) { + has_pref_indicator_tests = true; + break; + } } + if (!has_pref_indicator_tests) + continue; } - if (!has_indicator_tests) - continue; LOG(INFO) << "Testing policy: " << *it; + if (!policy_test_case->indicator_selector().empty()) { + // Check that no controlled setting indicator is visible when no value is + // set by policy. + ClearProviderPolicy(); + VerifyControlledSettingIndicators(browser(), + policy_test_case->indicator_selector(), + std::string(), + std::string(), + false); + // Check that the appropriate controlled setting indicator is shown when a + // value is enforced by policy. + SetProviderPolicy(policy_test_case->test_policy(), + POLICY_LEVEL_MANDATORY); + VerifyControlledSettingIndicators(browser(), + policy_test_case->indicator_selector(), + std::string(), + "policy", + false); + } + for (ScopedVector<PrefMapping>::const_iterator pref_mapping = pref_mappings.begin(); pref_mapping != pref_mappings.end(); @@ -567,15 +620,13 @@ IN_PROC_BROWSER_TEST_P(PolicyPrefIndicatorTest, CheckPolicyIndicators) { ++indicator_test_case) { // Check that no controlled setting indicator is visible when no value // is set by policy. - UpdateProviderPolicy(PolicyMap()); + ClearProviderPolicy(); VerifyControlledSettingIndicators( browser(), indicator_selector, std::string(), std::string(), false); // Check that the appropriate controlled setting indicator is shown when // a value is enforced by policy. - PolicyMap policies; - policies.LoadFrom(&(*indicator_test_case)->policy(), - POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER); - UpdateProviderPolicy(policies); + SetProviderPolicy((*indicator_test_case)->policy(), + POLICY_LEVEL_MANDATORY); VerifyControlledSettingIndicators(browser(), indicator_selector, (*indicator_test_case)->value(), "policy", @@ -594,9 +645,8 @@ IN_PROC_BROWSER_TEST_P(PolicyPrefIndicatorTest, CheckPolicyIndicators) { // Check that the appropriate controlled setting indicator is shown when // a value is recommended by policy and the user has not overridden the // recommendation. - policies.LoadFrom(&(*indicator_test_case)->policy(), - POLICY_LEVEL_RECOMMENDED, POLICY_SCOPE_USER); - UpdateProviderPolicy(policies); + SetProviderPolicy((*indicator_test_case)->policy(), + POLICY_LEVEL_RECOMMENDED); VerifyControlledSettingIndicators(browser(), indicator_selector, (*indicator_test_case)->value(), "recommended", diff --git a/chrome/browser/resources/options/browser_options.css b/chrome/browser/resources/options/browser_options.css index de61805..a3dcba1 100644 --- a/chrome/browser/resources/options/browser_options.css +++ b/chrome/browser/resources/options/browser_options.css @@ -24,40 +24,53 @@ } #account-picture-wrapper { - border: 1px solid rgba(0, 0, 0, 0.3); - border-radius: 4px; - cursor: pointer; - display: inline-block; float: left; margin: 0 2px 10px 0; - padding: 3px; - position: relative; } html[dir=rtl] #account-picture-wrapper { float: right; } +#account-picture-control { + border: 1px solid rgba(0, 0, 0, 0.3); + border-radius: 4px; + display: inline-block; + padding: 3px; + position: relative; +} + #account-picture { height: 56px; vertical-align: middle; width: 56px; } +#account-picture:disabled { + cursor: default; +} + #change-picture-caption { background: rgba(0, 0, 0, 0.5); bottom: 0; color: white; + cursor: pointer; font-size: small; margin: 3px 0; position: absolute; text-align: center; + visibility: hidden; /* Width of #account-picture. */ width: 56px; } -#account-picture-wrapper:not(:hover) #change-picture-caption { - visibility: hidden; +#account-picture:not(:disabled):hover + #change-picture-caption, +#account-picture:not(:disabled) + #change-picture-caption:hover { + visibility: visible; +} + +#account-picture-indicator { + -webkit-margin-end: 3px; } #sync-general { diff --git a/chrome/browser/resources/options/browser_options.js b/chrome/browser/resources/options/browser_options.js index 6e568fb..fcb48fc 100644 --- a/chrome/browser/resources/options/browser_options.js +++ b/chrome/browser/resources/options/browser_options.js @@ -220,13 +220,8 @@ cr.define('options', function() { this.updateAccountPicture_(); - $('account-picture-wrapper').oncontextmenu = function(e) { - e.preventDefault(); - }; - - $('account-picture').onclick = function() { - OptionsPage.navigateToPage('changePicture'); - }; + $('account-picture').onclick = this.showImagerPickerOverlay_; + $('change-picture-caption').onclick = this.showImagerPickerOverlay_; $('manage-accounts-button').onclick = function(event) { OptionsPage.navigateToPage('accounts'); @@ -1179,6 +1174,25 @@ cr.define('options', function() { $('themes-reset').disabled = !enabled; }, + setAccountPictureManaged_: function(managed) { + var picture = $('account-picture'); + if (managed || UIAccountTweaks.loggedInAsGuest()) { + picture.disabled = true; + ChangePictureOptions.closeOverlay(); + } else { + picture.disabled = false; + } + + // Create a synthetic pref change event decorated as + // CoreOptionsHandler::CreateValueForPref() does. + var event = new Event('account-picture'); + if (managed) + event.value = { controlledBy: 'policy' }; + else + event.value = {}; + $('account-picture-indicator').handlePrefChange(event); + }, + /** * (Re)loads IMG element with current user account picture. * @private @@ -1512,6 +1526,14 @@ cr.define('options', function() { if (index != undefined) $('bluetooth-paired-devices-list').deleteItemAtIndex(index); } + }, + + /** + * Shows the overlay dialog for changing the user avatar image. + * @private + */ + showImagerPickerOverlay_: function() { + OptionsPage.navigateToPage('changePicture'); } }; @@ -1527,6 +1549,7 @@ cr.define('options', function() { 'removeBluetoothDevice', 'removeCloudPrintConnectorSection', 'scrollToSection', + 'setAccountPictureManaged', 'setAutoOpenFileTypesDisplayed', 'setBluetoothState', 'setFontSize', diff --git a/chrome/browser/resources/options/chromeos/change_picture_options.js b/chrome/browser/resources/options/chromeos/change_picture_options.js index 9b457f6..795ba8f 100644 --- a/chrome/browser/resources/options/chromeos/change_picture_options.js +++ b/chrome/browser/resources/options/chromeos/change_picture_options.js @@ -103,7 +103,7 @@ cr.define('options', function() { this.oldImage_ = null; $('change-picture-overlay-confirm').addEventListener( - 'click', this.closePage_.bind(this)); + 'click', this.closeOverlay_.bind(this)); chrome.send('onChangePicturePageInitialized'); }, @@ -142,11 +142,12 @@ cr.define('options', function() { }, /** - * Closes current page, returning back to Personal Stuff page. + * Closes the overlay, returning to the main settings page. * @private */ - closePage_: function() { - OptionsPage.closeOverlay(); + closeOverlay_: function() { + if (!$('change-picture-page').hidden) + OptionsPage.closeOverlay(); }, /** @@ -171,7 +172,7 @@ cr.define('options', function() { */ handleChooseFile_: function() { chrome.send('chooseFile'); - this.closePage_(); + this.closeOverlay_(); }, /** @@ -223,7 +224,7 @@ cr.define('options', function() { this.handleChooseFile_(); break; default: - this.closePage_(); + this.closeOverlay_(); break; } }, @@ -297,6 +298,7 @@ cr.define('options', function() { // Forward public APIs to private implementations. [ + 'closeOverlay', 'setCameraPresent', 'setDefaultImages', 'setOldImage', diff --git a/chrome/browser/resources/options/sync_section.html b/chrome/browser/resources/options/sync_section.html index 805accc..fdf8988 100644 --- a/chrome/browser/resources/options/sync_section.html +++ b/chrome/browser/resources/options/sync_section.html @@ -14,9 +14,13 @@ <if expr="pp_ifdef('chromeos')"> <div id="account-picture-wrapper"> - <input type="image" id="account-picture" tabindex="0" - alt="" i18n-values="aria-label:changePicture"> - <div id="change-picture-caption" i18n-content="changePictureCaption"></div> + <div id="account-picture-control"> + <input type="image" id="account-picture" tabindex="0" + alt="" i18n-values="aria-label:changePicture"> + <div id="change-picture-caption" i18n-content="changePicture"></div> + </div> + <span id="account-picture-indicator" class="controlled-setting-indicator"> + </span> </div> <div id="sync-general"> </if> <!-- pp_ifdef('chromeos') --> diff --git a/chrome/browser/ui/webui/options/browser_options_handler.cc b/chrome/browser/ui/webui/options/browser_options_handler.cc index 9443532..80f9e8d 100644 --- a/chrome/browser/ui/webui/options/browser_options_handler.cc +++ b/chrome/browser/ui/webui/options/browser_options_handler.cc @@ -9,7 +9,6 @@ #include "apps/shell_window.h" #include "apps/shell_window_registry.h" -#include "base/basictypes.h" #include "base/bind.h" #include "base/bind_helpers.h" #include "base/command_line.h" @@ -103,9 +102,15 @@ #include "chrome/browser/chromeos/settings/cros_settings.h" #include "chrome/browser/chromeos/system/timezone_util.h" #include "chrome/browser/policy/browser_policy_connector.h" +#include "chrome/browser/policy/policy_service.h" +#include "chrome/browser/policy/profile_policy_connector.h" +#include "chrome/browser/policy/profile_policy_connector_factory.h" #include "chrome/browser/ui/browser_window.h" #include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/power_manager_client.h" +#include "components/policy/core/common/policy_map.h" +#include "components/policy/core/common/policy_namespace.h" +#include "policy/policy_constants.h" #include "ui/gfx/image/image_skia.h" #endif // defined(OS_CHROMEOS) @@ -777,7 +782,18 @@ void BrowserOptionsHandler::InitializeHandler() { base::Bind(&BrowserOptionsHandler::OnSigninAllowedPrefChange, base::Unretained(this))); -#if !defined(OS_CHROMEOS) +#if defined(OS_CHROMEOS) + if (!policy_registrar_) { + policy_registrar_.reset(new policy::PolicyChangeRegistrar( + policy::ProfilePolicyConnectorFactory::GetForProfile(profile)-> + policy_service(), + policy::PolicyNamespace(policy::POLICY_DOMAIN_CHROME, std::string()))); + policy_registrar_->Observe( + policy::key::kUserAvatarImage, + base::Bind(&BrowserOptionsHandler::OnUserImagePolicyChanged, + base::Unretained(this))); + } +#else // !defined(OS_CHROMEOS) profile_pref_registrar_.Add( prefs::kProxy, base::Bind(&BrowserOptionsHandler::SetupProxySettingsSection, @@ -822,6 +838,14 @@ void BrowserOptionsHandler::InitializePage() { web_ui()->CallJavascriptFunction( "BrowserOptions.enableFactoryResetSection"); } + + OnAccountPictureManagedChanged( + policy::ProfilePolicyConnectorFactory::GetForProfile( + Profile::FromWebUI(web_ui()))-> + policy_service()->GetPolicies( + policy::PolicyNamespace(policy::POLICY_DOMAIN_CHROME, + std::string())) + .Get(policy::key::kUserAvatarImage)); #endif } @@ -1176,6 +1200,11 @@ void BrowserOptionsHandler::UpdateAccountPicture() { email_value); } } + +void BrowserOptionsHandler::OnAccountPictureManagedChanged(bool managed) { + web_ui()->CallJavascriptFunction("BrowserOptions.setAccountPictureManaged", + base::FundamentalValue(managed)); +} #endif scoped_ptr<DictionaryValue> BrowserOptionsHandler::GetSyncStateDictionary() { @@ -1272,7 +1301,17 @@ void BrowserOptionsHandler::MouseExists(bool exists) { base::FundamentalValue val(exists); web_ui()->CallJavascriptFunction("BrowserOptions.showMouseControls", val); } -#endif + +void BrowserOptionsHandler::OnUserImagePolicyChanged( + const base::Value* previous_policy, + const base::Value* current_policy) { + const bool had_policy = !!previous_policy; + const bool has_policy = !!current_policy; + if (had_policy != has_policy) + OnAccountPictureManagedChanged(has_policy); +} + +#endif // defined(OS_CHROMEOS) void BrowserOptionsHandler::UpdateSyncState() { web_ui()->CallJavascriptFunction("BrowserOptions.updateSyncState", diff --git a/chrome/browser/ui/webui/options/browser_options_handler.h b/chrome/browser/ui/webui/options/browser_options_handler.h index 52028d5..b75a0aa 100644 --- a/chrome/browser/ui/webui/options/browser_options_handler.h +++ b/chrome/browser/ui/webui/options/browser_options_handler.h @@ -7,6 +7,8 @@ #include <vector> +#include "base/basictypes.h" +#include "base/compiler_specific.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" @@ -31,6 +33,14 @@ class CloudPrintSetupHandler; class CustomHomePagesTableModel; class TemplateURLService; +namespace base { +class Value; +} + +namespace policy { +class PolicyChangeRegistrar; +} + namespace options { // Chrome browser options page UI handler. @@ -85,6 +95,10 @@ class BrowserOptionsHandler // PointerDeviceObserver::Observer implementation. virtual void TouchpadExists(bool exists) OVERRIDE; virtual void MouseExists(bool exists) OVERRIDE; + + // Will be called when the policy::key::kUserAvatarImage policy changes. + void OnUserImagePolicyChanged(const base::Value* previous_policy, + const base::Value* current_policy); #endif void UpdateSyncState(); @@ -152,6 +166,11 @@ class BrowserOptionsHandler #if defined(OS_CHROMEOS) void UpdateAccountPicture(); + + // Updates the UI, allowing the user to change the avatar image if |managed| + // is |false| and preventing the user from changing the avatar image if + // |managed| is |true|. + void OnAccountPictureManagedChanged(bool managed); #endif // Callback for the "selectDownloadLocation" message. This will prompt the @@ -302,6 +321,9 @@ class BrowserOptionsHandler DoublePrefMember default_zoom_level_; PrefChangeRegistrar profile_pref_registrar_; +#if defined(OS_CHROMEOS) + scoped_ptr<policy::PolicyChangeRegistrar> policy_registrar_; +#endif // Used to get WeakPtr to self for use on the UI thread. base::WeakPtrFactory<BrowserOptionsHandler> weak_ptr_factory_; diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index bfd4a23..3665de2 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -1044,8 +1044,6 @@ 'browser/chromeos/login/wizard_in_process_browser_test.cc', 'browser/chromeos/login/wizard_in_process_browser_test.h', 'browser/chromeos/memory/oom_priority_manager_browsertest.cc', - 'browser/chromeos/policy/cloud_external_data_manager_base_test_util.cc', - 'browser/chromeos/policy/cloud_external_data_manager_base_test_util.h', 'browser/chromeos/policy/device_local_account_browsertest.cc', 'browser/chromeos/policy/device_policy_cros_browser_test.cc', 'browser/chromeos/policy/device_policy_cros_browser_test.h', diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 61c78ac..3381ba2 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -103,6 +103,8 @@ 'browser/chromeos/login/mock_user_manager.h', 'browser/chromeos/login/test/oobe_screen_waiter.cc', 'browser/chromeos/login/test/oobe_screen_waiter.h', + 'browser/chromeos/policy/cloud_external_data_manager_base_test_util.cc', + 'browser/chromeos/policy/cloud_external_data_manager_base_test_util.h', 'browser/chromeos/policy/device_policy_builder.cc', 'browser/chromeos/policy/device_policy_builder.h', 'browser/chromeos/policy/stub_enterprise_install_attributes.cc', diff --git a/chrome/test/data/policy/policy_test_cases.json b/chrome/test/data/policy/policy_test_cases.json index 925e7c1..b6369f6 100644 --- a/chrome/test/data/policy/policy_test_cases.json +++ b/chrome/test/data/policy/policy_test_cases.json @@ -10,7 +10,7 @@ "pref_mappings": [ { "pref": "The affected preference's name.", "local_state": "Whether |pref| is registered in local state's PrefService instead of the profile's PrefService. Defaults to |false| if not specified.", - "note": "The following entries should be specified if controlled setting indicators exist for |pref| in the settings UI", + "note": "The following entries should be specified if controlled setting indicators exist for |pref| in the settings UI.", "indicator_test_setup_js": "Any JavaScript that should be executed before testing the indicators. This should be specified only if an explicit user action must be simulated (e.g. clicking a button).", "indicator_selector": "A CSS selector that locates all controlled setting indicators for |pref|. This is appended to the selector 'span.controlled-setting-indicator' and if not specified, defaults to '[pref=(the value of |pref|)', e.g. '[pref=homepage]'.", "note 2": "Any number of test cases may be specified in the following array.", @@ -21,7 +21,9 @@ } ] } - ] + ], + "note 2": "The following entry should be specified if there is a controlled setting indicator that reacts to the policy directly, without a preference serving as an intermediary.", + "indicator_selector": "A CSS selector that locates the controlled setting indicator directly affected by the policy. This is appended to the selector 'span.controlled-setting-indicator'." }, "HomepageLocation": { @@ -1549,7 +1551,14 @@ }, "UserAvatarImage": { - "os": ["chromeos"] + "os": ["chromeos"], + "test_policy": { + "UserAvatarImage": { + "url": "http://localhost/", + "hash": "01234567890012345678900123456789001234567890" + } + }, + "indicator_selector": "#account-picture-indicator" }, "----- Chrome OS policies ------------------------------------------------": {}, |