Resetting the HttpRequestInfo pointers in HttpNetworkTransaction and streams

content/browser/loader/resource_dispatcher_host_unittest.cc

Index: content/browser/loader/resource_dispatcher_host_unittest.cc
diff --git a/content/browser/loader/resource_dispatcher_host_unittest.cc b/content/browser/loader/resource_dispatcher_host_unittest.cc
index 53a49ad78a0f6e96f91435b3f6b119152533df9a..12c260db551d36eefe1ed11a946a646c0d4a2d38 100644
--- a/content/browser/loader/resource_dispatcher_host_unittest.cc
+++ b/content/browser/loader/resource_dispatcher_host_unittest.cc
@@ -60,6 +60,7 @@
 #include "content/public/test/test_renderer_host.h"
 #include "content/test/test_content_browser_client.h"
 #include "content/test/test_navigation_url_loader_delegate.h"
+#include "net/base/chunked_upload_data_stream.h"
 #include "net/base/elements_upload_data_stream.h"
 #include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
@@ -504,18 +505,13 @@ class URLRequestLoadInfoJob : public net::URLRequestJob {
  public:
   URLRequestLoadInfoJob(net::URLRequest* request,
                         net::NetworkDelegate* network_delegate,
-                        const net::LoadState& load_state,
-                        const net::UploadProgress& upload_progress)
+                        const net::LoadState& load_state)
       : net::URLRequestJob(request, network_delegate),
-        load_state_(load_state),
-        upload_progress_(upload_progress) {}
+        load_state_(load_state) {}
 
   // net::URLRequestJob implementation:
   void Start() override {}
   net::LoadState GetLoadState() const override { return load_state_; }
-  net::UploadProgress GetUploadProgress() const override {
-    return upload_progress_;
-  }
 
  private:
   ~URLRequestLoadInfoJob() override {}
@@ -533,7 +529,6 @@ class URLRequestLoadInfoJob : public net::URLRequestJob {
   }
 
   const net::LoadState load_state_;
-  const net::UploadProgress upload_progress_;
 };
 
 class ResourceDispatcherHostTest;
@@ -887,6 +882,34 @@ void CheckRequestCompleteErrorCode(const IPC::Message& message,
   ASSERT_EQ(expected_error_code, error_code);
 }
 
+// Class to test LoadInfo UploadProgress.
+class LoadInfoTestUploadStream : public net::UploadDataStream {
+ public:
+  LoadInfoTestUploadStream(net::UploadProgress progress)
+      : net::UploadDataStream(false, 0), progress_(progress), stream_(0) {}
+  net::UploadProgress GetUploadProgress() const override { return progress_; }
+
+ private:
+  int InitInternal(const net::BoundNetLog& net_log) override {
+    stream_.Init(base::Bind(&LoadInfoTestUploadStream::OnInitCompleted,
+                            base::Unretained(this)),
+                 net_log);
+    return net::OK;
+  }
+
+  int ReadInternal(net::IOBuffer* buf, int buf_len) override {
+    return stream_.Read(buf, buf_len,
+                        base::Bind(&LoadInfoTestUploadStream::OnReadCompleted,
+                                   base::Unretained(this)));
+  }

[Randy Smith (Not in Mondays), 2016/09/09 20:56:43]

nit, suggestion: Is there an existing test that makes sure these interfaces get
exercised?  (I.e. that a URLRequest with this set as it's upload data stream
will actually try to upload some data?)   My guess is "no" since it doesn't look
like the URLRequests used for this test had an UploadDataStream set for them,
which makes me worried that some future test writer will use this class,
assuming it'll work, and it won't.  If I'm right on this (I'm not certain) would
you be willing to write a stub test using this class or use NOT_REACHED() inside
the bodies of the {Init,Read}Internal functions to signal clearly to future
modifiers what this class will and won't do?

[shivanisha, 2016/09/13 19:58:34]

This code is no longer needed as a result of an independent CL which changed the
way these tests work. Also, Upload plumbing removal now being reviewed in
https://codereview.chromium.org/2298823002/

+
+  void ResetInternal() override { stream_.Reset(); }
+
+  net::UploadProgress progress_;
+  net::ChunkedUploadDataStream stream_;
+  DISALLOW_COPY_AND_ASSIGN(LoadInfoTestUploadStream);
+};
+
 class ResourceDispatcherHostTest : public testing::TestWithParam<TestConfig>,
                                    public IPC::Sender {
  public:
@@ -942,7 +965,16 @@ class ResourceDispatcherHostTest : public testing::TestWithParam<TestConfig>,
       MakeTestRequest(request_info[i].route_id, i + 1, request_info[i].url);
       wait_for_request_create_loop_->Run();
       wait_for_request_create_loop_.reset();
+
+      // set upload progress
+      net::URLRequest* url_request =
+          ResourceDispatcherHostImpl::Get()->GetURLRequest(
+              GlobalRequestID(filter_->child_id(), i + 1));
+      EXPECT_TRUE(url_request != nullptr);
+      url_request->set_upload(base::MakeUnique<LoadInfoTestUploadStream>(
+          request_info[i].upload_progress));
     }
+
     return ResourceDispatcherHostImpl::Get()->GetLoadInfoForAllRoutes();
   }
 
@@ -3758,7 +3790,7 @@ net::URLRequestJob* TestURLRequestJobFactory::MaybeCreateJobWithProtocolHandler(
     std::unique_ptr<LoadInfoTestRequestInfo> info =
         std::move(test_fixture_->loader_test_request_info_);
     return new URLRequestLoadInfoJob(request, network_delegate,
-                                     info->load_state, info->upload_progress);
+                                     info->load_state);
   }
   if (hang_after_start_) {
     return new net::URLRequestFailedJob(request, network_delegate,