Reduce/Remove URLRequest dependencies from AppCacheRequestHandler

content/browser/appcache/appcache_request.h

+// 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;
+
+  virtual const std::string& GetMethod() const = 0;
+
+  virtual const GURL& GetFirstPartyForCookies() const = 0;

[michaeln, 2017/05/01 20:53:58]

style nit: you could remove some blank lines and smush these getters closer
together

[ananta, 2017/05/03 00:16:47]

The functions were supposed to have comments. Fixed now

+
+  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;
+
+  // Getters for the request types we currently support.

[michaeln, 2017/05/01 20:53:58]

It might be nice to make these private to dissuade their use general code? Of
course it makes sense for the AppCacheUrlRequestJob to be able to access
GetURLRequest(), but we need to avoid the dependency in general.

[ananta, 2017/05/03 00:16:47]

Moved them to the protected section.

+  virtual net::URLRequest* GetURLRequest() const { return nullptr; }
+
+  virtual const ResourceRequest* GetResourceRequest() const { return nullptr; }
+
+  // Returns true if the scheme and method are supported for AppCache.
+  static bool IsSchemeAndMethodSupportedForAppCache(
+      const AppCacheRequest* request);
+
+ protected:
+  AppCacheRequest() {}
+
+  DISALLOW_COPY_AND_ASSIGN(AppCacheRequest);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_APPCACHE_APPCACHE_REQUEST_H_