Commit 1853406f authored by David Black's avatar David Black Committed by Commit Bot

Move persistence logic to `HoldingSpacePersistenceDelegate`.

Following this CL, there will be one delegate left to add to move
download manager logic out of `HoldingSpaceKeyedService`.

Bug: 1119496
Change-Id: Ida9e3df4f45386f977ac0ce4d2a7b9f96e4cf6c2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2383191
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: default avatarToni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803010}
parent 124e35e6
tbarzic@chromium.org
newcomer@chromium.org
dmblack@chromium.org
dmblack@google.com
......@@ -1876,6 +1876,8 @@ static_library("ui") {
"ash/holding_space/holding_space_keyed_service_delegate.h",
"ash/holding_space/holding_space_keyed_service_factory.cc",
"ash/holding_space/holding_space_keyed_service_factory.h",
"ash/holding_space/holding_space_persistence_delegate.cc",
"ash/holding_space/holding_space_persistence_delegate.h",
"ash/holding_space/holding_space_thumbnail_loader.cc",
"ash/holding_space/holding_space_thumbnail_loader.h",
"ash/holding_space/holding_space_util.cc",
......
......@@ -65,16 +65,14 @@ class HoldingSpaceFileSystemDelegate::FileSystemWatcher {
// HoldingSpaceFileSystemDelegate ----------------------------------------------
HoldingSpaceFileSystemDelegate::HoldingSpaceFileSystemDelegate(
Profile* profile,
HoldingSpaceModel* model,
FileRemovedCallback file_removed_callback)
: HoldingSpaceKeyedServiceDelegate(model),
: HoldingSpaceKeyedServiceDelegate(profile, model),
file_removed_callback_(file_removed_callback),
file_system_watcher_runner_(base::ThreadPool::CreateSequencedTaskRunner(
{base::MayBlock(), base::TaskPriority::BEST_EFFORT})) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
file_system_watcher_ = std::make_unique<FileSystemWatcher>(
base::Bind(&HoldingSpaceFileSystemDelegate::OnFilePathChanged,
weak_factory_.GetWeakPtr()));
}
HoldingSpaceFileSystemDelegate::~HoldingSpaceFileSystemDelegate() {
......@@ -84,6 +82,13 @@ HoldingSpaceFileSystemDelegate::~HoldingSpaceFileSystemDelegate() {
file_system_watcher_.release());
}
void HoldingSpaceFileSystemDelegate::Init() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
file_system_watcher_ = std::make_unique<FileSystemWatcher>(
base::Bind(&HoldingSpaceFileSystemDelegate::OnFilePathChanged,
weak_factory_.GetWeakPtr()));
}
// TODO(dmblack): Watch `item`'s parent directory instead of its backing file so
// that we can reuse the same watcher across multiple holding space items.
void HoldingSpaceFileSystemDelegate::OnHoldingSpaceItemAdded(
......
......@@ -25,8 +25,10 @@ class HoldingSpaceFileSystemDelegate : public HoldingSpaceKeyedServiceDelegate {
using FileRemovedCallback =
base::RepeatingCallback<void(const base::FilePath&)>;
HoldingSpaceFileSystemDelegate(HoldingSpaceModel* model,
HoldingSpaceFileSystemDelegate(Profile* profile,
HoldingSpaceModel* model,
FileRemovedCallback file_removed_callback);
HoldingSpaceFileSystemDelegate(const HoldingSpaceFileSystemDelegate&) =
delete;
HoldingSpaceFileSystemDelegate& operator=(
......@@ -37,6 +39,7 @@ class HoldingSpaceFileSystemDelegate : public HoldingSpaceKeyedServiceDelegate {
class FileSystemWatcher;
// HoldingSpaceKeyedServiceDelegate:
void Init() override;
void OnHoldingSpaceItemAdded(const HoldingSpaceItem* item) override;
void OnHoldingSpaceItemRemoved(const HoldingSpaceItem* item) override;
......
......@@ -9,7 +9,6 @@
#include <vector>
#include "ash/public/cpp/holding_space/holding_space_model.h"
#include "ash/public/cpp/holding_space/holding_space_model_observer.h"
#include "base/scoped_observer.h"
#include "base/strings/string16.h"
#include "chrome/browser/profiles/profile_manager.h"
......@@ -47,22 +46,14 @@ class FileSystemURL;
namespace ash {
class HoldingSpaceItem;
using HoldingSpaceItemPtr = std::unique_ptr<HoldingSpaceItem>;
// Browser context keyed service that:
// * Manages the temporary holding space per-profile data model.
// * Serves as an entry point to add holding space items from Chrome.
class HoldingSpaceKeyedService : public KeyedService,
public HoldingSpaceModelObserver,
public ProfileManagerObserver,
public content::DownloadManager::Observer,
public download::DownloadItem::Observer {
public:
// Preference path at which holding space items are persisted.
// NOTE: Any changes to persistence must be backwards compatible.
static constexpr char kPersistencePath[] = "ash.holding_space.items";
HoldingSpaceKeyedService(content::BrowserContext* context,
const AccountId& account_id);
HoldingSpaceKeyedService(const HoldingSpaceKeyedService& other) = delete;
......@@ -97,6 +88,9 @@ class HoldingSpaceKeyedService : public KeyedService,
// Adds a download item backed by the provided absolute file path.
void AddDownload(const base::FilePath& download_path);
// Adds the specified `item` to the holding space model.
void AddItem(std::unique_ptr<HoldingSpaceItem> item);
const HoldingSpaceClient* client_for_testing() const {
return &holding_space_client_;
}
......@@ -115,10 +109,6 @@ class HoldingSpaceKeyedService : public KeyedService,
// KeyedService:
void Shutdown() override;
// HoldingSpaceModelObserver:
void OnHoldingSpaceItemAdded(const HoldingSpaceItem* item) override;
void OnHoldingSpaceItemRemoved(const HoldingSpaceItem* item) override;
// ProfileManagerObserver:
void OnProfileAdded(Profile* profile) override;
......@@ -135,20 +125,15 @@ class HoldingSpaceKeyedService : public KeyedService,
// - `download_items_observer_`.
void RemoveDownloadManagerObservers();
// Restores `holding_space_model_` from persistent storage.
void RestoreModelFromPersistence();
void RestoreModelByExistence(
std::vector<HoldingSpaceItemPtr> existing_items,
std::vector<HoldingSpaceItemPtr> non_existing_items);
void OnModelRestored();
// Resolves file attributes from a `file_path`;
GURL ResolveFileSystemUrl(const base::FilePath& file_path) const;
gfx::ImageSkia ResolveImage(const base::FilePath& file_path) const;
// Invoked when the associated profile is ready.
void OnProfileReady();
// Invoked when the specified `file_path` is removed.
void OnFileRemoved(const base::FilePath& file_path);
// Invoked when the holding space model has been restored from persistence.
void OnModelRestored();
content::BrowserContext* const browser_context_;
const AccountId account_id_;
......@@ -162,9 +147,6 @@ class HoldingSpaceKeyedService : public KeyedService,
// service. They operate autonomously of one another.
std::vector<std::unique_ptr<HoldingSpaceKeyedServiceDelegate>> delegates_;
ScopedObserver<HoldingSpaceModel, HoldingSpaceModelObserver>
holding_space_model_observer_{this};
ScopedObserver<ProfileManager, ProfileManagerObserver>
profile_manager_observer_{this};
......
......@@ -4,13 +4,33 @@
#include "chrome/browser/ui/ash/holding_space/holding_space_keyed_service_delegate.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/profiles/profile_manager.h"
namespace ash {
namespace {
ProfileManager* GetProfileManager() {
return g_browser_process->profile_manager();
}
} // namespace
HoldingSpaceKeyedServiceDelegate::~HoldingSpaceKeyedServiceDelegate() = default;
void HoldingSpaceKeyedServiceDelegate::NotifyHoldingSpaceModelRestored() {
DCHECK(is_restoring_);
is_restoring_ = false;
OnHoldingSpaceModelRestored();
}
HoldingSpaceKeyedServiceDelegate::HoldingSpaceKeyedServiceDelegate(
Profile* profile,
HoldingSpaceModel* model)
: model_(model) {
: profile_(profile), model_(model) {
// It is expected that `profile` already be ready prior to delegate creation.
DCHECK(GetProfileManager()->IsValidProfile(profile));
holding_space_model_observer_.Add(model);
}
......@@ -20,4 +40,6 @@ void HoldingSpaceKeyedServiceDelegate::OnHoldingSpaceItemAdded(
void HoldingSpaceKeyedServiceDelegate::OnHoldingSpaceItemRemoved(
const HoldingSpaceItem* item) {}
void HoldingSpaceKeyedServiceDelegate::OnHoldingSpaceModelRestored() {}
} // namespace ash
......@@ -9,6 +9,8 @@
#include "ash/public/cpp/holding_space/holding_space_model_observer.h"
#include "base/scoped_observer.h"
class Profile;
namespace ash {
// Abstract class for a delegate of `HoldingSpaceKeyedService`. Multiple
......@@ -17,20 +19,42 @@ class HoldingSpaceKeyedServiceDelegate : public HoldingSpaceModelObserver {
public:
~HoldingSpaceKeyedServiceDelegate() override;
// Invoked by `HoldingSpaceKeyedService` to initialize the delegate
// immediately after its construction. Delegates accepting callbacks from
// the service should *not* invoke callbacks during construction but are free
// to do so during or anytime after initialization.
virtual void Init() = 0;
// Invoked by `HoldingSpaceKeyedService` to notify delegates when the holding
// space model has been restored from persistence.
void NotifyHoldingSpaceModelRestored();
protected:
explicit HoldingSpaceKeyedServiceDelegate(HoldingSpaceModel* model);
HoldingSpaceKeyedServiceDelegate(Profile* profile, HoldingSpaceModel* model);
// Returns the `profile_` associated with the `HoldingSpaceKeyedService`.
Profile* profile() { return profile_; }
// Returns the holding space model owned by `HoldingSpaceKeyedService`.
const HoldingSpaceModel* model() const { return model_; }
// Returns if the holding space model is being restored from persistence.
bool is_restoring() const { return is_restoring_; }
private:
// HoldingSpaceModelObserver:
void OnHoldingSpaceItemAdded(const HoldingSpaceItem* item) override;
void OnHoldingSpaceItemRemoved(const HoldingSpaceItem* item) override;
// Owned by `HoldingSpaceKeyedService`.
// Invoked when the holding space model has been restored from persistence.
void OnHoldingSpaceModelRestored();
Profile* const profile_;
const HoldingSpaceModel* const model_;
// If the holding space model is being restored from persistence.
bool is_restoring_ = true;
ScopedObserver<HoldingSpaceModel, HoldingSpaceModelObserver>
holding_space_model_observer_{this};
};
......
......@@ -26,6 +26,7 @@
#include "chrome/browser/chromeos/profiles/profile_helper.h"
#include "chrome/browser/prefs/browser_prefs.h"
#include "chrome/browser/ui/ash/holding_space/holding_space_keyed_service_factory.h"
#include "chrome/browser/ui/ash/holding_space/holding_space_persistence_delegate.h"
#include "chrome/test/base/browser_with_test_window_test.h"
#include "chrome/test/base/testing_profile_manager.h"
#include "components/account_id/account_id.h"
......@@ -436,7 +437,7 @@ TEST_F(HoldingSpaceKeyedServiceTest, UpdatePersistentStorage) {
primary_holding_space_model->AddItem(std::move(holding_space_item));
EXPECT_EQ(*GetProfile()->GetPrefs()->GetList(
HoldingSpaceKeyedService::kPersistencePath),
HoldingSpacePersistenceDelegate::kPersistencePath),
persisted_holding_space_items);
}
......@@ -452,7 +453,7 @@ TEST_F(HoldingSpaceKeyedServiceTest, UpdatePersistentStorage) {
primary_holding_space_model->RemoveItem(holding_space_item->id());
EXPECT_EQ(*GetProfile()->GetPrefs()->GetList(
HoldingSpaceKeyedService::kPersistencePath),
HoldingSpacePersistenceDelegate::kPersistencePath),
persisted_holding_space_items);
}
}
......@@ -514,7 +515,7 @@ TEST_F(HoldingSpaceKeyedServiceTest, RestorePersistentStorage) {
}
pref_store->SetValueSilently(
HoldingSpaceKeyedService::kPersistencePath,
HoldingSpacePersistenceDelegate::kPersistencePath,
std::move(persisted_holding_space_items_before_restoration),
PersistentPrefStore::DEFAULT_PREF_WRITE_FLAGS);
}));
......@@ -546,7 +547,7 @@ TEST_F(HoldingSpaceKeyedServiceTest, RestorePersistentStorage) {
// Verify persisted holding space items.
EXPECT_EQ(*secondary_profile->GetPrefs()->GetList(
HoldingSpaceKeyedService::kPersistencePath),
HoldingSpacePersistenceDelegate::kPersistencePath),
persisted_holding_space_items_after_restoration);
}
......
// 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/ui/ash/holding_space/holding_space_persistence_delegate.h"
#include "ash/public/cpp/holding_space/holding_space_item.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/ash/holding_space/holding_space_util.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/scoped_user_pref_update.h"
namespace ash {
// static
constexpr char HoldingSpacePersistenceDelegate::kPersistencePath[];
HoldingSpacePersistenceDelegate::HoldingSpacePersistenceDelegate(
Profile* profile,
HoldingSpaceModel* model,
ItemRestoredCallback item_restored_callback,
ModelRestoredCallback model_restored_callback)
: HoldingSpaceKeyedServiceDelegate(profile, model),
item_restored_callback_(item_restored_callback),
model_restored_callback_(std::move(model_restored_callback)) {}
HoldingSpacePersistenceDelegate::~HoldingSpacePersistenceDelegate() = default;
// static
void HoldingSpacePersistenceDelegate::RegisterProfilePrefs(
user_prefs::PrefRegistrySyncable* registry) {
registry->RegisterListPref(kPersistencePath);
}
void HoldingSpacePersistenceDelegate::Init() {
// We expect that the associated profile is already ready when we are being
// initialized. That being the case, we can immediately proceed to restore
// the holding space model from persistence storage.
RestoreModelFromPersistence();
}
void HoldingSpacePersistenceDelegate::OnHoldingSpaceItemAdded(
const HoldingSpaceItem* item) {
if (is_restoring())
return;
// `kDownload` type holding space items have their own persistence mechanism.
if (item->type() == HoldingSpaceItem::Type::kDownload)
return;
// Write the new |item| to persistent storage.
ListPrefUpdate update(profile()->GetPrefs(), kPersistencePath);
update->Append(item->Serialize());
}
void HoldingSpacePersistenceDelegate::OnHoldingSpaceItemRemoved(
const HoldingSpaceItem* item) {
if (is_restoring())
return;
// `kDownload` type holding space items have their own persistence mechanism.
if (item->type() == HoldingSpaceItem::Type::kDownload)
return;
// Remove the |item| from persistent storage.
ListPrefUpdate update(profile()->GetPrefs(), kPersistencePath);
update->EraseListValueIf([&item](const base::Value& existing_item) {
return HoldingSpaceItem::DeserializeId(
base::Value::AsDictionaryValue(existing_item)) == item->id();
});
}
// TODO(dmblack): Restore download holding space items.
void HoldingSpacePersistenceDelegate::RestoreModelFromPersistence() {
DCHECK(model()->items().empty());
const auto* persisted_holding_space_items =
profile()->GetPrefs()->GetList(kPersistencePath);
// If persistent storage is empty we can immediately notify the callback of
// model restoration completion and quit early.
if (persisted_holding_space_items->GetList().empty()) {
std::move(model_restored_callback_).Run();
return;
}
std::vector<HoldingSpaceItemPtr> holding_space_items;
for (const auto& persisted_holding_space_item :
persisted_holding_space_items->GetList()) {
holding_space_items.push_back(HoldingSpaceItem::Deserialize(
base::Value::AsDictionaryValue(persisted_holding_space_item),
base::BindOnce(&holding_space_util::ResolveFileSystemUrl,
base::Unretained(profile())),
base::BindOnce(&holding_space_util::ResolveImage)));
}
holding_space_util::PartitionItemsByExistence(
profile(), std::move(holding_space_items),
base::BindOnce(&HoldingSpacePersistenceDelegate::RestoreModelByExistence,
weak_factory_.GetWeakPtr()));
}
void HoldingSpacePersistenceDelegate::RestoreModelByExistence(
std::vector<HoldingSpaceItemPtr> existing_items,
std::vector<HoldingSpaceItemPtr> non_existing_items) {
DCHECK(model()->items().empty());
// Restore existing holding space items.
for (auto& holding_space_item : existing_items)
item_restored_callback_.Run(std::move(holding_space_item));
// Clean up non-existing holding space items from persistence.
if (!non_existing_items.empty()) {
ListPrefUpdate update(profile()->GetPrefs(), kPersistencePath);
update->EraseListValueIf([&non_existing_items](
const base::Value& persisted_item) {
const std::string& persisted_item_id = HoldingSpaceItem::DeserializeId(
base::Value::AsDictionaryValue(persisted_item));
return std::any_of(
non_existing_items.begin(), non_existing_items.end(),
[&persisted_item_id](const HoldingSpaceItemPtr& non_existing_item) {
return non_existing_item->id() == persisted_item_id;
});
});
}
// Notify the callback of model restoration completion.
std::move(model_restored_callback_).Run();
}
} // namespace ash
// 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_UI_ASH_HOLDING_SPACE_HOLDING_SPACE_PERSISTENCE_DELEGATE_H_
#define CHROME_BROWSER_UI_ASH_HOLDING_SPACE_HOLDING_SPACE_PERSISTENCE_DELEGATE_H_
#include <memory>
#include <vector>
#include "base/callback.h"
#include "chrome/browser/ui/ash/holding_space/holding_space_keyed_service_delegate.h"
namespace user_prefs {
class PrefRegistrySyncable;
} // namespace user_prefs
namespace ash {
class HoldingSpaceItem;
using HoldingSpaceItemPtr = std::unique_ptr<HoldingSpaceItem>;
// A delegate of `HoldingSpaceKeyedService` tasked with persistence of holding
// space items. This includes the initial restoration of the model as well as
// incremental updates as items are added/removed at runtime.
class HoldingSpacePersistenceDelegate
: public HoldingSpaceKeyedServiceDelegate {
public:
// Preference path at which holding space items are persisted.
// NOTE: Any changes to persistence must be backwards compatible.
static constexpr char kPersistencePath[] = "ash.holding_space.items";
// Callback to invoke when the specified holding space item has been restored
// from persistence.
using ItemRestoredCallback =
base::RepeatingCallback<void(HoldingSpaceItemPtr)>;
// Callback to invoke when the model has been restored from persistence.
using ModelRestoredCallback = base::OnceClosure;
HoldingSpacePersistenceDelegate(
Profile* profile,
HoldingSpaceModel* model,
ItemRestoredCallback item_restored_callback,
ModelRestoredCallback model_restored_callback);
HoldingSpacePersistenceDelegate(const HoldingSpacePersistenceDelegate&) =
delete;
HoldingSpacePersistenceDelegate& operator=(
const HoldingSpacePersistenceDelegate&) = delete;
~HoldingSpacePersistenceDelegate() override;
// Registers profile preferences for holding space.
static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
private:
// HoldingSpaceKeyedServiceDelegate:
void Init() override;
void OnHoldingSpaceItemAdded(const HoldingSpaceItem* item) override;
void OnHoldingSpaceItemRemoved(const HoldingSpaceItem* item) override;
// Restores the holding space model from persistent storage.
void RestoreModelFromPersistence();
void RestoreModelByExistence(
std::vector<HoldingSpaceItemPtr> existing_items,
std::vector<HoldingSpaceItemPtr> non_existing_items);
// Callback to invoke when an item has been restored from persistence.
ItemRestoredCallback item_restored_callback_;
// Callback to invoke when the model has been restored from persistence.
ModelRestoredCallback model_restored_callback_;
base::WeakPtrFactory<HoldingSpacePersistenceDelegate> weak_factory_{this};
};
} // namespace ash
#endif // CHROME_BROWSER_UI_ASH_HOLDING_SPACE_HOLDING_SPACE_PERSISTENCE_DELEGATE_H_
......@@ -4,11 +4,14 @@
#include "chrome/browser/ui/ash/holding_space/holding_space_util.h"
#include "ash/public/cpp/file_icon_util.h"
#include "ash/public/cpp/holding_space/holding_space_item.h"
#include "base/barrier_closure.h"
#include "base/files/file_path.h"
#include "chrome/browser/chromeos/file_manager/app_id.h"
#include "chrome/browser/chromeos/file_manager/fileapi_util.h"
#include "storage/browser/file_system/file_system_context.h"
#include "url/gurl.h"
namespace ash {
namespace holding_space_util {
......@@ -85,5 +88,20 @@ void PartitionItemsByExistence(Profile* profile,
}
}
GURL ResolveFileSystemUrl(Profile* profile, const base::FilePath& file_path) {
GURL file_system_url;
if (!file_manager::util::ConvertAbsoluteFilePathToFileSystemUrl(
profile, file_path, file_manager::kFileManagerAppId,
&file_system_url)) {
VLOG(2) << "Unable to convert file path to File System URL.";
}
return file_system_url;
}
// TODO(dmblack): Use thumbnail service to asynchronously replace placeholders.
gfx::ImageSkia ResolveImage(const base::FilePath& file_path) {
return GetIconForPath(file_path);
}
} // namespace holding_space_util
} // namespace ash
......@@ -10,8 +10,17 @@
#include "base/callback_forward.h"
class GURL;
class Profile;
namespace base {
class FilePath;
} // namespace base
namespace gfx {
class ImageSkia;
} // namespace gfx
namespace ash {
class HoldingSpaceItem;
......@@ -36,6 +45,12 @@ void PartitionItemsByExistence(Profile* profile,
HoldingSpaceItemPtrList items,
PartitionItemsByExistenceCallback callback);
// Resolves the file system URL associated with the specified `file_path`.
GURL ResolveFileSystemUrl(Profile* profile, const base::FilePath& file_path);
// Resolves the image associated with the specified `file_path`.
gfx::ImageSkia ResolveImage(const base::FilePath& file_path);
} // namespace holding_space_util
} // namespace ash
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment