Skip to content

Commit

Permalink
Support to upload multiple custom images for NTP background (#15311)
Browse files Browse the repository at this point in the history
* Support to upload multiple custom images for NTP background

This PR allows users to upload multiple images for NTP background.

* The maximum number of images is 24
* These images are stored in a dedicated directory.
* Users can select one of these or toggle on "Random" button.
* Users also can remove these images.
  • Loading branch information
sangwoo108 authored Oct 4, 2022
1 parent 90d9dd6 commit 66a2993
Show file tree
Hide file tree
Showing 32 changed files with 506 additions and 110 deletions.
10 changes: 9 additions & 1 deletion browser/ntp_background/custom_background_file_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,14 @@ void CustomBackgroundFileManager::MoveImage(
MakeSureDirExists(std::move(on_check_dir));
}

void CustomBackgroundFileManager::RemoveImage(
const base::FilePath& file_path,
base::OnceCallback<void(bool /*result*/)> callback) {
base::ThreadPool::PostTaskAndReplyWithResult(
FROM_HERE, {base::MayBlock()},
base::BindOnce(base::DeleteFile, file_path), std::move(callback));
}

base::FilePath CustomBackgroundFileManager::GetCustomBackgroundDirectory()
const {
return profile_->GetPath().AppendASCII(
Expand Down Expand Up @@ -191,7 +199,7 @@ void CustomBackgroundFileManager::SaveImageAsPNG(
base::FilePath modified_path = target_path;
for (int i = 1; base::PathExists(modified_path); ++i) {
modified_path = target_path.InsertBeforeExtensionASCII(
base::StringPrintf("(%d)", i));
base::StringPrintf("-%d", i));
}

if (!base::WriteFile(
Expand Down
129 changes: 128 additions & 1 deletion browser/ntp_background/custom_background_file_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,21 @@

#include <memory>
#include <string>
#include <type_traits>

#include "base/callback.h"
#include "base/files/file_path.h"
#include "base/memory/ref_counted_memory.h"
#include "base/memory/weak_ptr.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "brave/components/ntp_background_images/browser/url_constants.h"
#include "url/gurl.h"
#include "url/url_util.h"

#if defined(OS_WIN)
#include "base/strings/sys_string_conversions.h"
#endif

namespace base {
class FilePath;
Expand All @@ -32,10 +43,124 @@ class Profile;
// * Manages custom images
// * Have a list of custom images in Prefs.
// * Make it sure that we have local files mapped to entries.
// * Deregisters custom image(TODO)
// * Deregisters custom image
// * remove the local file.
class CustomBackgroundFileManager final {
public:
// |Converter| is a convience class to convert values to and from each layer.
//
// [Web UI] - GURL with percent encoded path.
// || The path value should be same with the value in PrefService
// (path)
// ||
// (value)
// ||
// [PrefService] - std::string. The value is a file name. Encoding is
// || usually UTF8, but on some platforms it might not be
// (value) specified.
// ||
// (file name)
// ||
// [File System] - base::FilePath. On Windows, it uses wide string.
// and on other platforms, uses string but encoding might
// not be specified.
//
// This class is designed to be used for a short time and then destroyed.
// Don't pass this class around code base. Use this just right where you
// need to convert values.
//
// example:
// auto file_path = Converter(url, file_manager).To<base::FilePath>();
// auto url = Converter(prefs_value).To<GURL>();
//
template <class FromT>
class Converter final {
public:
explicit Converter(const FromT& value,
CustomBackgroundFileManager* file_manager = nullptr)
: file_manager_(file_manager) {
if constexpr (std::is_same_v<FromT, std::string>) {
DCHECK(!base::StartsWith(value,
ntp_background_images::kCustomWallpaperURL))
<< "URLs should be passed in as a GURL";
value_ = value;
} else if constexpr (std::is_same_v<FromT, GURL>) {
// GURL(webui data url) -> std::string(prefs value)
// When GURL is given, its path is percent encoded pref name. So
// decode it and create Converter with it.
DCHECK(value.SchemeIs("chrome") &&
value.host() == ntp_background_images::kCustomWallpaperHost)
<< "Not a custom wallpaper URL";

// remove leading slash
const auto path = value.path().substr(1);
DCHECK(!path.empty()) << "URL path is empty " << value;
url::RawCanonOutputT<char16_t> decoded_value;
url::DecodeURLEscapeSequences(path.data(), path.length(),
url::DecodeURLMode::kUTF8OrIsomorphic,
&decoded_value);
value_ = base::UTF16ToUTF8(
std::u16string(decoded_value.data(), decoded_value.length()));
} else {
// FilePath(local file path) -> std::string(prefs value)
static_assert(std::is_same_v<FromT, base::FilePath>,
"FromT must be one of std::string, GURL, base::FilePath");

// When base::FilePath is given, its file name is used as pref value.
// But the file path's underlying type and encoding is platform
// dependent. So, extract file name and convert it to UTF8 if needed.
#if defined(OS_WIN)
auto file_name = base::SysWideToUTF8(value.BaseName().value());
#else
auto file_name = std::string(value.BaseName().value().c_str());
#endif
DCHECK(!file_name.empty())
<< "Couldn't extract file name from the given path " << value;
value_ = file_name;
}
}
Converter(const Converter&) = delete;
Converter& operator=(const Converter&) = delete;
Converter(Converter&&) noexcept = delete;
Converter& operator=(Converter&&) noexcept = delete;
~Converter() = default;

// Converter functions. Not allowing to convert to what it was created from.
template <class ToT>
[[nodiscard]] std::enable_if_t<!std::is_same_v<FromT, ToT>, ToT> To()
const&& {
if constexpr (std::is_same_v<ToT, std::string>) {
return value_;
} else if constexpr (std::is_same_v<ToT, GURL>) {
// std::string(pref_value) -> GURL(webui data url)
// Do percent encoding and compose it with base url so that it can
// be used as webui data url.
url::RawCanonOutputT<char> encoded;
url::EncodeURIComponent(value_.c_str(), value_.length(), &encoded);
return GURL(ntp_background_images::kCustomWallpaperURL +
std::string(encoded.data(), encoded.length()));
} else {
static_assert(std::is_same_v<ToT, base::FilePath>,
"ToT must be one of std::string, GURL, base::FilePath");
// std::string(pref_value) -> base::FilePath(local file path)
DCHECK(file_manager_) << "Converting to local file path requires "
"CustomBackgroundFileManager";
base::FilePath file_path =
file_manager_->GetCustomBackgroundDirectory();
#if defined(OS_WIN)
file_path = file_path.Append(base::SysUTF8ToWide(value_));
#else
file_path = file_path.Append(value_);
#endif
return file_path;
}
}

private:
raw_ptr<CustomBackgroundFileManager> file_manager_ = nullptr;
std::string value_;
};

using SaveFileCallback = base::OnceCallback<void(const base::FilePath&)>;

explicit CustomBackgroundFileManager(Profile* profile);
Expand All @@ -48,6 +173,8 @@ class CustomBackgroundFileManager final {
SaveFileCallback callback);
void MoveImage(const base::FilePath& source_file_path,
base::OnceCallback<void(bool /*result*/)> callback);
void RemoveImage(const base::FilePath& file_path,
base::OnceCallback<void(bool /*result*/)> callback);

base::FilePath GetCustomBackgroundDirectory() const;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ IN_PROC_BROWSER_TEST_F(CustomBackgroundFileManagerBrowserTest,
kTestImageName);
if (i > 0) {
expected_path = expected_path.InsertBeforeExtensionASCII(
base::StringPrintf("(%d)", i));
base::StringPrintf("-%d", i));
}

auto check_res =
Expand Down
8 changes: 8 additions & 0 deletions browser/ntp_background/ntp_background_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <utility>

#include "base/notreached.h"
#include "base/ranges/algorithm.h"
#include "brave/components/constants/pref_names.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_service.h"
Expand Down Expand Up @@ -135,6 +136,13 @@ void NTPBackgroundPrefs::AddCustomImageToList(const std::string& file_name) {
update->GetList().Append(file_name);
}

void NTPBackgroundPrefs::RemoveCustomImageFromList(
const std::string& file_name) {
ListPrefUpdate update(service_, NTPBackgroundPrefs::kCustomImageListPrefName);
auto& list = update->GetList();
list.erase(base::ranges::remove(update->GetList(), file_name), list.end());
}

std::vector<std::string> NTPBackgroundPrefs::GetCustomImageList() const {
const auto* list = service_->GetList(kCustomImageListPrefName);
std::vector<std::string> result;
Expand Down
1 change: 1 addition & 0 deletions browser/ntp_background/ntp_background_prefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ class NTPBackgroundPrefs final {
absl::variant<GURL, std::string> GetSelectedValue() const;

void AddCustomImageToList(const std::string& file_name);
void RemoveCustomImageFromList(const std::string& file_name);
std::vector<std::string> GetCustomImageList() const;

private:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "brave/components/constants/pref_names.h"
#include "brave/components/ntp_background_images/browser/ntp_background_images_data.h"
#include "brave/components/ntp_background_images/browser/ntp_background_images_service.h"
#include "brave/components/ntp_background_images/browser/url_constants.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/common/pref_names.h"
#include "chrome/common/url_constants.h"
Expand Down Expand Up @@ -79,28 +80,20 @@ bool NTPCustomBackgroundImagesServiceDelegate::IsCustomImageBackgroundEnabled()
return NTPBackgroundPrefs(profile_->GetPrefs()).IsCustomImageType();
}

base::FilePath NTPCustomBackgroundImagesServiceDelegate::
GetCustomBackgroundImageLocalFilePath() const {
if (!IsCustomImageBackgroundEnabled())
return base::FilePath();
base::FilePath
NTPCustomBackgroundImagesServiceDelegate::GetCustomBackgroundImageLocalFilePath(
const GURL& url) const {
return CustomBackgroundFileManager::Converter(url, file_manager_.get())
.To<base::FilePath>();
}

auto value = NTPBackgroundPrefs(profile_->GetPrefs()).GetSelectedValue();
if (!absl::holds_alternative<std::string>(value)) {
// This can happen during migration.
return base::FilePath();
}
GURL NTPCustomBackgroundImagesServiceDelegate::GetCustomBackgroundImageURL()
const {
DCHECK(IsCustomImageBackgroundEnabled());

#if defined(OS_WIN)
// On Windows path is wchar type and we should convert it to utf8.
// So we suppose |value| is utf8.
const auto file_name =
base::FilePath::FromUTF8Unsafe(absl::get<std::string>(value)).value();
#else
// On other platform, path's encoding is not specified, and we store value
// as it was given.
const auto file_name = absl::get<std::string>(value);
#endif
return file_manager_->GetCustomBackgroundDirectory().Append(file_name);
auto prefs = NTPBackgroundPrefs(profile_->GetPrefs());
auto name = absl::get<std::string>(prefs.GetSelectedValue());
return CustomBackgroundFileManager::Converter(name).To<GURL>();
}

bool NTPCustomBackgroundImagesServiceDelegate::IsColorBackgroundEnabled()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ class NTPCustomBackgroundImagesServiceDelegate

// NTPCustomBackgroundImagesService::Delegate overrides:
bool IsCustomImageBackgroundEnabled() const override;
base::FilePath GetCustomBackgroundImageLocalFilePath() const override;
base::FilePath GetCustomBackgroundImageLocalFilePath(
const GURL& url) const override;
GURL GetCustomBackgroundImageURL() const override;
bool IsColorBackgroundEnabled() const override;
std::string GetColor() const override;
bool ShouldUseRandomValue() const override;
Expand Down
Loading

0 comments on commit 66a2993

Please sign in to comment.