Skip to content

Commit

Permalink
Convert URLLoaderFactory from //chrome and //extensions to new Mojo t…
Browse files Browse the repository at this point in the history
…ypes

This CL converts URLLoaderFactory from //chrome and //extensions
to new Mojo types using PendingRemote and Remote.

Bug: 955171
Change-Id: I0379f56d1069252e482f3fd3d15d3ec2087344a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1859971
Commit-Queue: Julie Kim <jkim@igalia.com>
Reviewed-by: Dominic Battré <battre@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Tommi <tommi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706976}
  • Loading branch information
jkim-julie authored and Commit Bot committed Oct 17, 2019
1 parent 0cb1801 commit fb6f530
Show file tree
Hide file tree
Showing 36 changed files with 187 additions and 150 deletions.
4 changes: 2 additions & 2 deletions android_webview/browser/aw_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -881,9 +881,9 @@ bool AwContentBrowserClient::HandleExternalProtocol(
ui::PageTransition page_transition,
bool has_user_gesture,
const base::Optional<url::Origin>& initiating_origin,
network::mojom::URLLoaderFactoryPtr* out_factory) {
mojo::PendingRemote<network::mojom::URLLoaderFactory>* out_factory) {
mojo::PendingReceiver<network::mojom::URLLoaderFactory> receiver =
mojo::MakeRequest(out_factory);
out_factory->InitWithNewPipeAndPassReceiver();
if (content::BrowserThread::CurrentlyOn(content::BrowserThread::IO)) {
// Manages its own lifetime.
new android_webview::AwProxyingURLLoaderFactory(
Expand Down
3 changes: 2 additions & 1 deletion android_webview/browser/aw_content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,8 @@ class AwContentBrowserClient : public content::ContentBrowserClient {
ui::PageTransition page_transition,
bool has_user_gesture,
const base::Optional<url::Origin>& initiating_origin,
network::mojom::URLLoaderFactoryPtr* out_factory) override;
mojo::PendingRemote<network::mojom::URLLoaderFactory>* out_factory)
override;
void RegisterNonNetworkSubresourceURLLoaderFactories(
int render_process_id,
int render_frame_id,
Expand Down
8 changes: 3 additions & 5 deletions chrome/browser/chrome_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -299,8 +299,6 @@
#include "media/mojo/buildflags.h"
#include "media/webrtc/webrtc_switches.h"
#include "mojo/public/cpp/bindings/pending_associated_receiver.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver_set.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "mojo/public/cpp/bindings/scoped_interface_endpoint_handle.h"
Expand Down Expand Up @@ -1579,7 +1577,7 @@ bool ChromeContentBrowserClient::ShouldTreatURLSchemeAsFirstPartyWhenTopLevel(
#endif
}

network::mojom::URLLoaderFactoryPtrInfo
mojo::PendingRemote<network::mojom::URLLoaderFactory>
ChromeContentBrowserClient::CreateURLLoaderFactoryForNetworkRequests(
content::RenderProcessHost* process,
network::mojom::NetworkContext* network_context,
Expand All @@ -1593,7 +1591,7 @@ ChromeContentBrowserClient::CreateURLLoaderFactoryForNetworkRequests(
header_client, request_initiator,
network_isolation_key);
#else
return network::mojom::URLLoaderFactoryPtrInfo();
return mojo::NullRemote();
#endif
}

Expand Down Expand Up @@ -4762,7 +4760,7 @@ bool ChromeContentBrowserClient::HandleExternalProtocol(
ui::PageTransition page_transition,
bool has_user_gesture,
const base::Optional<url::Origin>& initiating_origin,
network::mojom::URLLoaderFactoryPtr* out_factory) {
mojo::PendingRemote<network::mojom::URLLoaderFactory>* out_factory) {
#if BUILDFLAG(ENABLE_EXTENSIONS)
// External protocols are disabled for guests. An exception is made for the
// "mailto" protocol, so that pages that utilize it work properly in a
Expand Down
6 changes: 4 additions & 2 deletions chrome/browser/chrome_content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "extensions/buildflags/buildflags.h"
#include "media/media_buildflags.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "ppapi/buildflags/buildflags.h"
#include "services/network/public/mojom/network_context.mojom-forward.h"
#include "services/service_manager/public/cpp/binder_registry.h"
Expand Down Expand Up @@ -152,7 +153,7 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
const char* GetInitiatorSchemeBypassingDocumentBlocking() override;
bool ShouldTreatURLSchemeAsFirstPartyWhenTopLevel(
base::StringPiece scheme) override;
network::mojom::URLLoaderFactoryPtrInfo
mojo::PendingRemote<network::mojom::URLLoaderFactory>
CreateURLLoaderFactoryForNetworkRequests(
content::RenderProcessHost* process,
network::mojom::NetworkContext* network_context,
Expand Down Expand Up @@ -551,7 +552,8 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
ui::PageTransition page_transition,
bool has_user_gesture,
const base::Optional<url::Origin>& initiating_origin,
network::mojom::URLLoaderFactoryPtr* out_factory) override;
mojo::PendingRemote<network::mojom::URLLoaderFactory>* out_factory)
override;
std::unique_ptr<content::OverlayWindow> CreateWindowForPictureInPicture(
content::PictureInPictureWindowController* controller) override;
void RegisterRendererPreferenceWatcher(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ class CRLSetComponentInstallerTest : public PlatformTest {
request.request_initiator = url::Origin();

client_ = std::make_unique<network::TestURLLoaderClient>();
network::mojom::URLLoaderFactoryPtr loader_factory;
mojo::Remote<network::mojom::URLLoaderFactory> loader_factory;
network::mojom::URLLoaderFactoryParamsPtr params =
network::mojom::URLLoaderFactoryParams::New();
params->process_id = 0;
params->is_corb_enabled = false;
network_context_->CreateURLLoaderFactory(mojo::MakeRequest(&loader_factory),
std::move(params));
network_context_->CreateURLLoaderFactory(
loader_factory.BindNewPipeAndPassReceiver(), std::move(params));
loader_factory->CreateLoaderAndStart(
mojo::MakeRequest(&loader_), 1, 1,
network::mojom::kURLLoadOptionSendSSLInfoWithResponse |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "extensions/browser/event_router.h"
#include "extensions/browser/extension_host.h"
#include "extensions/browser/notification_types.h"
#include "mojo/public/cpp/bindings/pending_remote.h"

#if defined(OS_CHROMEOS)
#include "chrome/browser/chromeos/file_manager/path_util.h"
Expand Down Expand Up @@ -80,14 +81,15 @@ void OperationManager::StartWriteFromUrl(
return;
}

network::mojom::URLLoaderFactoryPtrInfo url_loader_factory_info;
mojo::PendingRemote<network::mojom::URLLoaderFactory>
url_loader_factory_remote;
content::BrowserContext::GetDefaultStoragePartition(browser_context_)
->GetURLLoaderFactoryForBrowserProcess()
->Clone(mojo::MakeRequest(&url_loader_factory_info));
->Clone(url_loader_factory_remote.InitWithNewPipeAndPassReceiver());

auto operation = base::MakeRefCounted<WriteFromUrlOperation>(
weak_factory_.GetWeakPtr(), extension_id,
std::move(url_loader_factory_info), url, hash, device_path,
std::move(url_loader_factory_remote), url, hash, device_path,
GetAssociatedDownloadFolder());
operations_[extension_id] = operation;
operation->PostTask(base::BindOnce(&Operation::Start, operation));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "chrome/browser/extensions/api/image_writer_private/error_messages.h"
#include "chrome/browser/extensions/api/image_writer_private/operation_manager.h"
#include "content/public/browser/browser_thread.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "net/traffic_annotation/network_traffic_annotation.h"
#include "net/url_request/url_fetcher.h"
#include "services/network/public/cpp/simple_url_loader.h"
Expand All @@ -21,13 +22,13 @@ using content::BrowserThread;
WriteFromUrlOperation::WriteFromUrlOperation(
base::WeakPtr<OperationManager> manager,
const ExtensionId& extension_id,
network::mojom::URLLoaderFactoryPtrInfo factory_info,
mojo::PendingRemote<network::mojom::URLLoaderFactory> factory_remote,
GURL url,
const std::string& hash,
const std::string& device_path,
const base::FilePath& download_folder)
: Operation(manager, extension_id, device_path, download_folder),
url_loader_factory_ptr_info_(std::move(factory_info)),
url_loader_factory_remote_(std::move(factory_remote)),
url_(url),
hash_(hash),
download_continuation_() {}
Expand Down Expand Up @@ -120,11 +121,11 @@ void WriteFromUrlOperation::Download(base::OnceClosure continuation) {
AddCleanUpFunction(
base::BindOnce(&WriteFromUrlOperation::DestroySimpleURLLoader, this));

network::mojom::URLLoaderFactoryPtr url_loader_factory_ptr;
url_loader_factory_ptr.Bind(std::move(url_loader_factory_ptr_info_));
mojo::Remote<network::mojom::URLLoaderFactory> url_loader_factory_remote(
std::move(url_loader_factory_remote_));

simple_url_loader_->DownloadToFile(
url_loader_factory_ptr.get(),
url_loader_factory_remote.get(),
base::BindOnce(&WriteFromUrlOperation::OnSimpleLoaderComplete,
base::Unretained(this)),
image_path_);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <stdint.h>

#include "chrome/browser/extensions/api/image_writer_private/operation.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "services/network/public/mojom/url_loader_factory.mojom.h"
#include "services/network/public/mojom/url_response_head.mojom-forward.h"
#include "url/gurl.h"
Expand All @@ -24,13 +25,14 @@ class OperationManager;
// Encapsulates a write of an image accessed via URL.
class WriteFromUrlOperation : public Operation {
public:
WriteFromUrlOperation(base::WeakPtr<OperationManager> manager,
const ExtensionId& extension_id,
network::mojom::URLLoaderFactoryPtrInfo factory_info,
GURL url,
const std::string& hash,
const std::string& storage_unit_id,
const base::FilePath& download_folder);
WriteFromUrlOperation(
base::WeakPtr<OperationManager> manager,
const ExtensionId& extension_id,
mojo::PendingRemote<network::mojom::URLLoaderFactory> factory_remote,
GURL url,
const std::string& hash,
const std::string& storage_unit_id,
const base::FilePath& download_folder);
void StartImpl() override;

protected:
Expand Down Expand Up @@ -61,7 +63,8 @@ class WriteFromUrlOperation : public Operation {
void VerifyDownloadComplete(base::OnceClosure continuation);

// Arguments
network::mojom::URLLoaderFactoryPtrInfo url_loader_factory_ptr_info_;
mojo::PendingRemote<network::mojom::URLLoaderFactory>
url_loader_factory_remote_;
GURL url_;
const std::string hash_;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ class WriteFromUrlOperationForTest : public WriteFromUrlOperation {
WriteFromUrlOperationForTest(
base::WeakPtr<OperationManager> manager,
const ExtensionId& extension_id,
network::mojom::URLLoaderFactoryPtrInfo factory_info,
mojo::PendingRemote<network::mojom::URLLoaderFactory> factory_remote,
GURL url,
const std::string& hash,
const std::string& storage_unit_id)
: WriteFromUrlOperation(manager,
extension_id,
std::move(factory_info),
std::move(factory_remote),
url,
hash,
storage_unit_id,
Expand Down Expand Up @@ -115,15 +115,16 @@ class ImageWriterWriteFromUrlOperationTest : public ImageWriterUnitTestBase {
scoped_refptr<WriteFromUrlOperationForTest> CreateOperation(
const GURL& url,
const std::string& hash) {
network::mojom::URLLoaderFactoryPtrInfo url_loader_factory_ptr_info;
mojo::PendingRemote<network::mojom::URLLoaderFactory>
url_loader_factory_remote;
content::BrowserContext::GetDefaultStoragePartition(&test_profile_)
->GetURLLoaderFactoryForBrowserProcess()
->Clone(mojo::MakeRequest(&url_loader_factory_ptr_info));
->Clone(url_loader_factory_remote.InitWithNewPipeAndPassReceiver());

scoped_refptr<WriteFromUrlOperationForTest> operation(
new WriteFromUrlOperationForTest(
manager_.AsWeakPtr(), kDummyExtensionId,
std::move(url_loader_factory_ptr_info), url, hash,
std::move(url_loader_factory_remote), url, hash,
test_utils_.GetDevicePath().AsUTF8Unsafe()));
operation->Start();
return operation;
Expand Down
20 changes: 12 additions & 8 deletions chrome/browser/extensions/api/web_request/web_request_apitest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
#include "extensions/test/result_catcher.h"
#include "extensions/test/test_extension_dir.h"
#include "google_apis/gaia/gaia_switches.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "net/dns/mock_host_resolver.h"
#include "net/http/http_util.h"
Expand Down Expand Up @@ -297,16 +298,17 @@ class ExtensionWebRequestApiTest : public ExtensionApiTest {
const char* expected_content_regular_window,
const char* exptected_content_incognito_window);

network::mojom::URLLoaderFactoryPtr CreateURLLoaderFactory() {
mojo::PendingRemote<network::mojom::URLLoaderFactory>
CreateURLLoaderFactory() {
network::mojom::URLLoaderFactoryParamsPtr params =
network::mojom::URLLoaderFactoryParams::New();
params->process_id = network::mojom::kBrowserProcessId;
params->is_corb_enabled = false;
network::mojom::URLLoaderFactoryPtr loader_factory;
mojo::PendingRemote<network::mojom::URLLoaderFactory> loader_factory;
content::BrowserContext::GetDefaultStoragePartition(profile())
->GetNetworkContext()
->CreateURLLoaderFactory(mojo::MakeRequest(&loader_factory),
std::move(params));
->CreateURLLoaderFactory(
loader_factory.InitWithNewPipeAndPassReceiver(), std::move(params));
return loader_factory;
}

Expand Down Expand Up @@ -1745,20 +1747,22 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
return;
}

auto loader_factory = CreateURLLoaderFactory();
mojo::Remote<network::mojom::URLLoaderFactory> loader_factory(
CreateURLLoaderFactory());
bool has_connection_error = false;
loader_factory.set_connection_error_handler(
loader_factory.set_disconnect_handler(
base::BindLambdaForTesting([&]() { has_connection_error = true; }));

InstallWebRequestExtension("extension1");
content::BrowserContext::GetDefaultStoragePartition(profile())
->FlushNetworkInterfaceForTesting();
EXPECT_TRUE(has_connection_error);
loader_factory.reset();

// The second time there should be no connection error.
loader_factory = CreateURLLoaderFactory();
loader_factory.Bind(CreateURLLoaderFactory());
has_connection_error = false;
loader_factory.set_connection_error_handler(
loader_factory.set_disconnect_handler(
base::BindLambdaForTesting([&]() { has_connection_error = true; }));
InstallWebRequestExtension("extension2");
content::BrowserContext::GetDefaultStoragePartition(profile())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ ChromeContentBrowserClientExtensionsPart::GetVpnServiceProxy(
}

// static
network::mojom::URLLoaderFactoryPtrInfo
mojo::PendingRemote<network::mojom::URLLoaderFactory>
ChromeContentBrowserClientExtensionsPart::
CreateURLLoaderFactoryForNetworkRequests(
content::RenderProcessHost* process,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "chrome/browser/chrome_content_browser_client_parts.h"
#include "content/public/browser/browser_or_resource_context.h"
#include "content/public/common/resource_type.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "services/network/public/mojom/network_context.mojom.h"
#include "services/network/public/mojom/url_loader_factory.mojom.h"
#include "ui/base/page_transition_types.h"
Expand Down Expand Up @@ -86,7 +87,7 @@ class ChromeContentBrowserClientExtensionsPart
static std::unique_ptr<content::VpnServiceProxy> GetVpnServiceProxy(
content::BrowserContext* browser_context);

static network::mojom::URLLoaderFactoryPtrInfo
static mojo::PendingRemote<network::mojom::URLLoaderFactory>
CreateURLLoaderFactoryForNetworkRequests(
content::RenderProcessHost* process,
network::mojom::NetworkContext* network_context,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "net/base/load_flags.h"
#include "net/http/http_response_headers.h"
#include "net/http/http_status_code.h"
Expand Down Expand Up @@ -164,15 +165,15 @@ void DialURLFetcher::StartDownload() {
// Bind the request to the system URLLoaderFactory obtained on UI thread.
// Currently this is the only way to guarantee a live URLLoaderFactory.
// TOOD(mmenke): Figure out a way to do this transparently on IO thread.
network::mojom::URLLoaderFactoryPtr loader_factory;
mojo::Remote<network::mojom::URLLoaderFactory> loader_factory;

// TODO(https://crbug.com/823869): Fix DeviceDescriptionServiceTest and remove
// this conditional.
auto mojo_request = mojo::MakeRequest(&loader_factory);
auto mojo_receiver = loader_factory.BindNewPipeAndPassReceiver();
if (content::BrowserThread::IsThreadInitialized(content::BrowserThread::UI)) {
base::PostTask(FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(&BindURLLoaderFactoryReceiverOnUIThread,
std::move(mojo_request)));
std::move(mojo_receiver)));
}

loader_->DownloadToString(
Expand Down
12 changes: 7 additions & 5 deletions chrome/browser/media/webrtc/webrtc_event_log_uploader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "net/base/load_flags.h"
#include "net/base/mime_util.h"
#include "net/http/http_status_code.h"
Expand Down Expand Up @@ -280,10 +281,11 @@ void WebRtcEventLogUploaderImpl::StartUpload(const std::string& upload_data) {
// Create a new mojo pipe. It's safe to pass this around and use
// immediately, even though it needs to finish initialization on the UI
// thread.
network::mojom::URLLoaderFactoryPtr url_loader_factory_ptr;
base::PostTask(FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(BindURLLoaderFactoryReceiver,
mojo::MakeRequest(&url_loader_factory_ptr)));
mojo::Remote<network::mojom::URLLoaderFactory> url_loader_factory_remote;
base::PostTask(
FROM_HERE, {content::BrowserThread::UI},
base::BindOnce(BindURLLoaderFactoryReceiver,
url_loader_factory_remote.BindNewPipeAndPassReceiver()));

url_loader_ = network::SimpleURLLoader::Create(
std::move(resource_request), kWebrtcEventLogUploaderTrafficAnnotation);
Expand All @@ -294,7 +296,7 @@ void WebRtcEventLogUploaderImpl::StartUpload(const std::string& upload_data) {
// See comment in destructor for an explanation about why using
// base::Unretained(this) is safe here.
url_loader_->DownloadToString(
url_loader_factory_ptr.get(),
url_loader_factory_remote.get(),
base::BindOnce(&WebRtcEventLogUploaderImpl::OnURLLoadComplete,
base::Unretained(this)),
kWebRtcEventLogMaxUploadIdBytes);
Expand Down
Loading

0 comments on commit fb6f530

Please sign in to comment.