cros: Change GURL back to std::string in SetOnlineWallpaper
This CL is exactly the revert of the diff between patch 6 and 7 of
CL 1013211:
https://chromium-review.googlesource.com/c/chromium/src/+/1013211/6..7
It was assumed that GURL and std::string are equivalent for this use
case. However GURL converts spaces in urls to "%20", therefore
wallpaper_manager.js fails to use the url to find the currently set
wallpaper. It's observed that this issue is only reproducible for
wallpapers whose urls have space.
Think it makes sense to use std::string since the url is not used for
downloading purpose. (Using url as wallpaper id is not the best
solution IMO, but changing it is a separate task.)
Bug: 846264
Change-Id: Ie83b6d9e95dccbe44dd8fca8c0843a85fcfd9758
Reviewed-on: https://chromium-review.googlesource.com/1072700
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562003}diff --git a/ash/public/interfaces/wallpaper.mojom b/ash/public/interfaces/wallpaper.mojom
index 8bd6c26..dfccafc 100644
--- a/ash/public/interfaces/wallpaper.mojom
+++ b/ash/public/interfaces/wallpaper.mojom
@@ -91,7 +91,7 @@
// called.
// |file_exists|: If the wallpaper file exists in local file system.
SetOnlineWallpaperIfExists(WallpaperUserInfo user_info,
- url.mojom.Url url,
+ string url,
WallpaperLayout layout,
bool preview_mode) => (bool file_exists);
@@ -107,7 +107,7 @@
// called.
SetOnlineWallpaperFromData(WallpaperUserInfo user_info,
string image_data,
- url.mojom.Url url,
+ string url,
WallpaperLayout layout,
bool preview_mode);
@@ -210,10 +210,10 @@
// |wallpaper_files_id|: The file id for user_info.account_id.
RemovePolicyWallpaper(WallpaperUserInfo user_info, string wallpaper_files_id);
- // Returns the file names of the wallpapers that exist in local file system
- // (i.e. |SetOnlineWallpaperFromData| was called earlier). The file name is
- // used as id to identify which wallpapers are available to be set offline.
- GetOfflineWallpaperList() => (array<string> file_names);
+ // Returns the urls of the wallpapers that exist in local file system (i.e.
+ // |SetOnlineWallpaperFromData| was called earlier). The url is used as id
+ // to identify which wallpapers are available to be set offline.
+ GetOfflineWallpaperList() => (array<string> url_list);
// Sets wallpaper animation duration. Passing an empty value disables the
// animation.
diff --git a/ash/wallpaper/wallpaper_controller.cc b/ash/wallpaper/wallpaper_controller.cc
index 54e8e74..3f031b1 100644
--- a/ash/wallpaper/wallpaper_controller.cc
+++ b/ash/wallpaper/wallpaper_controller.cc
@@ -341,7 +341,7 @@
// Returns the file path of the wallpaper corresponding to |url| if it exists in
// local file system, otherwise returns an empty file path.
-base::FilePath GetExistingOnlineWallpaperPath(const GURL& url) {
+base::FilePath GetExistingOnlineWallpaperPath(const std::string& url) {
WallpaperController::WallpaperResolution resolution =
WallpaperController::GetAppropriateResolution();
base::FilePath wallpaper_path =
@@ -361,7 +361,7 @@
// Saves the online wallpaper with both large and small sizes to local file
// system.
-void SaveOnlineWallpaper(const GURL& url,
+void SaveOnlineWallpaper(const std::string& url,
WallpaperLayout layout,
std::unique_ptr<gfx::ImageSkia> image) {
DCHECK(!GlobalChromeOSWallpapersDir().empty());
@@ -385,7 +385,7 @@
// Implementation of |WallpaperController::GetOfflineWallpaper|.
std::vector<std::string> GetOfflineWallpaperListImpl() {
DCHECK(!GlobalChromeOSWallpapersDir().empty());
- std::vector<std::string> file_names;
+ std::vector<std::string> url_list;
if (base::DirectoryExists(GlobalChromeOSWallpapersDir())) {
base::FileEnumerator files(GlobalChromeOSWallpapersDir(),
/*recursive=*/false,
@@ -396,11 +396,11 @@
if (!base::EndsWith(current.BaseName().RemoveExtension().value(),
kSmallWallpaperSuffix,
base::CompareCase::SENSITIVE)) {
- file_names.push_back(current.BaseName().value());
+ url_list.push_back(current.BaseName().value());
}
}
}
- return file_names;
+ return url_list;
}
} // namespace
@@ -486,9 +486,9 @@
// static
base::FilePath WallpaperController::GetOnlineWallpaperPath(
- const GURL& url,
+ const std::string& url,
WallpaperResolution resolution) {
- std::string file_name = url.ExtractFileName();
+ std::string file_name = GURL(url).ExtractFileName();
if (resolution == WALLPAPER_RESOLUTION_SMALL) {
file_name = base::FilePath(file_name)
.InsertBeforeExtension(kSmallWallpaperSuffix)
@@ -1093,7 +1093,7 @@
void WallpaperController::SetOnlineWallpaperIfExists(
mojom::WallpaperUserInfoPtr user_info,
- const GURL& url,
+ const std::string& url,
WallpaperLayout layout,
bool preview_mode,
SetOnlineWallpaperIfExistsCallback callback) {
@@ -1113,7 +1113,7 @@
void WallpaperController::SetOnlineWallpaperFromData(
mojom::WallpaperUserInfoPtr user_info,
const std::string& image_data,
- const GURL& url,
+ const std::string& url,
WallpaperLayout layout,
bool preview_mode) {
DCHECK(Shell::Get()->session_controller()->IsActiveUserSessionStarted());
@@ -1644,12 +1644,12 @@
confirm_preview_wallpaper_callback_ = base::BindOnce(
&WallpaperController::SetOnlineWallpaperImpl,
weak_factory_.GetWeakPtr(), params, image, /*show_wallpaper=*/false);
- reload_preview_wallpaper_callback_ = base::BindRepeating(
- &WallpaperController::ShowWallpaperImage, weak_factory_.GetWeakPtr(),
- image,
- WallpaperInfo{params.url.spec(), params.layout, ONLINE,
- base::Time::Now().LocalMidnight()},
- /*preview_mode=*/true);
+ reload_preview_wallpaper_callback_ =
+ base::BindRepeating(&WallpaperController::ShowWallpaperImage,
+ weak_factory_.GetWeakPtr(), image,
+ WallpaperInfo{params.url, params.layout, ONLINE,
+ base::Time::Now().LocalMidnight()},
+ /*preview_mode=*/true);
// Show the preview wallpaper.
reload_preview_wallpaper_callback_.Run();
} else {
@@ -1661,7 +1661,7 @@
const OnlineWallpaperParams& params,
const gfx::ImageSkia& image,
bool show_wallpaper) {
- WallpaperInfo wallpaper_info = {params.url.spec(), params.layout, ONLINE,
+ WallpaperInfo wallpaper_info = {params.url, params.layout, ONLINE,
base::Time::Now().LocalMidnight()};
if (!SetUserWallpaperInfo(params.account_id, wallpaper_info,
params.is_ephemeral)) {
@@ -1701,9 +1701,8 @@
base::FilePath wallpaper_path;
if (info.type == ONLINE) {
- const GURL url = GURL(info.location);
- DCHECK(url.is_valid());
- wallpaper_path = GetOnlineWallpaperPath(url, GetAppropriateResolution());
+ wallpaper_path =
+ GetOnlineWallpaperPath(info.location, GetAppropriateResolution());
// If the wallpaper exists and it already contains the correct image we
// can return immediately.
diff --git a/ash/wallpaper/wallpaper_controller.h b/ash/wallpaper/wallpaper_controller.h
index 79288b76..a0d4b5a 100644
--- a/ash/wallpaper/wallpaper_controller.h
+++ b/ash/wallpaper/wallpaper_controller.h
@@ -101,7 +101,7 @@
// Returns the path of the online wallpaper corresponding to |url| and
// |resolution|.
- static base::FilePath GetOnlineWallpaperPath(const GURL& url,
+ static base::FilePath GetOnlineWallpaperPath(const std::string& url,
WallpaperResolution resolution);
// Returns wallpaper subdirectory name for current resolution.
@@ -291,13 +291,13 @@
bool preview_mode) override;
void SetOnlineWallpaperIfExists(
mojom::WallpaperUserInfoPtr user_info,
- const GURL& url,
+ const std::string& url,
WallpaperLayout layout,
bool preview_mode,
SetOnlineWallpaperIfExistsCallback callback) override;
void SetOnlineWallpaperFromData(mojom::WallpaperUserInfoPtr user_info,
const std::string& image_data,
- const GURL& url,
+ const std::string& url,
WallpaperLayout layout,
bool preview_mode) override;
void SetDefaultWallpaper(mojom::WallpaperUserInfoPtr user_info,
@@ -389,7 +389,7 @@
struct OnlineWallpaperParams {
AccountId account_id;
bool is_ephemeral;
- GURL url;
+ std::string url;
WallpaperLayout layout;
bool preview_mode;
};
diff --git a/ash/wallpaper/wallpaper_controller_unittest.cc b/ash/wallpaper/wallpaper_controller_unittest.cc
index 4a7e5c26..a784ed68 100644
--- a/ash/wallpaper/wallpaper_controller_unittest.cc
+++ b/ash/wallpaper/wallpaper_controller_unittest.cc
@@ -85,8 +85,8 @@
const std::string wallpaper_files_id_2 = GetDummyFileId(account_id_2);
const std::string file_name_2 = GetDummyFileName(account_id_2);
-const GURL kDummyUrl = GURL("https://best_wallpaper/1");
-const GURL kDummyUrl2 = GURL("https://best_wallpaper/2");
+const std::string kDummyUrl = "https://best_wallpaper/1";
+const std::string kDummyUrl2 = "https://best_wallpaper/2";
const base::FilePath user_data_dir =
base::FilePath(FILE_PATH_LITERAL("user_data"));
@@ -495,7 +495,7 @@
// tests (the connector for the mojo service manager is null).
void SetOnlineWallpaperFromImage(const AccountId& account_id,
const gfx::ImageSkia& image,
- const GURL& url,
+ const std::string& url,
WallpaperLayout layout,
bool save_file,
bool preview_mode) {
@@ -945,8 +945,8 @@
// Verify that there's no offline wallpaper available in the beginning.
std::unique_ptr<base::RunLoop> run_loop = std::make_unique<base::RunLoop>();
controller_->GetOfflineWallpaperList(base::BindLambdaForTesting(
- [&run_loop](const std::vector<std::string>& file_names) {
- EXPECT_TRUE(file_names.empty());
+ [&run_loop](const std::vector<std::string>& url_list) {
+ EXPECT_TRUE(url_list.empty());
run_loop->Quit();
}));
run_loop->Run();
@@ -975,7 +975,7 @@
WallpaperInfo wallpaper_info;
EXPECT_TRUE(controller_->GetUserWallpaperInfo(account_id_1, &wallpaper_info,
false /*is_ephemeral=*/));
- WallpaperInfo expected_wallpaper_info(kDummyUrl.spec(), layout, ONLINE,
+ WallpaperInfo expected_wallpaper_info(kDummyUrl, layout, ONLINE,
base::Time::Now().LocalMidnight());
EXPECT_EQ(wallpaper_info, expected_wallpaper_info);
@@ -1007,9 +1007,9 @@
// file name should not contain the small wallpaper suffix.
run_loop.reset(new base::RunLoop());
controller_->GetOfflineWallpaperList(base::BindLambdaForTesting(
- [&run_loop](const std::vector<std::string>& file_names) {
- EXPECT_EQ(1U, file_names.size());
- EXPECT_EQ(kDummyUrl.ExtractFileName(), file_names[0]);
+ [&run_loop](const std::vector<std::string>& url_list) {
+ EXPECT_EQ(1U, url_list.size());
+ EXPECT_EQ(GURL(kDummyUrl).ExtractFileName(), url_list[0]);
run_loop->Quit();
}));
run_loop->Run();
@@ -1025,7 +1025,7 @@
EXPECT_EQ(0, GetWallpaperCount());
EXPECT_TRUE(controller_->GetUserWallpaperInfo(account_id_1, &wallpaper_info,
false /*is_ephemeral=*/));
- WallpaperInfo expected_wallpaper_info_2(kDummyUrl2.spec(), layout, ONLINE,
+ WallpaperInfo expected_wallpaper_info_2(kDummyUrl2, layout, ONLINE,
base::Time::Now().LocalMidnight());
EXPECT_EQ(wallpaper_info, expected_wallpaper_info_2);
}
@@ -1686,7 +1686,7 @@
EXPECT_TRUE(controller_->GetUserWallpaperInfo(account_id_1, &wallpaper_info,
false /*is_ephemeral=*/));
WallpaperInfo expected_online_wallpaper_info(
- kDummyUrl.spec(), layout, ONLINE, base::Time::Now().LocalMidnight());
+ kDummyUrl, layout, ONLINE, base::Time::Now().LocalMidnight());
EXPECT_EQ(wallpaper_info, expected_online_wallpaper_info);
// Now change the layout of the online wallpaper. Verify that it's a no-op.
@@ -2030,7 +2030,7 @@
EXPECT_EQ(online_wallpaper_color, GetWallpaperColor());
// Verify that the user wallpaper info is now updated to the online wallpaper
// info.
- WallpaperInfo online_wallpaper_info(kDummyUrl.spec(), layout, ONLINE,
+ WallpaperInfo online_wallpaper_info(kDummyUrl, layout, ONLINE,
base::Time::Now().LocalMidnight());
EXPECT_TRUE(controller_->GetUserWallpaperInfo(
account_id_1, &user_wallpaper_info, false /*is_ephemeral=*/));
@@ -2211,7 +2211,7 @@
EXPECT_EQ(0, GetWallpaperCount());
EXPECT_EQ(kWallpaperColor, GetWallpaperColor());
// However, the user wallpaper info should already be updated to the new info.
- WallpaperInfo synced_online_wallpaper_info(kDummyUrl2.spec(), layout, ONLINE,
+ WallpaperInfo synced_online_wallpaper_info(kDummyUrl2, layout, ONLINE,
base::Time::Now().LocalMidnight());
EXPECT_TRUE(controller_->GetUserWallpaperInfo(
account_id_1, &user_wallpaper_info, false /*is_ephemeral=*/));
diff --git a/chrome/browser/chromeos/extensions/wallpaper_private_api.cc b/chrome/browser/chromeos/extensions/wallpaper_private_api.cc
index f07a4ed..a6033da 100644
--- a/chrome/browser/chromeos/extensions/wallpaper_private_api.cc
+++ b/chrome/browser/chromeos/extensions/wallpaper_private_api.cc
@@ -340,8 +340,7 @@
EXTENSION_FUNCTION_VALIDATE(params);
WallpaperControllerClient::Get()->SetOnlineWallpaperIfExists(
- GetUserFromBrowserContext(browser_context())->GetAccountId(),
- GURL(params->url),
+ GetUserFromBrowserContext(browser_context())->GetAccountId(), params->url,
wallpaper_api_util::GetLayoutEnum(
wallpaper_base::ToString(params->layout)),
params->preview_mode,
@@ -379,7 +378,7 @@
WallpaperControllerClient::Get()->SetOnlineWallpaperFromData(
GetUserFromBrowserContext(browser_context())->GetAccountId(),
std::string(params->wallpaper.begin(), params->wallpaper.end()),
- GURL(params->url),
+ params->url,
wallpaper_api_util::GetLayoutEnum(
wallpaper_base::ToString(params->layout)),
params->preview_mode);
@@ -642,9 +641,9 @@
}
void WallpaperPrivateGetOfflineWallpaperListFunction::
- OnOfflineWallpaperListReturned(const std::vector<std::string>& file_names) {
+ OnOfflineWallpaperListReturned(const std::vector<std::string>& url_list) {
auto results = std::make_unique<base::ListValue>();
- results->AppendStrings(file_names);
+ results->AppendStrings(url_list);
Respond(OneArgument(std::move(results)));
}
diff --git a/chrome/browser/chromeos/extensions/wallpaper_private_api.h b/chrome/browser/chromeos/extensions/wallpaper_private_api.h
index 6e12e17..21cbed5 100644
--- a/chrome/browser/chromeos/extensions/wallpaper_private_api.h
+++ b/chrome/browser/chromeos/extensions/wallpaper_private_api.h
@@ -249,8 +249,7 @@
private:
// Responds with the list of urls.
- void OnOfflineWallpaperListReturned(
- const std::vector<std::string>& file_names);
+ void OnOfflineWallpaperListReturned(const std::vector<std::string>& url_list);
DISALLOW_COPY_AND_ASSIGN(WallpaperPrivateGetOfflineWallpaperListFunction);
};
diff --git a/chrome/browser/ui/ash/test_wallpaper_controller.cc b/chrome/browser/ui/ash/test_wallpaper_controller.cc
index aef6f23..e106dcc66 100644
--- a/chrome/browser/ui/ash/test_wallpaper_controller.cc
+++ b/chrome/browser/ui/ash/test_wallpaper_controller.cc
@@ -50,7 +50,7 @@
void TestWallpaperController::SetOnlineWallpaperIfExists(
ash::mojom::WallpaperUserInfoPtr user_info,
- const GURL& url,
+ const std::string& url,
ash::WallpaperLayout layout,
bool preview_mode,
ash::mojom::WallpaperController::SetOnlineWallpaperIfExistsCallback
@@ -61,7 +61,7 @@
void TestWallpaperController::SetOnlineWallpaperFromData(
ash::mojom::WallpaperUserInfoPtr user_info,
const std::string& image_data,
- const GURL& url,
+ const std::string& url,
ash::WallpaperLayout layout,
bool preview_mode) {
NOTIMPLEMENTED();
diff --git a/chrome/browser/ui/ash/test_wallpaper_controller.h b/chrome/browser/ui/ash/test_wallpaper_controller.h
index 174df350..d5267a4f 100644
--- a/chrome/browser/ui/ash/test_wallpaper_controller.h
+++ b/chrome/browser/ui/ash/test_wallpaper_controller.h
@@ -49,14 +49,14 @@
bool preview_mode) override;
void SetOnlineWallpaperIfExists(
ash::mojom::WallpaperUserInfoPtr user_info,
- const GURL& url,
+ const std::string& url,
ash::WallpaperLayout layout,
bool preview_mode,
ash::mojom::WallpaperController::SetOnlineWallpaperIfExistsCallback
callback) override;
void SetOnlineWallpaperFromData(ash::mojom::WallpaperUserInfoPtr user_info,
const std::string& image_data,
- const GURL& url,
+ const std::string& url,
ash::WallpaperLayout layout,
bool preview_mode) override;
void SetDefaultWallpaper(ash::mojom::WallpaperUserInfoPtr user_info,
diff --git a/chrome/browser/ui/ash/wallpaper_controller_client.cc b/chrome/browser/ui/ash/wallpaper_controller_client.cc
index 07e9142b..28348b7 100644
--- a/chrome/browser/ui/ash/wallpaper_controller_client.cc
+++ b/chrome/browser/ui/ash/wallpaper_controller_client.cc
@@ -194,7 +194,7 @@
void WallpaperControllerClient::SetOnlineWallpaperIfExists(
const AccountId& account_id,
- const GURL& url,
+ const std::string& url,
ash::WallpaperLayout layout,
bool preview_mode,
ash::mojom::WallpaperController::SetOnlineWallpaperIfExistsCallback
@@ -210,7 +210,7 @@
void WallpaperControllerClient::SetOnlineWallpaperFromData(
const AccountId& account_id,
const std::string& image_data,
- const GURL& url,
+ const std::string& url,
ash::WallpaperLayout layout,
bool preview_mode) {
ash::mojom::WallpaperUserInfoPtr user_info =
diff --git a/chrome/browser/ui/ash/wallpaper_controller_client.h b/chrome/browser/ui/ash/wallpaper_controller_client.h
index 611cad7..4482e15 100644
--- a/chrome/browser/ui/ash/wallpaper_controller_client.h
+++ b/chrome/browser/ui/ash/wallpaper_controller_client.h
@@ -39,14 +39,14 @@
bool preview_mode);
void SetOnlineWallpaperIfExists(
const AccountId& account_id,
- const GURL& url,
+ const std::string& url,
ash::WallpaperLayout layout,
bool preview_mode,
ash::mojom::WallpaperController::SetOnlineWallpaperIfExistsCallback
callback);
void SetOnlineWallpaperFromData(const AccountId& account_id,
const std::string& image_data,
- const GURL& url,
+ const std::string& url,
ash::WallpaperLayout layout,
bool preview_mode);
void SetDefaultWallpaper(const AccountId& account_id, bool show_wallpaper);
