Skip to content

Commit

Permalink
Reduce/Remove URLRequest dependencies from AppCacheRequestHandler
Browse files Browse the repository at this point in the history
The AppCacheRequestHandler class provides functionality for serving network requests from
the AppCache if applicable. This class is instantiated by the AppCacheInterceptor when a network request
is initiated. Its lifetime depends on the associated URLRequest.

The plan is to reuse this class in the network service world where we won't be intercepting
network requests in the browser process. This effectively means that there won't be any URLRequest as well

To achieve this the proposal is to provide an abstraction for a request called AppCachRequest. This class
will have two subclasses at the moment. AppCacheURLRequest and AppCacheURLLoaderRequest. The AppCacheURLRequest
class will be used by the current network implementation which relies on intercepting n/w requests.

The AppCacheURLLoaderRequest class will be used by the network service mojo implementation.
Next step is to provide an abstraction for the AppCacheURLRequestJob class.

BUG=715632
TBR=jam

Review-Url: https://codereview.chromium.org/2848493007
Cr-Commit-Position: refs/heads/master@{#469254}
  • Loading branch information
ananta authored and Commit bot committed May 4, 2017
1 parent 2d7e67c commit 4e8a529
Show file tree
Hide file tree
Showing 15 changed files with 523 additions and 170 deletions.
6 changes: 6 additions & 0 deletions content/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,8 @@ source_set("browser") {
"appcache/appcache_policy.h",
"appcache/appcache_quota_client.cc",
"appcache/appcache_quota_client.h",
"appcache/appcache_request.cc",
"appcache/appcache_request.h",
"appcache/appcache_request_handler.cc",
"appcache/appcache_request_handler.h",
"appcache/appcache_response.cc",
Expand All @@ -323,6 +325,10 @@ source_set("browser") {
"appcache/appcache_storage_impl.h",
"appcache/appcache_update_job.cc",
"appcache/appcache_update_job.h",
"appcache/appcache_url_loader_request.cc",
"appcache/appcache_url_loader_request.h",
"appcache/appcache_url_request.cc",
"appcache/appcache_url_request.h",
"appcache/appcache_url_request_job.cc",
"appcache/appcache_url_request_job.h",
"appcache/appcache_working_set.cc",
Expand Down
15 changes: 8 additions & 7 deletions content/browser/appcache/appcache_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "content/browser/appcache/appcache.h"
#include "content/browser/appcache/appcache_backend_impl.h"
#include "content/browser/appcache/appcache_policy.h"
#include "content/browser/appcache/appcache_request.h"
#include "content/browser/appcache/appcache_request_handler.h"
#include "net/url_request/url_request.h"
#include "storage/browser/quota/quota_manager_proxy.h"
Expand Down Expand Up @@ -312,29 +313,29 @@ AppCacheHost* AppCacheHost::GetParentAppCacheHost() const {
}

std::unique_ptr<AppCacheRequestHandler> AppCacheHost::CreateRequestHandler(
net::URLRequest* request,
std::unique_ptr<AppCacheRequest> request,
ResourceType resource_type,
bool should_reset_appcache) {
if (is_for_dedicated_worker()) {
AppCacheHost* parent_host = GetParentAppCacheHost();
if (parent_host)
return parent_host->CreateRequestHandler(
request, resource_type, should_reset_appcache);
std::move(request), resource_type, should_reset_appcache);
return NULL;
}

if (AppCacheRequestHandler::IsMainResourceType(resource_type)) {
// Store the first party origin so that it can be used later in SelectCache
// for checking whether the creation of the appcache is allowed.
first_party_url_ = request->first_party_for_cookies();
return base::WrapUnique(
new AppCacheRequestHandler(this, resource_type, should_reset_appcache));
first_party_url_ = request->GetFirstPartyForCookies();
return base::WrapUnique(new AppCacheRequestHandler(
this, resource_type, should_reset_appcache, std::move(request)));
}

if ((associated_cache() && associated_cache()->is_complete()) ||
is_selection_pending()) {
return base::WrapUnique(
new AppCacheRequestHandler(this, resource_type, should_reset_appcache));
return base::WrapUnique(new AppCacheRequestHandler(
this, resource_type, should_reset_appcache, std::move(request)));
}
return NULL;
}
Expand Down
3 changes: 2 additions & 1 deletion content/browser/appcache/appcache_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class AppCache;
class AppCacheFrontend;
class AppCacheGroupTest;
class AppCacheHostTest;
class AppCacheRequest;
class AppCacheRequestHandler;
class AppCacheRequestHandlerTest;
class AppCacheStorageImplTest;
Expand Down Expand Up @@ -116,7 +117,7 @@ class CONTENT_EXPORT AppCacheHost
// Support for loading resources out of the appcache.
// May return NULL if the request isn't subject to retrieval from an appache.
std::unique_ptr<AppCacheRequestHandler> CreateRequestHandler(
net::URLRequest* request,
std::unique_ptr<AppCacheRequest> request,
ResourceType resource_type,
bool should_reset_appcache);

Expand Down
11 changes: 6 additions & 5 deletions content/browser/appcache/appcache_interceptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "content/browser/appcache/appcache_host.h"
#include "content/browser/appcache/appcache_request_handler.h"
#include "content/browser/appcache/appcache_service_impl.h"
#include "content/browser/appcache/appcache_url_request.h"
#include "content/browser/appcache/appcache_url_request_job.h"
#include "content/browser/appcache/chrome_appcache_service.h"
#include "content/browser/bad_message.h"
Expand Down Expand Up @@ -65,7 +66,8 @@ void AppCacheInterceptor::SetExtraRequestInfoForHost(
bool should_reset_appcache) {
// Create a handler for this request and associate it with the request.
std::unique_ptr<AppCacheRequestHandler> handler =
host->CreateRequestHandler(request, resource_type, should_reset_appcache);
host->CreateRequestHandler(AppCacheURLRequest::Create(request),
resource_type, should_reset_appcache);
if (handler)
SetHandler(request, std::move(handler));
}
Expand Down Expand Up @@ -141,7 +143,7 @@ net::URLRequestJob* AppCacheInterceptor::MaybeInterceptRequest(
AppCacheRequestHandler* handler = GetHandler(request);
if (!handler)
return NULL;
return handler->MaybeLoadResource(request, network_delegate);
return handler->MaybeLoadResource(network_delegate);
}

net::URLRequestJob* AppCacheInterceptor::MaybeInterceptRedirect(
Expand All @@ -151,16 +153,15 @@ net::URLRequestJob* AppCacheInterceptor::MaybeInterceptRedirect(
AppCacheRequestHandler* handler = GetHandler(request);
if (!handler)
return NULL;
return handler->MaybeLoadFallbackForRedirect(
request, network_delegate, location);
return handler->MaybeLoadFallbackForRedirect(network_delegate, location);
}

net::URLRequestJob* AppCacheInterceptor::MaybeInterceptResponse(
net::URLRequest* request, net::NetworkDelegate* network_delegate) const {
AppCacheRequestHandler* handler = GetHandler(request);
if (!handler)
return NULL;
return handler->MaybeLoadFallbackForResponse(request, network_delegate);
return handler->MaybeLoadFallbackForResponse(network_delegate);
}

} // namespace content
26 changes: 26 additions & 0 deletions content/browser/appcache/appcache_request.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright (c) 2017 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 "content/browser/appcache/appcache_request.h"
#include "content/common/appcache_interfaces.h"
#include "net/url_request/url_request.h"

namespace content {

// static
bool AppCacheRequest::IsSchemeAndMethodSupportedForAppCache(
const AppCacheRequest* request) {
return IsSchemeSupportedForAppCache(request->GetURL()) &&
IsMethodSupportedForAppCache(request->GetMethod());
}

net::URLRequest* AppCacheRequest::GetURLRequest() {
return nullptr;
}

ResourceRequest* AppCacheRequest::GetResourceRequest() {
return nullptr;
}

} // namespace content
79 changes: 79 additions & 0 deletions content/browser/appcache/appcache_request.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Copyright (c) 2017 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 CONTENT_BROWSER_APPCACHE_APPCACHE_REQUEST_H_
#define CONTENT_BROWSER_APPCACHE_APPCACHE_REQUEST_H_

#include "base/logging.h"
#include "base/strings/string16.h"
#include "base/threading/non_thread_safe.h"
#include "content/common/content_export.h"
#include "url/gurl.h"

namespace net {
class URLRequest;
}

namespace content {
struct ResourceRequest;

// Interface for an AppCache request. Subclasses implement this interface to
// wrap custom request objects like URLRequest, etc to ensure that these
// dependencies stay out of the AppCache code.
class CONTENT_EXPORT AppCacheRequest
: NON_EXPORTED_BASE(public base::NonThreadSafe) {
public:
virtual ~AppCacheRequest() {}

// The URL for this request.
virtual const GURL& GetURL() const = 0;

// The method for this request
virtual const std::string& GetMethod() const = 0;

// Used for cookie policy.
virtual const GURL& GetFirstPartyForCookies() const = 0;

// The referrer for this request.
virtual const GURL GetReferrer() const = 0;

// Returns true if the request was successful.
virtual bool IsSuccess() const = 0;

// Returns true if the request was cancelled.
virtual bool IsCancelled() const = 0;

// Returns true if the request had an error.
virtual bool IsError() const = 0;

// Returns the HTTP response code.
virtual int GetResponseCode() const = 0;

// Get response header(s) by name. Returns an empty string if the header
// wasn't found,
virtual std::string GetResponseHeaderByName(
const std::string& name) const = 0;

// Returns true if the scheme and method are supported for AppCache.
static bool IsSchemeAndMethodSupportedForAppCache(
const AppCacheRequest* request);

protected:
friend class AppCacheRequestHandler;

AppCacheRequest() {}

// Getters for the request types we currently support.
virtual net::URLRequest* GetURLRequest();

// Returns the underlying ResourceRequest. Please note that only one of
// GetURLRequest() and GetResourceRequest() should return valid results.
virtual ResourceRequest* GetResourceRequest();

DISALLOW_COPY_AND_ASSIGN(AppCacheRequest);
};

} // namespace content

#endif // CONTENT_BROWSER_APPCACHE_APPCACHE_REQUEST_H_
Loading

0 comments on commit 4e8a529

Please sign in to comment.