[profiles] Add profile colors into ProfileAttributesEntry

Upcoming updates to the multi-profile experience make the "main" color
of the profile more prominent on several surfaces. One of these surfaces
is the profile picker window which should have access to colors
attributed to unloaded profiles.

Currently, the profile theme info is stored in profile preferences which
are read from the disk when a profile is loaded. This CL caches required
colors in ProfileAttributesStorage. Data from ProfileAttributesStorage
can be accessed without having to actually load a profile from disk.

The colors are updated every time the profile's theme changes. This is
implemented by listening for the
chrome::NOTIFICATION_BROWSER_THEME_CHANGED notification in
ProfileThemeUpdateService.

Bug: 1102384
Change-Id: I4683ded5d7182117dd2a712437f64f1f00ceed72
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2317796
Commit-Queue: Alex Ilin <alexilin@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792683}

chrome/browser/BUILD.gn

@@ -3407,6 +3407,10 @@ static_library("browser") {
       "profile_resetter/triggered_profile_resetter_factory.h",
       "profiles/profile_shortcut_manager.cc",
       "profiles/profile_shortcut_manager.h",
+      "profiles/profile_theme_update_service.cc",
+      "profiles/profile_theme_update_service.h",
+      "profiles/profile_theme_update_service_factory.cc",
+      "profiles/profile_theme_update_service_factory.h",
       "profiles/profile_window.cc",
       "profiles/profile_window.h",
       "renderer_context_menu/accessibility_labels_bubble_model.cc",

chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc

@@ -110,6 +110,7 @@
 #include "chrome/browser/feedback/feedback_uploader_factory_chrome.h"
 #include "chrome/browser/media/feeds/media_feeds_service_factory.h"
 #include "chrome/browser/metrics/desktop_session_duration/desktop_profile_session_durations_service_factory.h"
+#include "chrome/browser/profiles/profile_theme_update_service_factory.h"
 #include "chrome/browser/resource_coordinator/local_site_characteristics_data_store_factory.h"
 #include "chrome/browser/search/instant_service_factory.h"
 #include "chrome/browser/storage/storage_notification_service_factory.h"
@@ -328,6 +329,9 @@ void ChromeBrowserMainExtraPartsProfiles::
   prerender::PrerenderLinkManagerFactory::GetInstance();
   prerender::PrerenderManagerFactory::GetInstance();
   ProfileSyncServiceFactory::GetInstance();
+#if !defined(OS_ANDROID)
+  ProfileThemeUpdateServiceFactory::GetInstance();
+#endif
   ProtocolHandlerRegistryFactory::GetInstance();
   RendererUpdaterFactory::GetInstance();
 #if !defined(OS_ANDROID)

chrome/browser/profiles/profile_attributes_entry.cc

@@ -43,6 +43,11 @@ const char kMetricsBucketIndex[] = "metrics_bucket_index";
 const char kSigninRequiredKey[] = "signin_required";
 const char kHostedDomain[] = "hosted_domain";
 
+// Profile colors info.
+const char kProfileHighlightColorKey[] = "profile_highlight_color";
+const char kDefaultAvatarFillColorKey[] = "default_avatar_fill_color";
+const char kDefaultAvatarStrokeColorKey[] = "default_avatar_stroke_color";
+
 // Low-entropy accounts info, for metrics only.
 const char kFirstAccountNameHash[] = "first_account_name_hash";
 const char kHasMultipleAccountNames[] = "has_multiple_account_names";
@@ -392,6 +397,29 @@ size_t ProfileAttributesEntry::GetAvatarIconIndex() const {
   return icon_index;
 }
 
+base::Optional<ProfileThemeColors>
+ProfileAttributesEntry::GetProfileThemeColors() const {
+  base::Optional<SkColor> profile_highlight_color =
+      GetProfileThemeColor(kProfileHighlightColorKey);
+  base::Optional<SkColor> default_avatar_fill_color =
+      GetProfileThemeColor(kDefaultAvatarFillColorKey);
+  base::Optional<SkColor> default_avatar_stroke_color =
+      GetProfileThemeColor(kDefaultAvatarStrokeColorKey);
+  if (!profile_highlight_color.has_value()) {
+    DCHECK(!default_avatar_fill_color.has_value() &&
+           !default_avatar_stroke_color.has_value());
+    return base::nullopt;
+  }
+
+  DCHECK(default_avatar_fill_color.has_value() &&
+         default_avatar_stroke_color.has_value());
+  ProfileThemeColors colors;
+  colors.profile_highlight_color = profile_highlight_color.value();
+  colors.default_avatar_fill_color = default_avatar_fill_color.value();
+  colors.default_avatar_stroke_color = default_avatar_stroke_color.value();
+  return colors;
+}
+
 size_t ProfileAttributesEntry::GetMetricsBucketIndex() {
   int bucket_index = GetInteger(kMetricsBucketIndex);
   if (bucket_index == kIntegerNotSet) {
@@ -541,6 +569,20 @@ void ProfileAttributesEntry::SetAvatarIconIndex(size_t icon_index) {
   profile_info_cache_->NotifyOnProfileAvatarChanged(profile_path);
 }
 
+void ProfileAttributesEntry::SetProfileThemeColors(
+    const base::Optional<ProfileThemeColors>& colors) {
+  if (colors.has_value()) {
+    SetInteger(kProfileHighlightColorKey, colors->profile_highlight_color);
+    SetInteger(kDefaultAvatarFillColorKey, colors->default_avatar_fill_color);
+    SetInteger(kDefaultAvatarStrokeColorKey,
+               colors->default_avatar_stroke_color);
+  } else {
+    ClearValue(kProfileHighlightColorKey);
+    ClearValue(kDefaultAvatarFillColorKey);
+    ClearValue(kDefaultAvatarStrokeColorKey);
+  }
+}
+
 void ProfileAttributesEntry::SetHostedDomain(std::string hosted_domain) {
   SetString(kHostedDomain, hosted_domain);
 }
@@ -723,6 +765,14 @@ int ProfileAttributesEntry::GetInteger(const char* key) const {
   return value->GetInt();
 }
 
+base::Optional<SkColor> ProfileAttributesEntry::GetProfileThemeColor(
+    const char* key) const {
+  const base::Value* value = GetValue(key);
+  if (!value || !value->is_int())
+    return base::nullopt;
+  return value->GetInt();
+}
+
 // Type checking. Only IsDouble is implemented because others do not have
 // callsites.
 bool ProfileAttributesEntry::IsDouble(const char* key) const {

chrome/browser/profiles/profile_attributes_entry.h

@@ -14,9 +14,11 @@
 #include "base/files/file_path.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "base/strings/string16.h"
 #include "base/time/time.h"
 #include "base/values.h"
+#include "third_party/skia/include/core/SkColor.h"
 
 namespace gfx {
 class Image;
@@ -40,6 +42,12 @@ enum class NameForm {
 
 enum class AccountCategory { kConsumer, kEnterprise };
 
+struct ProfileThemeColors {
+  SkColor profile_highlight_color;
+  SkColor default_avatar_fill_color;
+  SkColor default_avatar_stroke_color;
+};
+
 class ProfileAttributesEntry {
  public:
   static void RegisterLocalStatePrefs(PrefRegistrySimple* registry);
@@ -130,6 +138,9 @@ class ProfileAttributesEntry {
   bool IsSignedInWithCredentialProvider() const;
   // Returns the index of the default icon used by the profile.
   size_t GetAvatarIconIndex() const;
+  // Returns the colors specified by the profile theme. base::nullopt indicates
+  // the default colors should be used for this profile.
+  base::Optional<ProfileThemeColors> GetProfileThemeColors() const;
   // Returns the metrics bucket this profile should be recorded in.
   // Note: The bucket index is assigned once and remains the same all time. 0 is
   // reserved for the guest profile.
@@ -160,6 +171,7 @@ class ProfileAttributesEntry {
   void SetIsUsingDefaultAvatar(bool value);
   void SetIsAuthError(bool value);
   void SetAvatarIconIndex(size_t icon_index);
+  void SetProfileThemeColors(const base::Optional<ProfileThemeColors>& colors);
 
   // Unlike for other string setters, the argument is expected to be UTF8
   // encoded.
@@ -254,6 +266,10 @@ class ProfileAttributesEntry {
   bool GetBool(const char* key) const;
   int GetInteger(const char* key) const;
 
+  // Internal getter that returns one of the profile theme colors or
+  // base::nullopt if the key is not present.
+  base::Optional<SkColor> GetProfileThemeColor(const char* key) const;
+
   // Type checking. Only IsDouble is implemented because others do not have
   // callsites.
   bool IsDouble(const char* key) const;

chrome/browser/profiles/profile_attributes_storage_unittest.cc

@@ -13,6 +13,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "build/build_config.h"
+#include "chrome/browser/profiles/profile_attributes_entry.h"
 #include "chrome/browser/profiles/profile_avatar_downloader.h"
 #include "chrome/browser/profiles/profile_avatar_icon_util.h"
 #include "chrome/browser/profiles/profile_info_cache.h"
@@ -913,3 +914,30 @@ TEST_F(ProfileAttributesStorageTest, ProfilesState_SingleProfile) {
   histogram_tester.ExpectTotalCount(
       "Profile.State.LastUsed_LatentMultiProfileOthers", 0);
 }
+
+TEST_F(ProfileAttributesStorageTest, ProfileThemeColors) {
+  AddTestingProfile();
+  base::FilePath profile_path = GetProfilePath("testing_profile_path0");
+
+  DisableObserver();  // No need to test observers in this test.
+
+  ProfileAttributesEntry* entry;
+  ASSERT_TRUE(storage()->GetProfileAttributesWithPath(profile_path, &entry));
+  EXPECT_EQ(base::nullopt, entry->GetProfileThemeColors());
+
+  ProfileThemeColors colors = {SK_ColorTRANSPARENT, SK_ColorBLACK,
+                               SK_ColorWHITE};
+  entry->SetProfileThemeColors(colors);
+  base::Optional<ProfileThemeColors> actual_colors =
+      entry->GetProfileThemeColors();
+  ASSERT_TRUE(actual_colors.has_value());
+  EXPECT_EQ(colors.profile_highlight_color,
+            actual_colors->profile_highlight_color);
+  EXPECT_EQ(colors.default_avatar_fill_color,
+            actual_colors->default_avatar_fill_color);
+  EXPECT_EQ(colors.default_avatar_stroke_color,
+            actual_colors->default_avatar_stroke_color);
+
+  entry->SetProfileThemeColors(base::nullopt);
+  EXPECT_EQ(base::nullopt, entry->GetProfileThemeColors());
+}

chrome/browser/profiles/profile_theme_update_service.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/profiles/profile_theme_update_service.h"
+
+#include "base/notreached.h"
+#include "base/optional.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_attributes_entry.h"
+#include "chrome/browser/profiles/profile_attributes_storage.h"
+#include "chrome/browser/themes/theme_properties.h"
+#include "chrome/browser/themes/theme_service.h"
+#include "content/public/browser/notification_details.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_source.h"
+#include "ui/base/theme_provider.h"
+
+ProfileThemeUpdateService::ProfileThemeUpdateService(
+    Profile* profile,
+    ProfileAttributesStorage* profile_attributes_storage,
+    ThemeService* theme_service)
+    : profile_(profile),
+      profile_attributes_storage_(profile_attributes_storage),
+      theme_service_(theme_service) {
+  notification_registrar_.Add(this, chrome::NOTIFICATION_BROWSER_THEME_CHANGED,
+                              content::NotificationService::AllSources());
+  // Kicks off an update on startup.
+  UpdateProfileThemeColors();
+}
+
+ProfileThemeUpdateService::~ProfileThemeUpdateService() = default;
+
+void ProfileThemeUpdateService::UpdateProfileThemeColors() {
+  ProfileAttributesEntry* entry = nullptr;
+  bool has_entry = profile_attributes_storage_->GetProfileAttributesWithPath(
+      profile_->GetPath(), &entry);
+  if (!has_entry)
+    return;
+
+  if (!theme_service_->UsingAutogeneratedTheme()) {
+    // Only save colors for autogenerated themes.
+    entry->SetProfileThemeColors(base::nullopt);
+    return;
+  }
+
+  const ui::ThemeProvider& theme_provider =
+      ThemeService::GetThemeProviderForProfile(profile_);
+  ProfileThemeColors colors;
+  // TODO(https://crbug.com/1102384): update this with the right colors, once we
+  // have them.
+  colors.profile_highlight_color =
+      theme_provider.GetColor(ThemeProperties::COLOR_FRAME_ACTIVE);
+  colors.default_avatar_fill_color =
+      theme_provider.GetColor(ThemeProperties::COLOR_FRAME_ACTIVE);
+  colors.default_avatar_stroke_color =
+      theme_provider.GetColor(ThemeProperties::COLOR_TOOLBAR);
+  entry->SetProfileThemeColors(colors);
+}
+
+// content::NotificationObserver:
+void ProfileThemeUpdateService::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  switch (type) {
+    case chrome::NOTIFICATION_BROWSER_THEME_CHANGED:
+      UpdateProfileThemeColors();
+      break;
+    default:
+      NOTREACHED();
+  }
+}

chrome/browser/profiles/profile_theme_update_service.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_PROFILES_PROFILE_THEME_UPDATE_SERVICE_H_
+#define CHROME_BROWSER_PROFILES_PROFILE_THEME_UPDATE_SERVICE_H_
+
+#include "components/keyed_service/core/keyed_service.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+
+namespace content {
+class NotificationSource;
+class NotificationDetails;
+}  // namespace content
+
+class Profile;
+class ProfileAttributesStorage;
+class ThemeService;
+
+// A KeyedService that listens to browser theme updates and updates profile
+// theme colors cached in ProfileAttributesStorage.
+//
+// These colors are used to display a list of profiles. They are cached to be
+// accessible without having to load the profiles from disk.
+class ProfileThemeUpdateService : public KeyedService,
+                                  public content::NotificationObserver {
+ public:
+  ProfileThemeUpdateService(
+      Profile* profile,
+      ProfileAttributesStorage* profile_attributes_storage,
+      ThemeService* theme_service);
+  ~ProfileThemeUpdateService() override;
+
+  // This class in uncopyable.
+  ProfileThemeUpdateService(const ProfileThemeUpdateService&) = delete;
+  ProfileThemeUpdateService& operator=(const ProfileThemeUpdateService&) =
+      delete;
+
+  // content::NotificationObserver:
+  void Observe(int type,
+               const content::NotificationSource& source,
+               const content::NotificationDetails& details) override;
+
+ private:
+  // Updates profile theme colors in |profile_attributes_storage_| for
+  // |profile_|.
+  void UpdateProfileThemeColors();
+
+  Profile* const profile_;
+  ProfileAttributesStorage* const profile_attributes_storage_;
+  ThemeService* const theme_service_;
+
+  content::NotificationRegistrar notification_registrar_;
+};
+
+#endif  // CHROME_BROWSER_PROFILES_PROFILE_THEME_UPDATE_SERVICE_H_

chrome/browser/profiles/profile_theme_update_service_factory.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/profiles/profile_theme_update_service_factory.h"
+
+#include "base/feature_list.h"
+#include "base/no_destructor.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/profiles/profile_theme_update_service.h"
+#include "chrome/browser/themes/theme_service_factory.h"
+#include "chrome/browser/ui/ui_features.h"
+#include "components/keyed_service/content/browser_context_dependency_manager.h"
+#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
+
+// static
+ProfileThemeUpdateService* ProfileThemeUpdateServiceFactory::GetForProfile(
+    Profile* profile) {
+  return static_cast<ProfileThemeUpdateService*>(
+      GetInstance()->GetServiceForBrowserContext(profile, true));
+}
+
+// static
+ProfileThemeUpdateServiceFactory*
+ProfileThemeUpdateServiceFactory::GetInstance() {
+  static base::NoDestructor<ProfileThemeUpdateServiceFactory> factory;
+  return factory.get();
+}
+
+ProfileThemeUpdateServiceFactory::ProfileThemeUpdateServiceFactory()
+    : BrowserContextKeyedServiceFactory(
+          "ProfileThemeUpdateServiceFactory",
+          BrowserContextDependencyManager::GetInstance()) {
+  DependsOn(ThemeServiceFactory::GetInstance());
+}
+
+ProfileThemeUpdateServiceFactory::~ProfileThemeUpdateServiceFactory() = default;
+
+KeyedService* ProfileThemeUpdateServiceFactory::BuildServiceInstanceFor(
+    content::BrowserContext* context) const {
+  if (!base::FeatureList::IsEnabled(features::kNewProfilePicker))
+    return nullptr;
+
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+  if (!profile_manager)
+    return nullptr;  // Some tests don't have a profile manager.
+
+  Profile* profile = Profile::FromBrowserContext(context);
+  return new ProfileThemeUpdateService(
+      profile, &profile_manager->GetProfileAttributesStorage(),
+      ThemeServiceFactory::GetForProfile(profile));
+}
+
+bool ProfileThemeUpdateServiceFactory::ServiceIsCreatedWithBrowserContext()
+    const {
+  return true;
+}
+
+bool ProfileThemeUpdateServiceFactory::ServiceIsNULLWhileTesting() const {
+  return true;
+}

chrome/browser/profiles/profile_theme_update_service_factory.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_PROFILES_PROFILE_THEME_UPDATE_SERVICE_FACTORY_H_
+#define CHROME_BROWSER_PROFILES_PROFILE_THEME_UPDATE_SERVICE_FACTORY_H_
+
+#include "base/no_destructor.h"
+#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
+
+class ProfileThemeUpdateService;
+class Profile;
+
+// Singleton that owns all ProfileThemeUpdateServices and associates them with
+// Profiles.
+class ProfileThemeUpdateServiceFactory
+    : public BrowserContextKeyedServiceFactory {
+ public:
+  static ProfileThemeUpdateService* GetForProfile(Profile* profile);
+  static ProfileThemeUpdateServiceFactory* GetInstance();
+
+  // This class is uncopyable.
+  ProfileThemeUpdateServiceFactory(const ProfileThemeUpdateServiceFactory&) =
+      delete;
+  ProfileThemeUpdateServiceFactory& operator=(
+      const ProfileThemeUpdateServiceFactory&) = delete;
+
+ private:
+  friend base::NoDestructor<ProfileThemeUpdateServiceFactory>;
+
+  ProfileThemeUpdateServiceFactory();
+  ~ProfileThemeUpdateServiceFactory() override;
+
+  // BrowserContextKeyedServiceFactory:
+  KeyedService* BuildServiceInstanceFor(
+      content::BrowserContext* context) const override;
+  bool ServiceIsCreatedWithBrowserContext() const override;
+  bool ServiceIsNULLWhileTesting() const override;
+};
+
+#endif  // CHROME_BROWSER_PROFILES_PROFILE_THEME_UPDATE_SERVICE_FACTORY_H_