Cronet Fix Channel Write after Close when request is canceled after success.

net/test/url_request/url_request_mock_data_job.cc

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.

[mmenke, 2014/12/09 21:00:03]

-(c), update year

[mef, 2014/12/09 22:20:59]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "net/test/url_request/url_request_mock_data_job.h"
+
+#include <vector>

[mmenke, 2014/12/09 21:00:03]

Not used.

[mef, 2014/12/09 22:20:59]

Done.

+
+#include "base/bind.h"
+#include "base/compiler_specific.h"

[mmenke, 2014/12/09 21:00:04]

Not used, I believe.

[mef, 2014/12/09 22:20:59]

Done.

+#include "base/memory/ref_counted_memory.h"

[mmenke, 2014/12/09 21:00:04]

Not used.

[mef, 2014/12/09 22:20:59]

Done.

+#include "base/message_loop/message_loop.h"
+#include "base/strings/string_number_conversions.h"
+#include "net/base/io_buffer.h"
+#include "net/base/net_errors.h"

[mmenke, 2014/12/09 21:00:04]

Not used

[mef, 2014/12/09 22:20:59]

Done.

+#include "net/base/url_util.h"
+#include "net/http/http_request_headers.h"
+#include "net/http/http_util.h"

[mmenke, 2014/12/09 21:00:03]

Is http util used?

[mef, 2014/12/09 22:20:59]

Done.

+#include "net/url_request/url_request_filter.h"
+#include "net/url_request/url_request_status.h"

[mmenke, 2014/12/09 21:00:04]

Not used.

[mef, 2014/12/09 22:20:59]

Done.

+
+namespace net {
+namespace {
+
+const char kMockHostname[] = "mock.data";
+
+// Gets the data from URL of the form:
+// scheme://kMockHostname/?data=abc&repeat_count=nnn.
+std::string GetDataFromRequest(net::URLRequest* request) {

[mmenke, 2014/12/09 21:00:03]

const net::URLRequest&, here and below?

[mef, 2014/12/09 22:20:59]

Done.

+  std::string value;
+  if (!GetValueForKeyInQuery(request->url(), "data", &value))
+    return "default_data";
+  return value;
+}
+
+// Gets the numeric repeat count from URL of the form:
+// scheme://kMockHostname/?data=abc&repeat_count=nnn.
+unsigned GetRepeatCountFromRequest(net::URLRequest* request) {
+  std::string value;
+  if (!GetValueForKeyInQuery(request->url(), "repeat", &value))
+    return 1;
+
+  unsigned repeat_count = 1;
+  if (!base::StringToUint(value, &repeat_count))
+    return 1;
+
+  return repeat_count;
+}
+
+GURL GetMockUrl(const std::string& scheme,
+                const std::string& hostname,
+                const std::string& data,
+                unsigned data_repeat_count) {
+  std::string url(scheme + "://" + hostname + "/");
+  url.append("?data=");
+  url.append(data);
+  url.append("&repeat=");
+  url.append(base::UintToString(data_repeat_count));
+  return GURL(url);
+}
+
+}  // namespace
+
+URLRequestMockDataJob::URLRequestMockDataJob(URLRequest* request,
+                                             NetworkDelegate* network_delegate,
+                                             const std::string& data,
+                                             unsigned data_repeat_count)
+    : URLRequestJob(request, network_delegate),
+      data_offset_(0),
+      weak_factory_(this) {
+  data_.reserve(data_repeat_count*data.length());

[mmenke, 2014/12/09 21:00:03]

Space before/after "*".

Actually, since this this is test-only code, suggest just getting rid of this
line.

[mef, 2014/12/09 22:20:59]

Done.

+  for (unsigned i = 0; i < data_repeat_count; ++i) {
+    data_ += data;

[mmenke, 2014/12/09 21:00:04]

Suggest using .append - think it's a little cleaner, and makes the fact the
first has an underscore stand out more.

[mef, 2014/12/09 22:20:59]

Done.

+  }
+}
+
+void URLRequestMockDataJob::Start() {
+  // Start reading asynchronously so that all error reporting and data
+  // callbacks happen as they would for network requests.
+  base::MessageLoop::current()->PostTask(
+      FROM_HERE,
+      base::Bind(&URLRequestMockDataJob::StartAsync,
+                 weak_factory_.GetWeakPtr()));
+}
+
+bool URLRequestMockDataJob::GetMimeType(std::string* mime_type) const {
+  *mime_type = mime_type_;

[mmenke, 2014/12/09 21:00:03]

Can we just use the default implementation of this method and the next?

[mef, 2014/12/09 22:20:59]

Done.

+  return true;
+}
+
+bool URLRequestMockDataJob::GetCharset(std::string* charset) {
+  *charset = charset_;
+  return true;
+}
+
+URLRequestMockDataJob::~URLRequestMockDataJob() {}
+
+bool URLRequestMockDataJob::ReadRawData(IOBuffer* buf, int buf_size,
+                                      int* bytes_read) {

[mmenke, 2014/12/09 21:00:03]

Fix indent

[mef, 2014/12/09 22:20:59]

Done.

+  DCHECK(bytes_read);
+  buf_size = static_cast<int>(std::min(

[mmenke, 2014/12/09 21:00:04]

This is a misnomer.  Suggest just setting *bytes_read directly, or creating a
new value, bytes_to_read.

[mef, 2014/12/09 22:20:59]

Done.

+      static_cast<int64>(buf_size),
+      data_.length() - data_offset_ + 1));

[mmenke, 2014/12/09 21:00:04]

Remove the "+1".  If they're the same, you're at the end.

[mef, 2014/12/09 22:20:59]

Done.

+  memcpy(buf->data(), data_.c_str() + data_offset_, buf_size);
+  data_offset_ += buf_size;
+  *bytes_read = buf_size;
+  return true;
+}
+
+void URLRequestMockDataJob::StartAsync() {
+  if (!request_)
+    return;
+
+  set_expected_content_size(data_.length());
+  NotifyHeadersComplete();
+}
+
+// static
+void URLRequestMockDataJob::AddUrlHandler() {
+  return AddUrlHandlerForHostname(kMockHostname);
+}
+
+// static
+void URLRequestMockDataJob::AddUrlHandlerForHostname(
+    const std::string& hostname) {
+  // Add |hostname| to net::URLRequestFilter for HTTP and HTTPS.
+  net::URLRequestFilter* filter = net::URLRequestFilter::GetInstance();
+  filter->AddHostnameHandler("http", hostname, URLRequestMockDataJob::Factory);
+  filter->AddHostnameHandler("https", hostname, URLRequestMockDataJob::Factory);

[mmenke, 2014/12/09 21:00:03]

Should use AddHostnameInterceptor instead - URLRequest::ProtocolFactory is
deprecated (or should be).

[mef, 2014/12/09 22:20:59]

Done.

+}
+
+// static
+GURL URLRequestMockDataJob::GetMockHttpUrl(const std::string& data,
+                                           int repeat_count) {
+  return GetMockHttpUrlForHostname(kMockHostname, data, repeat_count);
+}
+
+// static
+GURL URLRequestMockDataJob::GetMockHttpsUrl(const std::string& data,
+                                            int repeat_count) {
+  return GetMockHttpsUrlForHostname(kMockHostname, data, repeat_count);
+}
+
+// static
+GURL URLRequestMockDataJob::GetMockHttpUrlForHostname(
+    const std::string& hostname,
+    const std::string& data,
+    int repeat_count) {
+  return GetMockUrl("http", hostname, data, repeat_count);
+}
+
+// static
+GURL URLRequestMockDataJob::GetMockHttpsUrlForHostname(
+    const std::string& hostname,
+    const std::string& data,
+    int repeat_count) {
+  return GetMockUrl("https", hostname, data, repeat_count);

[mmenke, 2014/12/09 21:00:04]

repeat_count is an int, GetMockUrl is defined as taking an unsigned.  Suggest
ints in both places, since Java doesn't have unsigneds.

[mef, 2014/12/09 22:20:59]

Done.

+}
+
+// static
+net::URLRequestJob* URLRequestMockDataJob::Factory(
+    net::URLRequest* request,
+    net::NetworkDelegate* network_delegate,
+    const std::string& scheme) {
+  return new URLRequestMockDataJob(
+      request, network_delegate, GetDataFromRequest(request),
+      GetRepeatCountFromRequest(request));
+}
+
+}  // namespace net