[android_webview] Make intercepted URLRequests have status codes.

android_webview/browser/net/android_stream_reader_url_request_job.cc

Index: android_webview/browser/net/android_stream_reader_url_request_job.cc
diff --git a/android_webview/browser/net/android_stream_reader_url_request_job.cc b/android_webview/browser/net/android_stream_reader_url_request_job.cc
index 3050094aecc419fb85cc366fa6194dc5d9be5a16..9070d0139585164b646dd6d6ded00ee8e5d1b9b0 100644
--- a/android_webview/browser/net/android_stream_reader_url_request_job.cc
+++ b/android_webview/browser/net/android_stream_reader_url_request_job.cc
@@ -4,6 +4,8 @@
 
 #include "android_webview/browser/net/android_stream_reader_url_request_job.h"
 
+#include <string>
+
 #include "android_webview/browser/input_stream.h"
 #include "android_webview/browser/net/input_stream_reader.h"
 #include "base/android/jni_android.h"
@@ -12,6 +14,7 @@
 #include "base/bind_helpers.h"
 #include "base/lazy_instance.h"
 #include "base/message_loop.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/task_runner.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "base/threading/thread.h"
@@ -20,6 +23,8 @@
 #include "net/base/mime_util.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_util.h"
+#include "net/http/http_response_headers.h"
+#include "net/http/http_response_info.h"
 #include "net/http/http_util.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_job_manager.h"
@@ -30,6 +35,16 @@ using base::android::AttachCurrentThread;
 using base::PostTaskAndReplyWithResult;
 using content::BrowserThread;
 
+namespace {
+
+const int kHTTPOk = 200;

[benm (inactive), 2013/03/06 17:05:11]

these surely must be defined somewhere in chromium already?

[mkosiba (inactive), 2013/03/07 18:52:01]

yeath.. somewhere in uh.. drive_url_request_job. I don't think this is enough
duplication that creating a separate component / spending the effort to add this
to the content/ layer is worth it.

+const int kHTTPNotFound = 404;
+
+const char kHTTPOkText[] = "OK";
+const char kHTTPNotFoundText[] = "Not Found";
+
+} // namespace
+
 // The requests posted to the worker thread might outlive the job.  Thread-safe
 // ref counting is used to ensure that the InputStream and InputStreamReader
 // members of this class are still there when the closure is run on the worker
@@ -113,8 +128,9 @@ void AndroidStreamReaderURLRequestJob::StartAsync() {
       delegate_->OpenInputStream(env, request()));
 
   if (!stream) {
-    NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED,
-                                     net::ERR_FAILED));
+    // Clear the IO_PENDING status set in Start().
+    SetStatus(net::URLRequestStatus());
+    HeadersComplete(kHTTPNotFound, kHTTPNotFoundText);
     return;
   }
 
@@ -141,7 +157,7 @@ void AndroidStreamReaderURLRequestJob::OnReaderSeekCompleted(int result) {
   SetStatus(net::URLRequestStatus());
   if (result >= 0) {
     set_expected_content_size(result);
-    NotifyHeadersComplete();
+    HeadersComplete(kHTTPOk, kHTTPOkText);
   } else {
     NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED, result));
   }
@@ -174,7 +190,13 @@ base::TaskRunner* AndroidStreamReaderURLRequestJob::GetWorkerThreadRunner() {
 bool AndroidStreamReaderURLRequestJob::ReadRawData(net::IOBuffer* dest,
                                                    int dest_size,
                                                    int* bytes_read) {
-  DCHECK(input_stream_reader_wrapper_);
+  if (!input_stream_reader_wrapper_) {
+    // This will happen if opening the InputStream fails in which case the
+    // error is communicated by setting the HTTP response status header rather
+    // than failing the entire request.
+    *bytes_read = 0;
+    return true;
+  }
 
   PostTaskAndReplyWithResult(
       GetWorkerThreadRunner(),
@@ -222,6 +244,54 @@ bool AndroidStreamReaderURLRequestJob::GetCharset(std::string* charset) {
       env, request(), input_stream_reader_wrapper_->input_stream(), charset);
 }
 
+void AndroidStreamReaderURLRequestJob::HeadersComplete(
+    int status_code,

[mnaganov (inactive), 2013/03/06 13:33:10]

nit: please move the first arg in line with the function name

[mkosiba (inactive), 2013/03/07 18:52:01]

No can do. The chromium style guide says that you need to do either:

  // 1)
  void class::f(a,b,c)
  // 2)
  void class::f(a,
                b,
                c)
  // 3)
  void class:f(
      a,
      b,
      c)

In my case 2) is not possible since the second line would be longer than the
line limit, so have to stick with 3)

[mnaganov (inactive), 2013/03/08 09:15:47]

Ah, sorry. The second line visually looks as if it should fit, but perhaps it's
just longer by 1 char.

+    const std::string& status_text) {
+  std::string status("HTTP/1.1 ");

[benm (inactive), 2013/03/06 17:05:11]

It feels weird to me we need to do this from scratch.. no helpers int net/ or
something?

[mkosiba (inactive), 2013/03/07 18:52:01]

nope, I did spend a while searching. The Chromium net stack doesn't really care
about http headers, to it the headers are just data. Even the http job treats
them in a rather generic way.

+  status.append(base::IntToString(status_code));
+  status.append(" ");
+  status.append(status_text);
+  // HttpResponseHeaders expects its input string to be terminated by two NULs.
+  status.append("\0\0", 2);
+  net::HttpResponseHeaders* headers = new net::HttpResponseHeaders(status);
+
+  if (status_code == kHTTPOk) {
+    if (expected_content_size() != -1) {
+      std::string content_length_header(
+          net::HttpRequestHeaders::kContentLength);
+      content_length_header.append(": ");
+      content_length_header.append(
+          base::Int64ToString(expected_content_size()));

[mnaganov (inactive), 2013/03/06 13:33:10]

nit: is this wrap needed?

[mkosiba (inactive), 2013/03/07 18:52:01]

yes. If I join the lines they don't fit in the line length limit.

[mnaganov (inactive), 2013/03/08 09:15:47]

Ah, it's Rietveld fooling me--it displays a lot of whitespace on the right, so
visually it looks like as if it would fit. Sorry for giving wrong suggestions!

+      headers->AddHeader(content_length_header);
+    }
+
+    std::string mime_type;
+    if (GetMimeType(&mime_type) && !mime_type.empty()) {
+      std::string content_type_header(net::HttpRequestHeaders::kContentType);
+      content_type_header.append(": ");
+      content_type_header.append(mime_type);
+      headers->AddHeader(content_type_header);
+    }
+  }
+
+  response_info_.reset(new net::HttpResponseInfo());
+  response_info_->headers = headers;
+
+  NotifyHeadersComplete();
+}
+
+int AndroidStreamReaderURLRequestJob::GetResponseCode() const {
+  if (response_info_)
+    return response_info_->headers->response_code();
+  return URLRequestJob::GetResponseCode();
+}
+
+void AndroidStreamReaderURLRequestJob::GetResponseInfo(
+    net::HttpResponseInfo* info) {
+  if (response_info_)
+    *info = *response_info_;
+}
+
 void AndroidStreamReaderURLRequestJob::SetExtraRequestHeaders(
     const net::HttpRequestHeaders& headers) {
   std::string range_header;