Skip to content

Commit

Permalink
Replaced static URLRequestFileJob factory with non-static protocol ha…
Browse files Browse the repository at this point in the history
…ndler for File jobs

This is a refactor and shouldn't have any visible behavioral change.

BUG=crbug.com/142945
TEST=browser_tests --single_process --gtest_filter=FullscreenControllerTest.FullscreenFileURL

Review URL: https://chromiumcodereview.appspot.com/10700117

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@152381 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
shalev@chromium.org committed Aug 20, 2012
1 parent 37cfa2a commit 65dcdc5
Show file tree
Hide file tree
Showing 14 changed files with 181 additions and 37 deletions.
5 changes: 4 additions & 1 deletion chrome/browser/extensions/extension_protocols.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "net/base/net_errors.h"
#include "net/http/http_response_headers.h"
#include "net/http/http_response_info.h"
#include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_error_job.h"
#include "net/url_request/url_request_file_job.h"
#include "net/url_request/url_request_simple_job.h"
Expand Down Expand Up @@ -195,7 +196,9 @@ class URLRequestExtensionJob : public net::URLRequestFileJob {
const FilePath& directory_path,
const std::string& content_security_policy,
bool send_cors_header)
: net::URLRequestFileJob(request, FilePath()),
: net::URLRequestFileJob(request,
FilePath(),
request->context()->network_delegate()),
// TODO(tc): Move all of these files into resources.pak so we don't break
// when updating on Linux.
resource_(extension_id, directory_path,
Expand Down
7 changes: 5 additions & 2 deletions chrome/browser/extensions/extension_resource_protocols.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,18 @@
#include "chrome/common/chrome_paths.h"
#include "chrome/common/extensions/extension_file_util.h"
#include "content/public/browser/browser_thread.h"
#include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_file_job.h"

namespace {

class ExtensionResourcesJob : public net::URLRequestFileJob {
public:
explicit ExtensionResourcesJob(net::URLRequest* request)
: net::URLRequestFileJob(request, FilePath()),
thread_id_(content::BrowserThread::UI) {
: net::URLRequestFileJob(request,
FilePath(),
request->context()->network_delegate()),
thread_id_(content::BrowserThread::UI) {
}

virtual void Start() OVERRIDE;
Expand Down
23 changes: 23 additions & 0 deletions chrome/browser/profiles/off_the_record_profile_io_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "chrome/browser/net/chrome_network_delegate.h"
#include "chrome/browser/net/chrome_url_request_context.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/webui/chrome_url_data_manager_backend.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/extensions/extension.h"
#include "chrome/common/pref_names.h"
Expand All @@ -27,6 +28,7 @@
#include "net/ftp/ftp_network_layer.h"
#include "net/http/http_cache.h"
#include "net/http/http_server_properties_impl.h"
#include "net/url_request/file_protocol_handler.h"
#include "net/url_request/ftp_protocol_handler.h"
#include "net/url_request/url_request_job_factory.h"
#include "webkit/database/database_tracker.h"
Expand Down Expand Up @@ -251,6 +253,27 @@ void OffTheRecordProfileIOData::LazyInitializeInternal(
main_job_factory_.reset(new net::URLRequestJobFactory);
extensions_job_factory_.reset(new net::URLRequestJobFactory);

int set_protocol = main_job_factory_->SetProtocolHandler(
chrome::kFileScheme, new net::FileProtocolHandler(network_delegate()));
DCHECK(set_protocol);
// TODO(shalev): Without a network_delegate this protocol handler will never
// handle file: requests, but as a side effect it makes
// job_factory::IsHandledProtocol return true, which prevents attempts to
// handle the protocol externally.
set_protocol = extensions_job_factory_->SetProtocolHandler(
chrome::kFileScheme, new net::FileProtocolHandler(NULL));
DCHECK(set_protocol);

set_protocol = main_job_factory_->SetProtocolHandler(
chrome::kChromeDevToolsScheme,
CreateDevToolsProtocolHandler(chrome_url_data_manager_backend(),
network_delegate()));
DCHECK(set_protocol);
set_protocol = extensions_job_factory_->SetProtocolHandler(
chrome::kChromeDevToolsScheme,
CreateDevToolsProtocolHandler(chrome_url_data_manager_backend(), NULL));
DCHECK(set_protocol);

net::URLRequestJobFactory* job_factories[2];
job_factories[0] = main_job_factory_.get();
job_factories[1] = extensions_job_factory_.get();
Expand Down
31 changes: 31 additions & 0 deletions chrome/browser/profiles/profile_impl_io_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "chrome/browser/net/sqlite_server_bound_cert_store.h"
#include "chrome/browser/prefs/pref_member.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/webui/chrome_url_data_manager_backend.h"
#include "chrome/common/chrome_constants.h"
#include "chrome/common/chrome_notification_types.h"
#include "chrome/common/chrome_switches.h"
Expand All @@ -32,6 +33,7 @@
#include "net/base/server_bound_cert_service.h"
#include "net/ftp/ftp_network_layer.h"
#include "net/http/http_cache.h"
#include "net/url_request/file_protocol_handler.h"
#include "net/url_request/ftp_protocol_handler.h"
#include "net/url_request/url_request_job_factory.h"
#include "webkit/quota/special_storage_policy.h"
Expand Down Expand Up @@ -449,6 +451,35 @@ void ProfileImplIOData::LazyInitializeInternal(
media_request_job_factory_.reset(new net::URLRequestJobFactory);
extensions_job_factory_.reset(new net::URLRequestJobFactory);

int set_protocol = main_job_factory_->SetProtocolHandler(
chrome::kFileScheme, new net::FileProtocolHandler(network_delegate()));
DCHECK(set_protocol);
set_protocol = media_request_job_factory_->SetProtocolHandler(
chrome::kFileScheme, new net::FileProtocolHandler(network_delegate()));
DCHECK(set_protocol);
// TODO(shalev): Without a network_delegate this protocol handler will never
// handle file: requests, but as a side effect it makes
// job_factory::IsHandledProtocol return true, which prevents attempts to
// handle the protocol externally.
set_protocol = extensions_job_factory_->SetProtocolHandler(
chrome::kFileScheme, new net::FileProtocolHandler(NULL));
DCHECK(set_protocol);

set_protocol = main_job_factory_->SetProtocolHandler(
chrome::kChromeDevToolsScheme,
CreateDevToolsProtocolHandler(chrome_url_data_manager_backend(),
network_delegate()));
DCHECK(set_protocol);
set_protocol = media_request_job_factory_->SetProtocolHandler(
chrome::kChromeDevToolsScheme,
CreateDevToolsProtocolHandler(chrome_url_data_manager_backend(),
network_delegate()));
DCHECK(set_protocol);
set_protocol = extensions_job_factory_->SetProtocolHandler(
chrome::kChromeDevToolsScheme,
CreateDevToolsProtocolHandler(chrome_url_data_manager_backend(), NULL));
DCHECK(set_protocol);

net::URLRequestJobFactory* job_factories[3];
job_factories[0] = main_job_factory_.get();
job_factories[1] = media_request_job_factory_.get();
Expand Down
4 changes: 0 additions & 4 deletions chrome/browser/profiles/profile_io_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -567,10 +567,6 @@ void ProfileIOData::SetUpJobFactoryDefaults(
ChromeURLDataManagerBackend::CreateProtocolHandler(
chrome_url_data_manager_backend_.get()));
DCHECK(set_protocol);
set_protocol = job_factory->SetProtocolHandler(
chrome::kChromeDevToolsScheme,
CreateDevToolsProtocolHandler(chrome_url_data_manager_backend_.get()));
DCHECK(set_protocol);
set_protocol = job_factory->SetProtocolHandler(
chrome::kDataScheme, new net::DataProtocolHandler());
DCHECK(set_protocol);
Expand Down
20 changes: 13 additions & 7 deletions chrome/browser/ui/webui/chrome_url_data_manager_backend.cc
Original file line number Diff line number Diff line change
Expand Up @@ -568,21 +568,26 @@ bool IsSupportedURL(const GURL& url, FilePath* path) {
class DevToolsJobFactory
: public net::URLRequestJobFactory::ProtocolHandler {
public:
explicit DevToolsJobFactory(ChromeURLDataManagerBackend* backend);
DevToolsJobFactory(ChromeURLDataManagerBackend* backend,
net::NetworkDelegate* network_delegate);
virtual ~DevToolsJobFactory();

virtual net::URLRequestJob* MaybeCreateJob(
net::URLRequest* request) const OVERRIDE;

private:
// |backend_| is owned by ProfileIOData, which owns this ProtocolHandler.
// |backend_| and |network_delegate_| are owned by ProfileIOData, which owns
// this ProtocolHandler.
ChromeURLDataManagerBackend* const backend_;
net::NetworkDelegate* network_delegate_;

DISALLOW_COPY_AND_ASSIGN(DevToolsJobFactory);
};

DevToolsJobFactory::DevToolsJobFactory(ChromeURLDataManagerBackend* backend)
: backend_(backend) {
DevToolsJobFactory::DevToolsJobFactory(ChromeURLDataManagerBackend* backend,
net::NetworkDelegate* network_delegate)
: backend_(backend),
network_delegate_(network_delegate) {
DCHECK(backend_);
}

Expand All @@ -593,7 +598,7 @@ DevToolsJobFactory::MaybeCreateJob(net::URLRequest* request) const {
if (ShouldLoadFromDisk()) {
FilePath path;
if (IsSupportedURL(request->url(), &path))
return new net::URLRequestFileJob(request, path);
return new net::URLRequestFileJob(request, path, network_delegate_);
}

return new URLRequestChromeJob(request, backend_);
Expand All @@ -602,6 +607,7 @@ DevToolsJobFactory::MaybeCreateJob(net::URLRequest* request) const {
} // namespace

net::URLRequestJobFactory::ProtocolHandler*
CreateDevToolsProtocolHandler(ChromeURLDataManagerBackend* backend) {
return new DevToolsJobFactory(backend);
CreateDevToolsProtocolHandler(ChromeURLDataManagerBackend* backend,
net::NetworkDelegate* network_delegate) {
return new DevToolsJobFactory(backend, network_delegate);
}
4 changes: 3 additions & 1 deletion chrome/browser/ui/webui/chrome_url_data_manager_backend.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class RefCountedMemory;
}

namespace net {
class NetworkDelegate;
class URLRequest;
class URLRequestJob;
}
Expand Down Expand Up @@ -85,6 +86,7 @@ class ChromeURLDataManagerBackend {
};

net::URLRequestJobFactory::ProtocolHandler*
CreateDevToolsProtocolHandler(ChromeURLDataManagerBackend* backend);
CreateDevToolsProtocolHandler(ChromeURLDataManagerBackend* backend,
net::NetworkDelegate* network_delegate);

#endif // CHROME_BROWSER_UI_WEBUI_CHROME_URL_DATA_MANAGER_BACKEND_H_
5 changes: 4 additions & 1 deletion content/test/net/url_request_mock_http_job.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "content/public/common/url_constants.h"
#include "net/base/net_util.h"
#include "net/http/http_response_headers.h"
#include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_filter.h"

const char kMockHostname[] = "mock.http";
Expand Down Expand Up @@ -82,7 +83,9 @@ FilePath URLRequestMockHTTPJob::GetOnDiskPath(const FilePath& base_path,

URLRequestMockHTTPJob::URLRequestMockHTTPJob(net::URLRequest* request,
const FilePath& file_path)
: net::URLRequestFileJob(request, file_path) { }
: net::URLRequestFileJob(request,
file_path,
request->context()->network_delegate()) { }

// Public virtual version.
void URLRequestMockHTTPJob::GetResponseInfo(net::HttpResponseInfo* info) {
Expand Down
2 changes: 2 additions & 0 deletions net/net.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,8 @@
'udp/udp_socket_win.h',
'url_request/data_protocol_handler.cc',
'url_request/data_protocol_handler.h',
'url_request/file_protocol_handler.cc',
'url_request/file_protocol_handler.h',
'url_request/fraudulent_certificate_reporter.h',
'url_request/ftp_protocol_handler.cc',
'url_request/ftp_protocol_handler.h',
Expand Down
48 changes: 48 additions & 0 deletions net/url_request/file_protocol_handler.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "net/url_request/file_protocol_handler.h"

#include "base/logging.h"
#include "net/base/net_errors.h"
#include "net/base/net_util.h"
#include "net/url_request/url_request_error_job.h"
#include "net/url_request/url_request_file_dir_job.h"
#include "net/url_request/url_request_file_job.h"

namespace net {

FileProtocolHandler::FileProtocolHandler(
NetworkDelegate* network_delegate)
: network_delegate_(network_delegate) {
}

URLRequestJob* FileProtocolHandler::MaybeCreateJob(URLRequest* request) const {
FilePath file_path;
const bool is_file = FileURLToFilePath(request->url(), &file_path);

// Check file access permissions.
if (!network_delegate_ ||
!network_delegate_->CanAccessFile(*request, file_path)) {
return new URLRequestErrorJob(request, ERR_ACCESS_DENIED);
}

// We need to decide whether to create URLRequestFileJob for file access or
// URLRequestFileDirJob for directory access. To avoid accessing the
// filesystem, we only look at the path string here.
// The code in the URLRequestFileJob::Start() method discovers that a path,
// which doesn't end with a slash, should really be treated as a directory,
// and it then redirects to the URLRequestFileDirJob.
if (is_file &&
file_util::EndsWithSeparator(file_path) &&
file_path.IsAbsolute()) {
return new URLRequestFileDirJob(request, file_path);
}

// Use a regular file request job for all non-directories (including invalid
// file names).
return new URLRequestFileJob(request, file_path, network_delegate_);
}

} // namespace net
33 changes: 33 additions & 0 deletions net/url_request/file_protocol_handler.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef NET_URL_REQUEST_FILE_PROTOCOL_HANDLER_H_
#define NET_URL_REQUEST_FILE_PROTOCOL_HANDLER_H_

#include "base/basictypes.h"
#include "base/compiler_specific.h"
#include "net/url_request/url_request_job_factory.h"

namespace net {

class NetworkDelegate;
class URLRequestJob;

// Implements a ProtocolHandler for File jobs. If |network_delegate_| is NULL,
// then all file requests will fail with ERR_ACCESS_DENIED.
class NET_EXPORT FileProtocolHandler :
public URLRequestJobFactory::ProtocolHandler {
public:
explicit FileProtocolHandler(NetworkDelegate* network_delegate);
virtual URLRequestJob* MaybeCreateJob(URLRequest* request) const OVERRIDE;

private:
NetworkDelegate* network_delegate_;

DISALLOW_COPY_AND_ASSIGN(FileProtocolHandler);
};

} // namespace net

#endif // NET_URL_REQUEST_FILE_PROTOCOL_HANDLER_H_
22 changes: 9 additions & 13 deletions net/url_request/url_request_file_job.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,9 @@ class URLRequestFileJob::AsyncResolver
};

URLRequestFileJob::URLRequestFileJob(URLRequest* request,
const FilePath& file_path)
: URLRequestJob(request, request->context()->network_delegate()),
const FilePath& file_path,
NetworkDelegate* network_delegate)
: URLRequestJob(request, network_delegate),
file_path_(file_path),
stream_(NULL),
is_directory_(false),
Expand All @@ -99,8 +100,11 @@ URLRequestJob* URLRequestFileJob::Factory(URLRequest* request,
const bool is_file = FileURLToFilePath(request->url(), &file_path);

// Check file access permissions.
if (!IsFileAccessAllowed(*request, file_path))
if (!request->context()->network_delegate() ||
!request->context()->network_delegate()->CanAccessFile(
*request, file_path)) {
return new URLRequestErrorJob(request, ERR_ACCESS_DENIED);
}

// We need to decide whether to create URLRequestFileJob for file access or
// URLRequestFileDirJob for directory access. To avoid accessing the
Expand All @@ -115,7 +119,8 @@ URLRequestJob* URLRequestFileJob::Factory(URLRequest* request,

// Use a regular file request job for all non-directories (including invalid
// file names).
return new URLRequestFileJob(request, file_path);
return new URLRequestFileJob(
request, file_path, request->context()->network_delegate());
}

void URLRequestFileJob::Start() {
Expand Down Expand Up @@ -250,15 +255,6 @@ void URLRequestFileJob::SetExtraRequestHeaders(
}
}

// static
bool URLRequestFileJob::IsFileAccessAllowed(const URLRequest& request,
const FilePath& path) {
const NetworkDelegate* delegate = request.context()->network_delegate();
if (delegate)
return delegate->CanAccessFile(request, path);
return false;
}

URLRequestFileJob::~URLRequestFileJob() {
DCHECK(!async_resolver_);
}
Expand Down
10 changes: 3 additions & 7 deletions net/url_request/url_request_file_job.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ namespace net {
// A request job that handles reading file URLs
class NET_EXPORT URLRequestFileJob : public URLRequestJob {
public:
URLRequestFileJob(URLRequest* request, const FilePath& file_path);
URLRequestFileJob(URLRequest* request,
const FilePath& file_path,
NetworkDelegate* network_delegate);

static URLRequest::ProtocolFactory Factory;

Expand Down Expand Up @@ -52,12 +54,6 @@ class NET_EXPORT URLRequestFileJob : public URLRequestJob {
FilePath file_path_;

private:
// Tests to see if access to |path| is allowed. If g_allow_file_access_ is
// true, then this will return true. If the NetworkDelegate associated with
// the |request| says it's OK, then this will also return true.
static bool IsFileAccessAllowed(const URLRequest& request,
const FilePath& path);

// Callback after fetching file info on a background thread.
void DidResolve(bool exists, const base::PlatformFileInfo& file_info);

Expand Down
Loading

0 comments on commit 65dcdc5

Please sign in to comment.