Use a more common and easily parsed format for chrome://fileicon
Instead of using custom dir/like/things, let's use the more standard:
?param1=val1¶m2=val2
Because it allows us to use URLSearchParams[1] in JS:
url.searchParams.set('param1', 'val1');
And net::QueryIterator in C++:
for (net::QueryIterator it(url); !it.IsAtEnd(); it.Advance()) {
// use it.GetKey() and it.GetValue()
}
R=dpapad@chromium.org
BUG=953962
[1] https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams
Change-Id: I708ae3349d826793b5d1dbd6e1c1683e08ce254e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1574373
Commit-Queue: Dan Beam <dbeam@chromium.org>
Auto-Submit: Dan Beam <dbeam@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652739}diff --git a/chrome/browser/ui/webui/fileicon_source.cc b/chrome/browser/ui/webui/fileicon_source.cc
index a7d16dd..828458e 100644
--- a/chrome/browser/ui/webui/fileicon_source.cc
+++ b/chrome/browser/ui/webui/fileicon_source.cc
@@ -11,7 +11,9 @@
#include "base/strings/string_split.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/browser_process.h"
+#include "chrome/common/webui_url_constants.h"
#include "net/base/escape.h"
+#include "net/base/url_util.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/base/webui/web_ui_util.h"
#include "ui/gfx/codec/png_codec.h"
@@ -29,27 +31,12 @@
// URL parameter specifying icon size.
const char kIconSizeParameter[] = "iconsize";
+// URL parameter specifying the file path for which to get an icon.
+const char kPathParameter[] = "path";
+
// URL parameter specifying scale factor.
const char kScaleFactorParameter[] = "scale";
-// Assuming the url is of the form '/path?query', convert the path portion into
-// a FilePath and return the resulting |file_path| and |query|. The path
-// portion may have been encoded using encodeURIComponent().
-void GetFilePathAndQuery(const std::string& url,
- base::FilePath* file_path,
- std::string* query) {
- // We receive the url with chrome://fileicon/ stripped but GURL expects it.
- const GURL gurl("chrome://fileicon/" + url);
- std::string path = net::UnescapeURLComponent(
- gurl.path().substr(1),
- net::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS |
- net::UnescapeRule::PATH_SEPARATORS | net::UnescapeRule::SPACES);
-
- *file_path = base::FilePath::FromUTF8Unsafe(path);
- *file_path = file_path->NormalizePathSeparators();
- query->assign(gurl.query());
-}
-
IconLoader::IconSize SizeStringToIconSize(const std::string& size_string) {
if (size_string == "small") return IconLoader::SMALL;
if (size_string == "large") return IconLoader::LARGE;
@@ -58,22 +45,21 @@
return IconLoader::NORMAL;
}
-// Simple parser for data on the query.
-void ParseQueryParams(const std::string& query,
+void ParseQueryParams(const std::string& path,
+ base::FilePath* file_path,
float* scale_factor,
IconLoader::IconSize* icon_size) {
- base::StringPairs parameters;
- if (icon_size)
- *icon_size = IconLoader::NORMAL;
- if (scale_factor)
- *scale_factor = 1.0f;
- base::SplitStringIntoKeyValuePairs(query, '=', '&', ¶meters);
- for (base::StringPairs::const_iterator iter = parameters.begin();
- iter != parameters.end(); ++iter) {
- if (icon_size && iter->first == kIconSizeParameter)
- *icon_size = SizeStringToIconSize(iter->second);
- else if (scale_factor && iter->first == kScaleFactorParameter)
- webui::ParseScaleFactor(iter->second, scale_factor);
+ GURL request = GURL(chrome::kChromeUIFileiconURL).Resolve(path);
+ for (net::QueryIterator it(request); !it.IsAtEnd(); it.Advance()) {
+ std::string key = it.GetKey();
+ if (key == kPathParameter) {
+ *file_path = base::FilePath::FromUTF8Unsafe(it.GetUnescapedValue())
+ .NormalizePathSeparators();
+ } else if (key == kIconSizeParameter) {
+ *icon_size = SizeStringToIconSize(it.GetValue());
+ } else if (key == kScaleFactorParameter) {
+ webui::ParseScaleFactor(it.GetValue(), scale_factor);
+ }
}
}
@@ -127,15 +113,13 @@
}
void FileIconSource::StartDataRequest(
- const std::string& url_path,
+ const std::string& path,
const content::ResourceRequestInfo::WebContentsGetter& wc_getter,
const content::URLDataSource::GotDataCallback& callback) {
- std::string query;
base::FilePath file_path;
- IconLoader::IconSize icon_size;
+ IconLoader::IconSize icon_size = IconLoader::NORMAL;
float scale_factor = 1.0f;
- GetFilePathAndQuery(url_path, &file_path, &query);
- ParseQueryParams(query, &scale_factor, &icon_size);
+ ParseQueryParams(path, &file_path, &scale_factor, &icon_size);
FetchFileIcon(file_path, scale_factor, icon_size, callback);
}
diff --git a/chrome/browser/ui/webui/fileicon_source_unittest.cc b/chrome/browser/ui/webui/fileicon_source_unittest.cc
index 83d32c9..42dd71a 100644
--- a/chrome/browser/ui/webui/fileicon_source_unittest.cc
+++ b/chrome/browser/ui/webui/fileicon_source_unittest.cc
@@ -33,10 +33,6 @@
public:
FileIconSourceTest() = default;
- static TestFileIconSource* CreateFileIconSource() {
- return new TestFileIconSource();
- }
-
private:
content::TestBrowserThreadBundle test_browser_thread_bundle_;
};
@@ -47,48 +43,56 @@
float scale_factor;
IconLoader::IconSize size;
} kBasicExpectations[] = {
- { "foo?bar", FILE_PATH_LITERAL("foo"), 1.0f, IconLoader::NORMAL },
- { "foo?bar&scale=2x", FILE_PATH_LITERAL("foo"), 2.0f, IconLoader::NORMAL },
- { "foo?iconsize=small", FILE_PATH_LITERAL("foo"), 1.0f, IconLoader::SMALL },
- { "foo?iconsize=normal", FILE_PATH_LITERAL("foo"), 1.0f, IconLoader::NORMAL },
- { "foo?iconsize=large", FILE_PATH_LITERAL("foo"), 1.0f, IconLoader::LARGE },
- { "foo?iconsize=asdf", FILE_PATH_LITERAL("foo"), 1.0f, IconLoader::NORMAL },
- { "foo?blah=b&iconsize=small", FILE_PATH_LITERAL("foo"), 1.0f,
- IconLoader::SMALL },
- { "foo?blah&iconsize=small", FILE_PATH_LITERAL("foo"), 1.0f,
- IconLoader::SMALL },
- { "a%3Fb%26c%3Dd.txt?iconsize=small", FILE_PATH_LITERAL("a?b&c=d.txt"), 1.0f,
- IconLoader::SMALL },
- { "a%3Ficonsize%3Dsmall?iconsize=large",
- FILE_PATH_LITERAL("a?iconsize=small"), 1.0f, IconLoader::LARGE },
- { "o%40%23%24%25%26*()%20%2B%3D%3F%2C%3A%3B%22.jpg",
- FILE_PATH_LITERAL("o@#$%&*() +=?,:;\".jpg"), 1.0f, IconLoader::NORMAL },
+ {"?path=foo&bar", FILE_PATH_LITERAL("foo"), 1.0f, IconLoader::NORMAL},
+ {"?path=foo&bar&scale=2x", FILE_PATH_LITERAL("foo"), 2.0f,
+ IconLoader::NORMAL},
+ {"?path=foo&iconsize=small", FILE_PATH_LITERAL("foo"), 1.0f,
+ IconLoader::SMALL},
+ {"?path=foo&iconsize=normal", FILE_PATH_LITERAL("foo"), 1.0f,
+ IconLoader::NORMAL},
+ {"?path=foo&iconsize=large", FILE_PATH_LITERAL("foo"), 1.0f,
+ IconLoader::LARGE},
+ {"?path=foo&iconsize=asdf", FILE_PATH_LITERAL("foo"), 1.0f,
+ IconLoader::NORMAL},
+ {"?path=foo&blah=b&iconsize=small", FILE_PATH_LITERAL("foo"), 1.0f,
+ IconLoader::SMALL},
+ {"?path=foo&blah&iconsize=small", FILE_PATH_LITERAL("foo"), 1.0f,
+ IconLoader::SMALL},
+ {"?path=a%3Fb%26c%3Dd.txt&iconsize=small", FILE_PATH_LITERAL("a?b&c=d.txt"),
+ 1.0f, IconLoader::SMALL},
+ {"?path=a%3Ficonsize%3Dsmall&iconsize=large",
+ FILE_PATH_LITERAL("a?iconsize=small"), 1.0f, IconLoader::LARGE},
+ {"?path=o%40%23%24%25%26*()%20%2B%3D%3F%2C%3A%3B%22.jpg",
+ FILE_PATH_LITERAL("o@#$%&*() +=?,:;\".jpg"), 1.0f, IconLoader::NORMAL},
#if defined(OS_WIN)
- { "c:/foo/bar/baz", FILE_PATH_LITERAL("c:\\foo\\bar\\baz"), 1.0f,
- IconLoader::NORMAL },
- { "/foo?bar=asdf&asdf", FILE_PATH_LITERAL("\\foo"), 1.0f,
- IconLoader::NORMAL },
- { "c%3A%2Fusers%2Ffoo%20user%2Fbar.txt",
- FILE_PATH_LITERAL("c:\\users\\foo user\\bar.txt"), 1.0f,
- IconLoader::NORMAL },
- { "c%3A%2Fusers%2F%C2%A9%202000.pdf",
- FILE_PATH_LITERAL("c:\\users\\\xa9 2000.pdf"), 1.0f, IconLoader::NORMAL },
- { "%E0%B6%9A%E0%B6%BB%E0%B7%9D%E0%B6%B8%E0%B7%8A",
- FILE_PATH_LITERAL("\x0d9a\x0dbb\x0ddd\x0db8\x0dca"), 1.0f,
- IconLoader::NORMAL },
- { "%2Ffoo%2Fbar", FILE_PATH_LITERAL("\\foo\\bar"), 1.0f, IconLoader::NORMAL },
- { "%2Fbaz%20(1).txt?iconsize=small", FILE_PATH_LITERAL("\\baz (1).txt"),
- 1.0f, IconLoader::SMALL },
+ {"?path=c%3A%2Ffoo%2Fbar%2Fbaz", FILE_PATH_LITERAL("c:\\foo\\bar\\baz"),
+ 1.0f, IconLoader::NORMAL},
+ {"?path=%2Ffoo&bar=asdf&asdf", FILE_PATH_LITERAL("\\foo"), 1.0f,
+ IconLoader::NORMAL},
+ {"?path=c%3A%2Fusers%2Ffoo%20user%2Fbar.txt",
+ FILE_PATH_LITERAL("c:\\users\\foo user\\bar.txt"), 1.0f,
+ IconLoader::NORMAL},
+ {"?path=c%3A%2Fusers%2F%C2%A9%202000.pdf",
+ FILE_PATH_LITERAL("c:\\users\\\xa9 2000.pdf"), 1.0f, IconLoader::NORMAL},
+ {"?path=%E0%B6%9A%E0%B6%BB%E0%B7%9D%E0%B6%B8%E0%B7%8A",
+ FILE_PATH_LITERAL("\x0d9a\x0dbb\x0ddd\x0db8\x0dca"), 1.0f,
+ IconLoader::NORMAL},
+ {"?path=%2Ffoo%2Fbar", FILE_PATH_LITERAL("\\foo\\bar"), 1.0f,
+ IconLoader::NORMAL},
+ {"?path=%2Fbaz%20(1).txt&iconsize=small",
+ FILE_PATH_LITERAL("\\baz (1).txt"), 1.0f, IconLoader::SMALL},
#else
- { "/foo/bar/baz", FILE_PATH_LITERAL("/foo/bar/baz"), 1.0f,
- IconLoader::NORMAL },
- { "/foo?bar", FILE_PATH_LITERAL("/foo"), 1.0f, IconLoader::NORMAL },
- { "%2Ffoo%2f%E0%B6%9A%E0%B6%BB%E0%B7%9D%E0%B6%B8%E0%B7%8A",
- FILE_PATH_LITERAL("/foo/\xe0\xb6\x9a\xe0\xb6\xbb\xe0\xb7\x9d")
- FILE_PATH_LITERAL("\xe0\xb6\xb8\xe0\xb7\x8a"), 1.0f, IconLoader::NORMAL },
- { "%2Ffoo%2Fbar", FILE_PATH_LITERAL("/foo/bar"), 1.0f, IconLoader::NORMAL },
- { "%2Fbaz%20(1).txt?iconsize=small", FILE_PATH_LITERAL("/baz (1).txt"), 1.0f,
- IconLoader::SMALL },
+ {"?path=%2Ffoo%2Fbar%2Fbaz", FILE_PATH_LITERAL("/foo/bar/baz"), 1.0f,
+ IconLoader::NORMAL},
+ {"?path=%2Ffoo&bar", FILE_PATH_LITERAL("/foo"), 1.0f, IconLoader::NORMAL},
+ {"?path=%2Ffoo%2f%E0%B6%9A%E0%B6%BB%E0%B7%9D%E0%B6%B8%E0%B7%8A",
+ FILE_PATH_LITERAL("/foo/\xe0\xb6\x9a\xe0\xb6\xbb\xe0\xb7\x9d")
+ FILE_PATH_LITERAL("\xe0\xb6\xb8\xe0\xb7\x8a"),
+ 1.0f, IconLoader::NORMAL},
+ {"?path=%2Ffoo%2Fbar", FILE_PATH_LITERAL("/foo/bar"), 1.0f,
+ IconLoader::NORMAL},
+ {"?path=%2Fbaz%20(1).txt&iconsize=small", FILE_PATH_LITERAL("/baz (1).txt"),
+ 1.0f, IconLoader::SMALL},
#endif
};
@@ -107,7 +111,7 @@
supported_scale_factors);
for (unsigned i = 0; i < base::size(kBasicExpectations); i++) {
- std::unique_ptr<TestFileIconSource> source(CreateFileIconSource());
+ auto source = std::make_unique<TestFileIconSource>();
content::URLDataSource::GotDataCallback callback;
EXPECT_CALL(*source.get(),
FetchFileIcon(
diff --git a/chrome/common/webui_url_constants.cc b/chrome/common/webui_url_constants.cc
index 2a017d96..9924ff3 100644
--- a/chrome/common/webui_url_constants.cc
+++ b/chrome/common/webui_url_constants.cc
@@ -68,6 +68,7 @@
const char kChromeUIExtensionsURL[] = "chrome://extensions/";
const char kChromeUIFaviconHost[] = "favicon";
const char kChromeUIFaviconURL[] = "chrome://favicon/";
+const char kChromeUIFileiconURL[] = "chrome://fileicon/";
const char kChromeUIFlagsHost[] = "flags";
const char kChromeUIFlagsURL[] = "chrome://flags/";
const char kChromeUIGCMInternalsHost[] = "gcm-internals";
diff --git a/chrome/common/webui_url_constants.h b/chrome/common/webui_url_constants.h
index a3c406b..488d5c99 100644
--- a/chrome/common/webui_url_constants.h
+++ b/chrome/common/webui_url_constants.h
@@ -73,6 +73,7 @@
extern const char kChromeUIExtensionsURL[];
extern const char kChromeUIFaviconHost[];
extern const char kChromeUIFaviconURL[];
+extern const char kChromeUIFileiconURL[];
extern const char kChromeUIFlagsHost[];
extern const char kChromeUIFlagsURL[];
extern const char kChromeUIGCMInternalsHost[];
diff --git a/chrome/test/data/webui/icon_test.html b/chrome/test/data/webui/icon_test.html
index bd5d15e..555a61a 100644
--- a/chrome/test/data/webui/icon_test.html
+++ b/chrome/test/data/webui/icon_test.html
@@ -35,7 +35,7 @@
function testGetFileIconUrl() {
assertEquals(cr.icon.getFileIconUrl('file path'),
- 'chrome://fileicon/file%20path?scale=' +
+ 'chrome://fileicon/?path=file+path&scale=' +
window.devicePixelRatio + 'x');
}
diff --git a/ui/webui/resources/js/icon.js b/ui/webui/resources/js/icon.js
index 094eae8..eac95d4 100644
--- a/ui/webui/resources/js/icon.js
+++ b/ui/webui/resources/js/icon.js
@@ -36,8 +36,10 @@
* @return {string}
*/
function getFileIconUrl(filePath) {
- return 'chrome://fileicon/' + encodeURIComponent(filePath) +
- '?scale=' + window.devicePixelRatio + 'x';
+ const url = new URL('chrome://fileicon/');
+ url.searchParams.set('path', filePath);
+ url.searchParams.set('scale', window.devicePixelRatio + 'x');
+ return url.toString();
}
/**
