Substitute ResourceHandlers with dummy ResourceHandler while request is transferred

content/browser/renderer_host/transferred_resource_handler.h

Index: content/browser/renderer_host/transferred_resource_handler.h
diff --git a/content/browser/renderer_host/transferred_resource_handler.h b/content/browser/renderer_host/transferred_resource_handler.h
new file mode 100644
index 0000000000000000000000000000000000000000..51233aca6eca5c985aa4de96d667ece58b4ca559
--- /dev/null
+++ b/content/browser/renderer_host/transferred_resource_handler.h
@@ -0,0 +1,62 @@
+// 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 CONTENT_BROWSER_RENDERER_HOST_TRANSFERRED_RESOURCE_HANDLER_H_
+#define CONTENT_BROWSER_RENDERER_HOST_TRANSFERRED_RESOURCE_HANDLER_H_
+#pragma once
+
+#include "base/memory/ref_counted.h"
+#include "content/browser/renderer_host/resource_handler.h"
+
+// ResourceHandler that DCHECKs on all events but canceling and failing of
+// requests. This is used for requests that are paused to be transferred to
+// a new renderer.
+class TransferredResourceHandler : public ResourceHandler {

[darin (slow to review), 2012/01/06 22:59:02]

Perhaps it would be useful to factor out an UnreachedResourceHandler, which
purely asserts NOTREACHED on all methods.  Then subclass that to special-case
the behavior of OnResponseCompleted and OnRequestClosed.

Or... maybe you could rename TransferredResourceHandler to
DoomedResourceHandler.  It expects to only receive failure and close events. 
That sort of makes sense.  It could make sense for such a resource handler to
retain a reference to the ResourceHandler it replaces.

[battre, 2012/01/06 23:37:56]

I think this is not very useful as we won't have a second class that will derive
from UnreachedResourceHandler.
Is "DoomedResourceHandler" really an appropriate name? The URLRequest is not
doomed.

This is what is going on:

1) A Renderer tries to open a website.
2) The ResourceDispatcherHost creates a URLRequest
3) We decide that the website needs to be opened in a different Renderer (due to
cross-origin navigation)
4) The ResourceDispatcherHost parks the URLRequest and asks the new Renderer to
open the same page.
5) The new Renderer sends the request to the ResourceDispatcherHost
6) The ResourceDispatcherHost recognizes the request and resume the old
URLRequest instead of creating a new one

For the time during which the URLRequest is parked, we need to assign it a
temporary ResourceHandler. The old ones depend on the old RenderProcessHost
instance that gets killed.

This ResourceHandler is really just a dummy that is replaced when the RDH
resumes the URLRequest.

I think I would consider neither the URLRequest nor the ResourceHandler as
"doomed".

I would propose "TransferredRequestResourceHandler". We have a
TransferNavigationResourceHandler that checks whether the request gets
transferred to a new renderer. The RH would handle transferred URLRequests.

Previous suggestions "TransferredResourceHandler" and
"PendingTransferredResourceHandler" would work for me as well.

WDYT? 

+ public:
+  // As the TransferredResourceHandler is constructed and subsituted from code
+  // of another ResourceHandler, we need to make sure that this other handler
+  // does not lose its last reference and gets destroyed by being substituted.
+  // Therefore, we retain a reference to |old_handlers| that prevents the
+  // destruction.
+  TransferredResourceHandler(ResourceHandler* old_handlers);

[darin (slow to review), 2012/01/06 22:59:02]

nit: why is this variable name pluralized?  it is not an array is it?

[battre, 2012/01/06 23:37:56]

It a linked chain of handlers. I don't mind using singular because we are
pointing to the first one. I will change this.

+
+  // ResourceHandler implementation:
+  virtual bool OnUploadProgress(int request_id,
+                                uint64 position,
+                                uint64 size) OVERRIDE;
+  virtual bool OnRequestRedirected(int request_id,
+                                   const GURL& new_url,
+                                   content::ResourceResponse* response,
+                                   bool* defer) OVERRIDE;
+  virtual bool OnResponseStarted(int request_id,
+                                 content::ResourceResponse* response) OVERRIDE;
+  virtual bool OnWillStart(int request_id,
+                           const GURL& url,
+                           bool* defer) OVERRIDE;
+  virtual bool OnWillRead(int request_id,
+                          net::IOBuffer** buf,
+                          int* buf_size,
+                          int min_size) OVERRIDE;
+  virtual bool OnReadCompleted(int request_id,
+                               int* bytes_read) OVERRIDE;
+  virtual bool OnResponseCompleted(int request_id,
+                                   const net::URLRequestStatus& status,
+                                   const std::string& security_info) OVERRIDE;
+  virtual void OnRequestClosed() OVERRIDE;
+  virtual void OnDataDownloaded(int request_id,
+                                int bytes_downloaded) OVERRIDE;
+
+ private:
+  virtual ~TransferredResourceHandler();
+
+  // Reference to the old ResourceHandlers that were active before this one
+  // replaced them. We need to retain a reference here because we replace
+  // the old handlers while they are being executed. Replacing them without
+  // retaining the reference might delete the last reference and destroy them.
+  scoped_refptr<ResourceHandler> old_handlers_;
+
+  DISALLOW_COPY_AND_ASSIGN(TransferredResourceHandler);
+};
+
+#endif  // CONTENT_BROWSER_RENDERER_HOST_TRANSFERRED_RESOURCE_HANDLER_H_