Commit d6ac1584 authored by Mark Pilgrim's avatar Mark Pilgrim Committed by Commit Bot

Migrate WebApkIconHasher to SimpleURLLoader

Bug: 773295
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ic37eb92d81b0f0f16f997fad635d22933b3ce098
Reviewed-on: https://chromium-review.googlesource.com/1040445
Commit-Queue: Mark Pilgrim <pilgrim@chromium.org>
Reviewed-by: default avatarYaron Friedman <yfriedman@chromium.org>
Reviewed-by: default avatarMatt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555858}
parent bf25b82b
......@@ -8,9 +8,9 @@
#include "base/strings/utf_string_conversions.h"
#include "base/threading/thread_task_runner_handle.h"
#include "net/base/data_url.h"
#include "net/http/http_status_code.h"
#include "net/url_request/url_fetcher.h"
#include "net/url_request/url_request_context_getter.h"
#include "services/network/public/cpp/resource_request.h"
#include "services/network/public/cpp/simple_url_loader.h"
#include "services/network/public/mojom/url_loader_factory.mojom.h"
#include "third_party/smhasher/src/MurmurHash2.h"
namespace {
......@@ -36,17 +36,16 @@ std::string ComputeMurmur2Hash(const std::string& raw_image_data) {
// static
void WebApkIconHasher::DownloadAndComputeMurmur2Hash(
net::URLRequestContextGetter* request_context_getter,
network::mojom::URLLoaderFactory* url_loader_factory,
const GURL& icon_url,
const Murmur2HashCallback& callback) {
DownloadAndComputeMurmur2HashWithTimeout(request_context_getter, icon_url,
kDownloadTimeoutInMilliseconds,
callback);
DownloadAndComputeMurmur2HashWithTimeout(
url_loader_factory, icon_url, kDownloadTimeoutInMilliseconds, callback);
}
// static
void WebApkIconHasher::DownloadAndComputeMurmur2HashWithTimeout(
net::URLRequestContextGetter* request_context_getter,
network::mojom::URLLoaderFactory* url_loader_factory,
const GURL& icon_url,
int timeout_ms,
const Murmur2HashCallback& callback) {
......@@ -69,47 +68,53 @@ void WebApkIconHasher::DownloadAndComputeMurmur2HashWithTimeout(
}
// The icon hasher will delete itself when it is done.
new WebApkIconHasher(request_context_getter, icon_url, timeout_ms, callback);
new WebApkIconHasher(url_loader_factory, icon_url, timeout_ms, callback);
}
WebApkIconHasher::WebApkIconHasher(
net::URLRequestContextGetter* url_request_context_getter,
network::mojom::URLLoaderFactory* url_loader_factory,
const GURL& icon_url,
int timeout_ms,
const Murmur2HashCallback& callback)
: callback_(callback) {
DCHECK(url_loader_factory);
download_timeout_timer_.Start(
FROM_HERE, base::TimeDelta::FromMilliseconds(timeout_ms),
base::Bind(&WebApkIconHasher::OnDownloadTimedOut,
base::Unretained(this)));
url_fetcher_ = net::URLFetcher::Create(icon_url, net::URLFetcher::GET, this);
url_fetcher_->SetRequestContext(url_request_context_getter);
url_fetcher_->Start();
auto resource_request = std::make_unique<network::ResourceRequest>();
resource_request->url = icon_url;
simple_url_loader_ = network::SimpleURLLoader::Create(
std::move(resource_request), NO_TRAFFIC_ANNOTATION_YET);
simple_url_loader_->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
url_loader_factory,
base::BindOnce(&WebApkIconHasher::OnSimpleLoaderComplete,
base::Unretained(this)));
}
WebApkIconHasher::~WebApkIconHasher() {}
void WebApkIconHasher::OnURLFetchComplete(const net::URLFetcher* source) {
void WebApkIconHasher::OnSimpleLoaderComplete(
std::unique_ptr<std::string> response_body) {
download_timeout_timer_.Stop();
if (!source->GetStatus().is_success() ||
source->GetResponseCode() != net::HTTP_OK) {
// Check for non-empty body in case of HTTP 204 (no content) response.
if (!response_body || response_body->empty()) {
RunCallback("");
return;
}
// WARNING: We are running in the browser process. |raw_image_data| is the
// image's raw, unsanitized bytes from the web. |raw_image_data| may contain
// WARNING: We are running in the browser process. |*response_body| is the
// image's raw, unsanitized bytes from the web. |*response_body| may contain
// malicious data. Decoding unsanitized bitmap data to an SkBitmap in the
// browser process is a security bug.
std::string raw_image_data;
source->GetResponseAsString(&raw_image_data);
RunCallback(ComputeMurmur2Hash(raw_image_data));
RunCallback(ComputeMurmur2Hash(*response_body));
}
void WebApkIconHasher::OnDownloadTimedOut() {
url_fetcher_.reset();
simple_url_loader_.reset();
RunCallback("");
}
......
......@@ -11,16 +11,17 @@
#include "base/callback.h"
#include "base/macros.h"
#include "base/timer/timer.h"
#include "net/url_request/url_fetcher_delegate.h"
#include "url/gurl.h"
namespace net {
class URLFetcher;
class URLRequestContextGetter;
}
namespace network {
class SimpleURLLoader;
namespace mojom {
class URLLoaderFactory;
} // namespace mojom
} // namespace network
// Downloads an icon and takes a Murmur2 hash of the downloaded image.
class WebApkIconHasher : public net::URLFetcherDelegate {
class WebApkIconHasher {
public:
using Murmur2HashCallback =
base::Callback<void(const std::string& /* icon_murmur2_hash */)>;
......@@ -31,25 +32,24 @@ class WebApkIconHasher : public net::URLFetcherDelegate {
// encoding/decoding beforehand). |callback| is called with an empty string if
// the image cannot not be downloaded in time (e.g. 404 HTTP error code).
static void DownloadAndComputeMurmur2Hash(
net::URLRequestContextGetter* request_context_getter,
network::mojom::URLLoaderFactory* url_loader_factory,
const GURL& icon_url,
const Murmur2HashCallback& callback);
static void DownloadAndComputeMurmur2HashWithTimeout(
net::URLRequestContextGetter* request_context_getter,
network::mojom::URLLoaderFactory* url_loader_factory,
const GURL& icon_url,
int timeout_ms,
const Murmur2HashCallback& callback);
private:
WebApkIconHasher(net::URLRequestContextGetter* request_context_getter,
WebApkIconHasher(network::mojom::URLLoaderFactory* url_loader_factory,
const GURL& icon_url,
int timeout_ms,
const Murmur2HashCallback& callback);
~WebApkIconHasher() override;
~WebApkIconHasher();
// net::URLFetcherDelegate:
void OnURLFetchComplete(const net::URLFetcher* source) override;
void OnSimpleLoaderComplete(std::unique_ptr<std::string> response_body);
// Called if downloading the icon takes too long.
void OnDownloadTimedOut();
......@@ -60,7 +60,7 @@ class WebApkIconHasher : public net::URLFetcherDelegate {
// Called with the image hash.
Murmur2HashCallback callback_;
std::unique_ptr<net::URLFetcher> url_fetcher_;
std::unique_ptr<network::SimpleURLLoader> simple_url_loader_;
// Fails WebApkIconHasher if the download takes too long.
base::OneShotTimer download_timeout_timer_;
......
......@@ -9,36 +9,36 @@
#include "base/bind.h"
#include "base/callback_forward.h"
#include "base/files/file_path.h"
#include "base/files/file_util.h"
#include "base/memory/ref_counted.h"
#include "base/path_service.h"
#include "base/run_loop.h"
#include "base/threading/thread_task_runner_handle.h"
#include "content/public/test/test_browser_thread_bundle.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "net/url_request/url_request_test_util.h"
#include "net/http/http_util.h"
#include "services/network/public/cpp/resource_request.h"
#include "services/network/public/cpp/resource_response.h"
#include "services/network/public/cpp/url_loader_completion_status.h"
#include "services/network/public/mojom/url_loader_factory.mojom.h"
#include "services/network/test/test_url_loader_factory.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
namespace {
const base::FilePath::CharType kTestDataDir[] =
FILE_PATH_LITERAL("chrome/test/data");
const char kIconUrl[] = "/android/google.png";
// Murmur2 hash for |kIconUrl|.
// Murmur2 hash for |icon_url| in Success test.
const char kIconMurmur2Hash[] = "2081059568551351877";
// Runs WebApkIconHasher and blocks till the murmur2 hash is computed.
class WebApkIconHasherRunner {
public:
WebApkIconHasherRunner()
: url_request_context_getter_(new net::TestURLRequestContextGetter(
base::ThreadTaskRunnerHandle::Get())) {}
WebApkIconHasherRunner() {}
~WebApkIconHasherRunner() {}
void Run(const GURL& icon_url) {
void Run(network::mojom::URLLoaderFactory* url_loader_factory,
const GURL& icon_url) {
WebApkIconHasher::DownloadAndComputeMurmur2HashWithTimeout(
url_request_context_getter_.get(), icon_url, 300,
url_loader_factory, icon_url, 300,
base::Bind(&WebApkIconHasherRunner::OnCompleted,
base::Unretained(this)));
......@@ -55,8 +55,8 @@ class WebApkIconHasherRunner {
on_completed_callback_.Run();
}
scoped_refptr<net::TestURLRequestContextGetter>
url_request_context_getter_;
// Fake factory that can be primed to return fake data.
network::TestURLLoaderFactory test_url_loader_factory_;
// Called once the Murmur2 hash is taken.
base::Closure on_completed_callback_;
......@@ -75,24 +75,35 @@ class WebApkIconHasherTest : public ::testing::Test {
: thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP) {}
~WebApkIconHasherTest() override {}
void SetUp() override {
test_server_.AddDefaultHandlers(base::FilePath(kTestDataDir));
ASSERT_TRUE(test_server_.Start());
protected:
network::TestURLLoaderFactory* test_url_loader_factory() {
return &test_url_loader_factory_;
}
net::test_server::EmbeddedTestServer* test_server() { return &test_server_; }
private:
content::TestBrowserThreadBundle thread_bundle_;
net::EmbeddedTestServer test_server_;
network::TestURLLoaderFactory test_url_loader_factory_;
DISALLOW_COPY_AND_ASSIGN(WebApkIconHasherTest);
};
TEST_F(WebApkIconHasherTest, Success) {
GURL icon_url = test_server()->GetURL(kIconUrl);
std::string icon_url =
"http://www.google.com/chrome/test/data/android/google.png";
base::FilePath source_path;
base::FilePath icon_path;
ASSERT_TRUE(PathService::Get(base::DIR_SOURCE_ROOT, &source_path));
icon_path = source_path.AppendASCII("chrome")
.AppendASCII("test")
.AppendASCII("data")
.AppendASCII("android")
.AppendASCII("google.png");
std::string icon_data;
ASSERT_TRUE(base::ReadFileToString(icon_path, &icon_data));
test_url_loader_factory()->AddResponse(icon_url, icon_data);
WebApkIconHasherRunner runner;
runner.Run(icon_url);
runner.Run(test_url_loader_factory(), GURL(icon_url));
EXPECT_EQ(kIconMurmur2Hash, runner.murmur2_hash());
}
......@@ -101,36 +112,45 @@ TEST_F(WebApkIconHasherTest, DataUri) {
"AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO"
"9TXL0Y4OHwAAAABJRU5ErkJggg==");
WebApkIconHasherRunner runner;
runner.Run(icon_url);
runner.Run(test_url_loader_factory(), icon_url);
EXPECT_EQ("536500236142107998", runner.murmur2_hash());
}
TEST_F(WebApkIconHasherTest, DataUriInvalid) {
GURL icon_url("data:image/png;base64");
WebApkIconHasherRunner runner;
runner.Run(icon_url);
runner.Run(test_url_loader_factory(), icon_url);
EXPECT_EQ("", runner.murmur2_hash());
}
TEST_F(WebApkIconHasherTest, InvalidUrl) {
GURL icon_url("http::google.com");
WebApkIconHasherRunner runner;
runner.Run(icon_url);
runner.Run(test_url_loader_factory(), icon_url);
EXPECT_EQ("", runner.murmur2_hash());
}
TEST_F(WebApkIconHasherTest, DownloadTimedOut) {
GURL icon_url = test_server()->GetURL("/slow?100");
std::string icon_url = "http://www.google.com/timeout";
WebApkIconHasherRunner runner;
runner.Run(icon_url);
runner.Run(test_url_loader_factory(), GURL(icon_url));
EXPECT_EQ("", runner.murmur2_hash());
}
// Test that the hash callback is called with an empty string if an HTTP error
// prevents the icon URL from being fetched.
TEST_F(WebApkIconHasherTest, HTTPError) {
GURL icon_url = test_server()->GetURL("/nocontent");
std::string icon_url = "http://www.google.com/404";
network::ResourceResponseHead head;
std::string headers("HTTP/1.1 404 Not Found\nContent-type: text/html\n\n");
head.headers = new net::HttpResponseHeaders(
net::HttpUtil::AssembleRawHeaders(headers.c_str(), headers.size()));
head.mime_type = "text/html";
network::URLLoaderCompletionStatus status;
status.decoded_body_length = 0;
test_url_loader_factory()->AddResponse(GURL(icon_url), head, "", status);
WebApkIconHasherRunner runner;
runner.Run(icon_url);
runner.Run(test_url_loader_factory(), GURL(icon_url));
EXPECT_EQ("", runner.murmur2_hash());
}
......@@ -36,8 +36,10 @@
#include "chrome/browser/profiles/profile.h"
#include "chrome/common/chrome_switches.h"
#include "components/version_info/version_info.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/browsing_data_remover.h"
#include "content/public/browser/storage_partition.h"
#include "content/public/common/manifest_util.h"
#include "jni/WebApkInstaller_jni.h"
#include "net/base/load_flags.h"
......@@ -580,7 +582,9 @@ void WebApkInstaller::OnHaveSufficientSpaceForInstall() {
// We redownload the icon in order to take the Murmur2 hash. The redownload
// should be fast because the icon should be in the HTTP cache.
WebApkIconHasher::DownloadAndComputeMurmur2Hash(
GetRequestContext(browser_context_),
content::BrowserContext::GetDefaultStoragePartition(browser_context_)
->GetURLLoaderFactoryForBrowserProcess()
.get(),
install_shortcut_info_->best_primary_icon_url,
base::Bind(&WebApkInstaller::OnGotPrimaryIconMurmur2Hash,
weak_ptr_factory_.GetWeakPtr()));
......@@ -598,7 +602,9 @@ void WebApkInstaller::OnGotPrimaryIconMurmur2Hash(
install_shortcut_info_->best_badge_icon_url !=
install_shortcut_info_->best_primary_icon_url) {
WebApkIconHasher::DownloadAndComputeMurmur2Hash(
GetRequestContext(browser_context_),
content::BrowserContext::GetDefaultStoragePartition(browser_context_)
->GetURLLoaderFactoryForBrowserProcess()
.get(),
install_shortcut_info_->best_badge_icon_url,
base::Bind(&WebApkInstaller::OnGotBadgeIconMurmur2Hash,
weak_ptr_factory_.GetWeakPtr(), true, primary_icon_hash));
......
......@@ -14,6 +14,8 @@
#include "chrome/browser/android/webapk/webapk_web_manifest_checker.h"
#include "chrome/browser/installable/installable_manager.h"
#include "chrome/browser/profiles/profile.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/storage_partition.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/manifest.h"
#include "jni/WebApkUpdateDataFetcher_jni.h"
......@@ -151,7 +153,10 @@ void WebApkUpdateDataFetcher::OnDidGetInstallableData(
Profile::FromBrowserContext(web_contents()->GetBrowserContext());
WebApkIconHasher::DownloadAndComputeMurmur2Hash(
profile->GetRequestContext(), info_.best_primary_icon_url,
content::BrowserContext::GetDefaultStoragePartition(profile)
->GetURLLoaderFactoryForBrowserProcess()
.get(),
info_.best_primary_icon_url,
base::Bind(&WebApkUpdateDataFetcher::OnGotPrimaryIconMurmur2Hash,
weak_ptr_factory_.GetWeakPtr()));
}
......@@ -167,7 +172,10 @@ void WebApkUpdateDataFetcher::OnGotPrimaryIconMurmur2Hash(
Profile::FromBrowserContext(web_contents()->GetBrowserContext());
WebApkIconHasher::DownloadAndComputeMurmur2Hash(
profile->GetRequestContext(), info_.best_badge_icon_url,
content::BrowserContext::GetDefaultStoragePartition(profile)
->GetURLLoaderFactoryForBrowserProcess()
.get(),
info_.best_badge_icon_url,
base::Bind(&WebApkUpdateDataFetcher::OnDataAvailable,
weak_ptr_factory_.GetWeakPtr(), primary_icon_murmur2_hash,
true));
......
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