Commit 1255ee23 authored by Nicolas's avatar Nicolas Committed by Commit Bot

[BrowserSwitcher] Fix file:// URLs for sitelists

When the Network Service is enabled, LBS failed to download sitelists
for file:// URLs. This CL fixes the issue.

Bug: 936160
Change-Id: Iae19b7d9798236f35f7a8c525569b4cb241296c5
Reviewed-on: https://chromium-review.googlesource.com/c/1495575
Commit-Queue: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Reviewed-by: default avatarJulian Pastarmov <pastarmovj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636912}
parent b51cf5f4
...@@ -16,6 +16,8 @@ ...@@ -16,6 +16,8 @@
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "content/public/browser/browser_context.h" #include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "content/public/browser/file_url_loader.h"
#include "content/public/browser/shared_cors_origin_access_list.h"
#include "content/public/browser/storage_partition.h" #include "content/public/browser/storage_partition.h"
#include "net/base/load_flags.h" #include "net/base/load_flags.h"
#include "net/traffic_annotation/network_traffic_annotation.h" #include "net/traffic_annotation/network_traffic_annotation.h"
...@@ -83,9 +85,11 @@ XmlDownloader::XmlDownloader(Profile* profile, ...@@ -83,9 +85,11 @@ XmlDownloader::XmlDownloader(Profile* profile,
: sources_(std::move(sources)), : sources_(std::move(sources)),
all_done_callback_(std::move(all_done_callback)), all_done_callback_(std::move(all_done_callback)),
weak_ptr_factory_(this) { weak_ptr_factory_(this) {
factory_ = content::BrowserContext::GetDefaultStoragePartition(profile) file_url_factory_ =
->GetURLLoaderFactoryForBrowserProcess(); content::CreateFileURLLoaderFactory(base::FilePath(), nullptr);
DCHECK(factory_); other_url_factory_ =
content::BrowserContext::GetDefaultStoragePartition(profile)
->GetURLLoaderFactoryForBrowserProcess();
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE, FROM_HERE,
base::BindOnce(&XmlDownloader::FetchXml, weak_ptr_factory_.GetWeakPtr()), base::BindOnce(&XmlDownloader::FetchXml, weak_ptr_factory_.GetWeakPtr()),
...@@ -107,12 +111,19 @@ void XmlDownloader::FetchXml() { ...@@ -107,12 +111,19 @@ void XmlDownloader::FetchXml() {
kFetchNumRetries, kFetchNumRetries,
network::SimpleURLLoader::RetryMode::RETRY_ON_NETWORK_CHANGE); network::SimpleURLLoader::RetryMode::RETRY_ON_NETWORK_CHANGE);
source.url_loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie( source.url_loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
factory_.get(), GetURLLoaderFactoryForURL(source.url),
base::BindOnce(&XmlDownloader::ParseXml, weak_ptr_factory_.GetWeakPtr(), base::BindOnce(&XmlDownloader::ParseXml, weak_ptr_factory_.GetWeakPtr(),
base::Unretained(&source))); base::Unretained(&source)));
} }
} }
network::mojom::URLLoaderFactory* XmlDownloader::GetURLLoaderFactoryForURL(
const GURL& url) {
if (url.SchemeIsFile())
return file_url_factory_.get();
return other_url_factory_.get();
}
void XmlDownloader::ParseXml(RulesetSource* source, void XmlDownloader::ParseXml(RulesetSource* source,
std::unique_ptr<std::string> bytes) { std::unique_ptr<std::string> bytes) {
if (!bytes) { if (!bytes) {
......
...@@ -60,7 +60,10 @@ class XmlDownloader { ...@@ -60,7 +60,10 @@ class XmlDownloader {
void ParseXml(RulesetSource* source, std::unique_ptr<std::string> bytes); void ParseXml(RulesetSource* source, std::unique_ptr<std::string> bytes);
void DoneParsing(RulesetSource* source, ParsedXml xml); void DoneParsing(RulesetSource* source, ParsedXml xml);
scoped_refptr<network::SharedURLLoaderFactory> factory_; network::mojom::URLLoaderFactory* GetURLLoaderFactoryForURL(const GURL& url);
std::unique_ptr<network::mojom::URLLoaderFactory> file_url_factory_;
scoped_refptr<network::SharedURLLoaderFactory> other_url_factory_;
std::vector<RulesetSource> sources_; std::vector<RulesetSource> sources_;
......
...@@ -4,9 +4,14 @@ ...@@ -4,9 +4,14 @@
#include "chrome/browser/browser_switcher/browser_switcher_service.h" #include "chrome/browser/browser_switcher/browser_switcher_service.h"
#include <string.h>
#include "base/bind.h" #include "base/bind.h"
#include "base/files/file_path.h"
#include "base/files/file_util.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/test/test_timeouts.h" #include "base/test/test_timeouts.h"
#include "base/threading/thread_restrictions.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/browser_switcher/browser_switcher_prefs.h" #include "chrome/browser/browser_switcher/browser_switcher_prefs.h"
#include "chrome/browser/browser_switcher/browser_switcher_service_factory.h" #include "chrome/browser/browser_switcher/browser_switcher_service_factory.h"
...@@ -20,6 +25,7 @@ ...@@ -20,6 +25,7 @@
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "content/public/test/test_utils.h" #include "content/public/test/test_utils.h"
#include "content/public/test/url_loader_interceptor.h" #include "content/public/test/url_loader_interceptor.h"
#include "net/base/filename_util.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -36,13 +42,14 @@ namespace { ...@@ -36,13 +42,14 @@ namespace {
const char kAValidUrl[] = "http://example.com/"; const char kAValidUrl[] = "http://example.com/";
const char kAnInvalidUrl[] = "the quick brown fox jumps over the lazy dog"; const char kAnInvalidUrl[] = "the quick brown fox jumps over the lazy dog";
const char kSitelistXml[] =
"<rules version=\"1\"><docMode><domain docMode=\"9\">"
"docs.google.com</domain></docMode></rules>";
bool ReturnValidXml(content::URLLoaderInterceptor::RequestParams* params) { bool ReturnValidXml(content::URLLoaderInterceptor::RequestParams* params) {
std::string headers = "HTTP/1.1 200 OK\nContent-Type: text/html\n\n"; std::string headers = "HTTP/1.1 200 OK\nContent-Type: text/html\n\n";
std::string xml = content::URLLoaderInterceptor::WriteResponse(
"<rules version=\"1\"><docMode><domain docMode=\"9\">" headers, std::string(kSitelistXml), params->client.get());
"docs.google.com</domain></docMode></rules>";
content::URLLoaderInterceptor::WriteResponse(headers, xml,
params->client.get());
return true; return true;
} }
...@@ -167,6 +174,33 @@ IN_PROC_BROWSER_TEST_F(BrowserSwitcherServiceTest, ...@@ -167,6 +174,33 @@ IN_PROC_BROWSER_TEST_F(BrowserSwitcherServiceTest,
run_loop.Run(); run_loop.Run();
} }
IN_PROC_BROWSER_TEST_F(BrowserSwitcherServiceTest, ExternalFileUrl) {
base::ScopedAllowBlockingForTesting allow_blocking;
base::ScopedTempDir dir;
ASSERT_TRUE(dir.CreateUniqueTempDir());
base::FilePath sitelist_path = dir.GetPath().AppendASCII("sitelist.xml");
base::WriteFile(sitelist_path, kSitelistXml, strlen(kSitelistXml));
SetExternalUrl(net::FilePathToFileURL(sitelist_path).spec());
// Execute everything and make sure the rules are applied correctly.
auto* service =
BrowserSwitcherServiceFactory::GetForBrowserContext(browser()->profile());
base::RunLoop run_loop;
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
base::BindOnce(
[](BrowserSwitcherService* service, base::OnceClosure quit) {
EXPECT_FALSE(ShouldSwitch(service, GURL("http://google.com/")));
EXPECT_TRUE(ShouldSwitch(service, GURL("http://docs.google.com/")));
std::move(quit).Run();
},
service, run_loop.QuitClosure()),
TestTimeouts::action_timeout());
run_loop.Run();
}
IN_PROC_BROWSER_TEST_F(BrowserSwitcherServiceTest, IN_PROC_BROWSER_TEST_F(BrowserSwitcherServiceTest,
ExternalIgnoresFailedDownload) { ExternalIgnoresFailedDownload) {
SetExternalUrl(kAValidUrl); SetExternalUrl(kAValidUrl);
......
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