content::ResourceDispatcherHostImpl changes for stale-while-revalidate

content/browser/loader/async_revalidation_driver.h

+// Copyright 2015 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_LOADER_ASYNC_REVALIDATION_DRIVER_H_
+#define CONTENT_BROWSER_LOADER_ASYNC_REVALIDATION_DRIVER_H_
+
+#include <string>
+
+#include "base/callback.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "base/timer/timer.h"
+#include "content/common/content_export.h"
+#include "content/public/browser/resource_controller.h"
+#include "content/public/browser/resource_throttle.h"
+#include "net/base/io_buffer.h"
+#include "net/url_request/url_request.h"
+
+namespace net {
+
+class HttpCache;
+
+}  // namespace net

[davidben, 2015/10/08 21:57:51]

If there's just a few forward decls, I think we prefer without the comment and
newline, so

namespace net {
class HttpCache;
}

[Adam Rice, 2015/10/13 22:53:16]

Done.

+
+namespace content {
+
+class ResourceContext;
+
+// The key used to lookup an AsyncRevalidationDriver in a map. This key has a
+// distinct value for every in-progress async revalidation.
+struct AsyncRevalidationKey {
+  AsyncRevalidationKey(const ResourceContext* resource_context,
+                       const net::HttpCache* http_cache,
+                       const GURL& url);
+
+  // Create a prefix key that is used to match all of the
+  // AsyncRevalidationDrivers using |resource_context| in the map.
+  explicit AsyncRevalidationKey(const ResourceContext* resource_context);
+
+  // The key for a map needs to be copyable.
+  AsyncRevalidationKey(const AsyncRevalidationKey& rhs);
+  ~AsyncRevalidationKey();
+
+  // No operator= is generated because the struct members are immutable.
+
+  // |resource_context| and |http_cache| are never dereferenced; they are only
+  // compared to other values. We need to be able to find and delete all
+  // in-progress async revalidations using a particular ResourceContext, so that
+  // forms the first part of the key to make that operation efficient.

[davidben, 2015/10/08 21:57:51]

It would be better to make this key into a per-URLRequestContext object.

[Adam Rice, 2015/10/13 22:53:16]

I don't understand this comment. Each request must have a distinct key, so how
can it be a per-URLRequestContext object?

+  const ResourceContext* const resource_context;
+
+  // TODO(ricea): Check the assumption that different ResourceContext objects
+  // always have a distinct HttpCache.

[davidben, 2015/10/08 21:57:51]

A ResourceContext doesn't have an HttpCache. There are multiple
URLRequestContexts per ResourceContext. Both because there is both the normal
and the media URLRequestContext and because a BrowserContext has multiple
StoragePartitions. (I want to make there be one ResourceContext per
StoragePartition, but that's not how it works today.)

[Adam Rice, 2015/10/13 22:53:16]

This comment was confusing, sorry. I have verified that the HttpCache objects
are owned by the ResourceContexts and not shared between them.

+  const net::HttpCache* const http_cache;
+
+  // Derived from the url via net::HttpUtil::SpecForRequest().
+  const std::string url_key;
+
+  // Overloading operator< is bad, according to
+  // http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Operator_Overloading
+  // (accessed on 2014-07-17), so we have to overload operator() instead.

[davidben, 2015/10/08 21:57:51]

This comment is unnecessary.

[Adam Rice, 2015/10/13 22:53:16]

Removed.

+  struct LessThan {
+    bool operator()(const AsyncRevalidationKey& lhs,
+                    const AsyncRevalidationKey& rhs) const {
+      if (lhs.resource_context != rhs.resource_context)
+        return lhs.resource_context < rhs.resource_context;
+
+      if (lhs.http_cache != rhs.http_cache)
+        return lhs.http_cache < rhs.http_cache;
+
+      return lhs.url_key < rhs.url_key;
+    }
+  };
+};
+
+// This class is responsible for driving the URLRequest for an async
+// revalidation. It is passed an instance of ResourceThrottle created by
+// content::ResourceScheduler to perform throttling on the request.
+class CONTENT_EXPORT AsyncRevalidationDriver : public net::URLRequest::Delegate,
+                                               public ResourceController {
+ public:
+  // |completion_callback| is guaranteed to be called on completion,
+  // regardless of success or failure.
+  AsyncRevalidationDriver(scoped_ptr<net::URLRequest> request,
+                          scoped_ptr<ResourceThrottle> throttle,
+                          const base::Closure& completion_callback);
+  ~AsyncRevalidationDriver() override;
+
+  void StartRequest();
+  void CancelRequest();
+
+  //  net::URLRequest* request() { return request_.get(); }

[davidben, 2015/10/08 21:57:51]

?

[Adam Rice, 2015/10/13 22:53:16]

Good question. Removed.

+
+ private:
+  enum DeferredStage {
+    DEFERRED_NONE,
+    DEFERRED_START,
+    DEFERRED_NETWORK_START,
+    DEFERRED_READ,
+  };
+
+  // net::URLRequest::Delegate implementation:
+  void OnReceivedRedirect(net::URLRequest* request,
+                          const net::RedirectInfo& redirect_info,
+                          bool* defer) override;
+  void OnAuthRequired(net::URLRequest* request,
+                      net::AuthChallengeInfo* info) override;
+  void OnCertificateRequested(net::URLRequest* request,
+                              net::SSLCertRequestInfo* info) override;
+  void OnSSLCertificateError(net::URLRequest* request,
+                             const net::SSLInfo& info,
+                             bool fatal) override;
+  void OnBeforeNetworkStart(net::URLRequest* request, bool* defer) override;
+  void OnResponseStarted(net::URLRequest* request) override;
+  void OnReadCompleted(net::URLRequest* request, int bytes_read) override;
+
+  // ResourceController implementation:
+  void Resume() override;
+
+  // For simplicity, this class assumes that ResourceScheduler never cancels
+  // requests, and so these three methods are never called.
+  void Cancel() override;
+  void CancelAndIgnore() override;
+  void CancelWithError(int error_code) override;
+
+  // Internal methods.
+  void StartRequestInternal();
+  void CancelRequestInternal(int error);
+  void CompleteResponseStarted();
+  void StartReading(bool is_continuation);
+  void ResumeReading();
+  void ReadMore(int* bytes_read);
+  // Calls |completion_callback_|.
+  void ResponseCompleted();
+  void OnReadTimeout();
+  void RecordDefer(DeferredStage deferred_stage);
+
+  bool is_deferred() const { return deferred_stage_ != DEFERRED_NONE; }
+
+  DeferredStage deferred_stage_ = DEFERRED_NONE;
+
+  // When this class cancels a redirect, URLRequest calls both the
+  // OnResponseStarted() and OnReadCompleted() callbacks. This class should not
+  // run |completion_callback_|.
+  //
+  // TODO(ricea): Work out why URLRequest calls both methods on cancellation and
+  // make it stop.

[davidben, 2015/10/08 21:57:51]

OnResponseStarted should happen. That's to signal that Start() completed,
success or no. You check the URLRequest's status. Not sure about
OnReadCompleted, but that deserves investigation.

(That we signal back to you again when you cancel a URLRequest is kinda odd, but
how the API works... I wish it didn't, but that would require we fix all code
which externally cancels a URLRequest outside of is owner.)

+  bool has_already_failed_ = false;
+
+  scoped_refptr<net::IOBuffer> read_buffer_;
+  base::Timer read_timer_;
+
+  const scoped_ptr<net::URLRequest> request_;
+  scoped_ptr<ResourceThrottle> throttle_;
+
+  base::Closure completion_callback_;
+
+  base::WeakPtrFactory<AsyncRevalidationDriver> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(AsyncRevalidationDriver);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_LOADER_ASYNC_REVALIDATION_DRIVER_H_