Persist account images from AccountTrackerService to disk

This CL implements the storage mechanism for the images in
AccountTrackerService. Images are save to and loaded from
disk.

Bug: 794522
Change-Id: I6e54b5467e11d8f32a8518ab042218e74914ca3c
Reviewed-on: https://chromium-review.googlesource.com/913575Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: Thomas Tangl <tangltom@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536693}

chrome/browser/signin/account_tracker_service_factory.cc

@@ -42,6 +42,7 @@ KeyedService* AccountTrackerServiceFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
   Profile* profile = static_cast<Profile*>(context);
   AccountTrackerService* service = new AccountTrackerService();
-  service->Initialize(ChromeSigninClientFactory::GetForProfile(profile));
+  service->Initialize(ChromeSigninClientFactory::GetForProfile(profile),
+                      profile->GetPath());
   return service;
 }

components/signin/core/browser/account_tracker_service.cc

@@ -8,10 +8,14 @@
 
 #include "base/callback.h"
 #include "base/command_line.h"
+#include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_split.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/task_runner_util.h"
+#include "base/task_scheduler/post_task.h"
+#include "base/task_scheduler/task_traits.h"
 #include "base/trace_event/trace_event.h"
 #include "build/build_config.h"
 #include "components/pref_registry/pref_registry_syncable.h"
@@ -44,6 +48,44 @@ void RemoveDeprecatedServiceFlags(PrefService* pref_service) {
   }
 }
 
+// Reads a PNG image from disk and decodes it. If the reading/decoding attempt
+// was unsuccessful, an empty image is returned.
+gfx::Image ReadImage(const base::FilePath& image_path) {
+  base::AssertBlockingAllowed();
+
+  if (!base::PathExists(image_path))
+    return gfx::Image();
+  std::string image_data;
+  if (!base::ReadFileToString(image_path, &image_data)) {
+    LOG(ERROR) << "Failed to read image from disk: " << image_path;
+    return gfx::Image();
+  }
+  return gfx::Image::CreateFrom1xPNGBytes(
+      base::RefCountedString::TakeString(&image_data));
+}
+
+// Saves |png_data| to disk at |image_path|.
+void SaveImage(scoped_refptr<base::RefCountedMemory> png_data,
+               const base::FilePath& image_path) {
+  base::AssertBlockingAllowed();
+  // Make sure the destination directory exists.
+  base::FilePath dir = image_path.DirName();
+  if (!base::DirectoryExists(dir) && !base::CreateDirectory(dir)) {
+    LOG(ERROR) << "Failed to create parent directory of: " << image_path;
+    return;
+  }
+  if (base::WriteFile(image_path, png_data->front_as<char>(),
+                      png_data->size()) == -1) {
+    LOG(ERROR) << "Failed to save image to file: " << image_path;
+  }
+}
+
+// Removes the image at path |image_path|.
+void RemoveImage(const base::FilePath& image_path) {
+  if (!base::DeleteFile(image_path, false /* recursive */))
+    LOG(ERROR) << "Failed to delete image.";
+}
+
 }  // namespace
 
 const char AccountTrackerService::kAccountInfoPref[] = "account_info";
@@ -56,7 +98,11 @@ const char AccountTrackerService::kNoHostedDomainFound[] = "NO_HOSTED_DOMAIN";
 // This must be a string which can never be a valid picture URL.
 const char AccountTrackerService::kNoPictureURLFound[] = "NO_PICTURE_URL";
 
-AccountTrackerService::AccountTrackerService() : signin_client_(nullptr) {}
+const char AccountTrackerService::kAccountsFolder[] = "Accounts";
+const char AccountTrackerService::kAvatarImagesFolder[] = "Avatar Images";
+
+AccountTrackerService::AccountTrackerService()
+    : signin_client_(nullptr), weak_factory_(this) {}
 
 AccountTrackerService::~AccountTrackerService() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -70,11 +116,21 @@ void AccountTrackerService::RegisterPrefs(
                                 AccountTrackerService::MIGRATION_NOT_STARTED);
 }
 
-void AccountTrackerService::Initialize(SigninClient* signin_client) {
+void AccountTrackerService::Initialize(SigninClient* signin_client,
+                                       const base::FilePath& user_data_dir) {
   DCHECK(signin_client);
   DCHECK(!signin_client_);
   signin_client_ = signin_client;
   LoadFromPrefs();
+  user_data_dir_ = user_data_dir;
+  if (!user_data_dir_.empty()) {
+    // |image_storage_task_runner_| is a sequenced runner because we want to
+    // avoid read and write operations to the same file at the same time.
+    image_storage_task_runner_ = base::CreateSequencedTaskRunnerWithTraits(
+        {base::MayBlock(), base::TaskPriority::USER_VISIBLE,
+         base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN});
+    LoadAccountImagesFromDisk();
+  }
 }
 
 void AccountTrackerService::Shutdown() {
@@ -213,6 +269,7 @@ void AccountTrackerService::StopTrackingAccount(const std::string& account_id) {
   if (base::ContainsKey(accounts_, account_id)) {
     AccountState state = std::move(accounts_[account_id]);
     RemoveFromPrefs(state);
+    RemoveAccountImageFromDisk(account_id);
     accounts_.erase(account_id);
 
     if (!state.info.gaia.empty())
@@ -260,6 +317,7 @@ void AccountTrackerService::SetAccountImage(const std::string& account_id,
                                             const gfx::Image& image) {
   DCHECK(base::ContainsKey(accounts_, account_id));
   accounts_[account_id].image = image;
+  SaveAccountImageToDisk(account_id, image);
   NotifyAccountImageUpdated(account_id, image);
 }
 
@@ -315,6 +373,7 @@ void AccountTrackerService::MigrateToGaiaId() {
     if (base::ContainsKey(accounts_, account_id)) {
       AccountState& state = accounts_[account_id];
       RemoveFromPrefs(state);
+      RemoveAccountImageFromDisk(account_id);
       accounts_.erase(account_id);
     }
   }
@@ -326,6 +385,52 @@ void AccountTrackerService::MigrateToGaiaId() {
   }
 }
 
+base::FilePath AccountTrackerService::GetImagePathFor(
+    const std::string& account_id) {
+  return user_data_dir_.AppendASCII(kAccountsFolder)
+      .AppendASCII(kAvatarImagesFolder)
+      .AppendASCII(account_id);
+}
+
+void AccountTrackerService::OnAccountImageLoaded(const std::string& account_id,
+                                                 gfx::Image image) {
+  if (base::ContainsKey(accounts_, account_id) &&
+      accounts_[account_id].image.IsEmpty()) {
+    accounts_[account_id].image = image;
+  }
+}
+
+void AccountTrackerService::LoadAccountImagesFromDisk() {
+  if (!image_storage_task_runner_)
+    return;
+  for (const std::pair<std::string, AccountState>& account : accounts_) {
+    const std::string& account_id = account.second.info.account_id;
+    PostTaskAndReplyWithResult(
+        image_storage_task_runner_.get(), FROM_HERE,
+        base::BindOnce(&ReadImage, GetImagePathFor(account_id)),
+        base::BindOnce(&AccountTrackerService::OnAccountImageLoaded,
+                       weak_factory_.GetWeakPtr(), account_id));
+  }
+}
+
+void AccountTrackerService::SaveAccountImageToDisk(
+    const std::string& account_id,
+    const gfx::Image& image) {
+  if (!image_storage_task_runner_)
+    return;
+  image_storage_task_runner_->PostTask(
+      FROM_HERE, base::BindOnce(&SaveImage, image.As1xPNGBytes(),
+                                GetImagePathFor(account_id)));
+}
+
+void AccountTrackerService::RemoveAccountImageFromDisk(
+    const std::string& account_id) {
+  if (!image_storage_task_runner_)
+    return;
+  image_storage_task_runner_->PostTask(
+      FROM_HERE, base::BindOnce(&RemoveImage, GetImagePathFor(account_id)));
+}
+
 void AccountTrackerService::LoadFromPrefs() {
   const base::ListValue* list =
       signin_client_->GetPrefs()->GetList(kAccountInfoPref);
@@ -395,6 +500,7 @@ void AccountTrackerService::LoadFromPrefs() {
     AccountState state;
     state.info.account_id = account_id;
     RemoveFromPrefs(state);
+    RemoveAccountImageFromDisk(account_id);
   }
 
   if (GetMigrationState() != MIGRATION_DONE) {

components/signin/core/browser/account_tracker_service.h

@@ -10,6 +10,7 @@
 #include <string>
 #include <vector>
 
+#include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/observer_list.h"
@@ -52,6 +53,10 @@ class AccountTrackerService : public KeyedService {
   // Child account service flag name.
   static const char kChildAccountServiceFlag[];
 
+  // Account folders used for storing account related data at disk.
+  static const char kAccountsFolder[];
+  static const char kAvatarImagesFolder[];
+
   // Clients of AccountTrackerService can implement this interface and register
   // with AddObserver() to learn about account information changes.
   class Observer {
@@ -88,7 +93,10 @@ class AccountTrackerService : public KeyedService {
   // Take a SigninClient rather than a PrefService and a URLRequestContextGetter
   // since RequestContext cannot be created at startup.
   // (see http://crbug.com/171406)
-  void Initialize(SigninClient* signin_client);
+  // If |user_data_dir| is empty, images will not be saved to or loaded from
+  // disk.
+  void Initialize(SigninClient* signin_client,
+                  const base::FilePath& user_data_dir = base::FilePath());
 
   // Returns the list of known accounts and for which gaia IDs
   // have been fetched.
@@ -162,6 +170,14 @@ class AccountTrackerService : public KeyedService {
   void SaveToPrefs(const AccountState& account);
   void RemoveFromPrefs(const AccountState& account);
 
+  // Used to load/save account images from/to disc.
+  base::FilePath GetImagePathFor(const std::string& account_id);
+  void OnAccountImageLoaded(const std::string& account_id, gfx::Image image);
+  void LoadAccountImagesFromDisk();
+  void SaveAccountImageToDisk(const std::string& account_id,
+                              const gfx::Image& image);
+  void RemoveAccountImageFromDisk(const std::string& account_id);
+
   // Gaia id migration.
   bool IsMigratable() const;
   void MigrateToGaiaId();
@@ -171,8 +187,17 @@ class AccountTrackerService : public KeyedService {
   std::map<std::string, AccountState> accounts_;
   base::ObserverList<Observer> observer_list_;
 
+  base::FilePath user_data_dir_;
+
+  // Task runner used for file operations on avatar images.
+  scoped_refptr<base::SequencedTaskRunner> image_storage_task_runner_;
+
   SEQUENCE_CHECKER(sequence_checker_);
 
+  // Used to pass weak pointers of |this| to tasks created by
+  // |image_storage_task_runner_|.
+  base::WeakPtrFactory<AccountTrackerService> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(AccountTrackerService);
 };