Make URLRequestFetchJob actually work

atom/browser/api/atom_api_protocol.cc

@@ -116,8 +116,7 @@ class CustomProtocolRequestJob : public AdapterRequestJob {
                        GetWeakPtr(), path));
         return;
       } else if (name == "RequestErrorJob") {
-        // Default value net::ERR_NOT_IMPLEMENTED
-        int error = -11;
+        int error = net::ERR_NOT_IMPLEMENTED;
         dict.Get("error", &error);
 
         BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,

atom/browser/api/lib/protocol.coffee

@@ -36,6 +36,6 @@ class RequestErrorJob
 
 protocol.RequestHttpJob =
 class RequestHttpJob
-  constructor: (@url) ->
+  constructor: ({@url}) ->
 
 module.exports = protocol

atom/browser/net/adapter_request_job.cc

@@ -68,6 +68,14 @@ bool AdapterRequestJob::GetCharset(std::string* charset) {
   return real_job_->GetCharset(charset);
 }
 
+void AdapterRequestJob::GetResponseInfo(net::HttpResponseInfo* info) {
+  real_job_->GetResponseInfo(info);
+}
+
+int AdapterRequestJob::GetResponseCode() const {
+  return real_job_->GetResponseCode();
+}
+
 base::WeakPtr<AdapterRequestJob> AdapterRequestJob::GetWeakPtr() {
   return weak_factory_.GetWeakPtr();
 }
@@ -107,6 +115,11 @@ void AdapterRequestJob::CreateFileJobAndStart(const base::FilePath& path) {
 }
 
 void AdapterRequestJob::CreateHttpJobAndStart(const GURL& url) {
+  if (!url.is_valid()) {
+    CreateErrorJobAndStart(net::ERR_INVALID_URL);
+    return;
+  }
+
   real_job_ = new URLRequestFetchJob(request(), network_delegate(), url);
   real_job_->Start();
 }

atom/browser/net/adapter_request_job.h

@@ -41,6 +41,8 @@ class AdapterRequestJob : public net::URLRequestJob {
   net::Filter* SetupFilter() const override;
   bool GetMimeType(std::string* mime_type) const override;
   bool GetCharset(std::string* charset) override;
+  void GetResponseInfo(net::HttpResponseInfo* info) override;
+  int GetResponseCode() const override;
 
   base::WeakPtr<AdapterRequestJob> GetWeakPtr();
 

atom/browser/net/url_request_fetch_job.cc

@@ -4,85 +4,141 @@
 
 #include "atom/browser/net/url_request_fetch_job.h"
 
-#include <string>
+#include <algorithm>
+#include <vector>
 
 #include "atom/browser/atom_browser_context.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
+#include "net/http/http_response_headers.h"
+#include "net/url_request/url_fetcher.h"
+#include "net/url_request/url_fetcher_response_writer.h"
 #include "net/url_request/url_request_status.h"
 
 namespace atom {
 
+namespace {
+
+// Pipe the response writer back to URLRequestFetchJob.
+class ResponsePiper : public net::URLFetcherResponseWriter {
+ public:
+  explicit ResponsePiper(URLRequestFetchJob* job)
+      : first_write_(true), job_(job) {}
+
+  // net::URLFetcherResponseWriter:
+  int Initialize(const net::CompletionCallback& callback) override {
+    return net::OK;
+  }
+  int Write(net::IOBuffer* buffer,
+            int num_bytes,
+            const net::CompletionCallback& callback) override {
+    job_->DataAvailable(buffer, num_bytes);
+    if (first_write_) {
+      // The URLFetcherResponseWriter doesn't have an event when headers have
+      // been read, so we have to emulate by hooking to first write event.
+      job_->HeadersCompleted();
+      first_write_ = false;
+    }
+    return num_bytes;
+  }
+  int Finish(const net::CompletionCallback& callback) override {
+    return net::OK;
+  }
+
+ private:
+  bool first_write_;
+  URLRequestFetchJob* job_;
+
+  DISALLOW_COPY_AND_ASSIGN(ResponsePiper);
+};
+
+}  // namespace
+
 URLRequestFetchJob::URLRequestFetchJob(
     net::URLRequest* request,
     net::NetworkDelegate* network_delegate,
     const GURL& url)
     : net::URLRequestJob(request, network_delegate),
       url_(url),
-      weak_ptr_factory_(this) {}
+      finished_(false),
+      weak_factory_(this) {}
 
-URLRequestFetchJob::~URLRequestFetchJob() {}
+void URLRequestFetchJob::HeadersCompleted() {
+  response_info_.reset(new net::HttpResponseInfo);
+  response_info_->headers = fetcher_->GetResponseHeaders();
+  NotifyHeadersComplete();
+}
+
+void URLRequestFetchJob::DataAvailable(net::IOBuffer* buffer, int num_bytes) {
+  buffer_.resize(buffer_.size() + num_bytes);
+  memcpy(buffer_.data() + buffer_.size() - num_bytes,
+         buffer->data(),
+         num_bytes);
+  SetStatus(net::URLRequestStatus());
+}
 
 void URLRequestFetchJob::Start() {
   fetcher_.reset(net::URLFetcher::Create(url_, net::URLFetcher::GET, this));
   auto context = AtomBrowserContext::Get()->url_request_context_getter();
   fetcher_->SetRequestContext(context);
-  fetcher_->SaveResponseWithWriter(scoped_ptr<net::URLFetcherResponseWriter>(
-      this));
+  fetcher_->SaveResponseWithWriter(make_scoped_ptr(new ResponsePiper(this)));
   fetcher_->Start();
 }
 
 void URLRequestFetchJob::Kill() {
-  weak_ptr_factory_.InvalidateWeakPtrs();
   URLRequestJob::Kill();
+  fetcher_.reset();
 }
 
 bool URLRequestFetchJob::ReadRawData(net::IOBuffer* dest,
                                      int dest_size,
                                      int* bytes_read) {
-  if (!dest_size) {
+  if (finished_) {
     *bytes_read = 0;
     return true;
   }
 
-  int to_read = dest_size < buffer_->BytesRemaining() ?
-      dest_size : buffer_->BytesRemaining();
-  memcpy(dest->data(), buffer_->data(), to_read);
-  buffer_->DidConsume(to_read);
-  if (!buffer_->BytesRemaining()) {
-    NotifyReadComplete(buffer_->size());
-    return true;
+  if (buffer_.size() == 0) {
+    SetStatus(net::URLRequestStatus(net::URLRequestStatus::IO_PENDING, 0));
+    return false;
   }
 
-  *bytes_read = to_read;
-  return false;
+  if (static_cast<size_t>(dest_size) >= buffer_.size()) {
+    // Copy all data at once (quick).
+    memcpy(dest->data(), buffer_.data(), buffer_.size());
+    *bytes_read = buffer_.size();
+    buffer_.clear();
+  } else {
+    // Can not fit all data, strip them (slow).
+    memcpy(dest->data(), buffer_.data(), dest_size);
+    *bytes_read = dest_size;
+    std::rotate(buffer_.begin(), buffer_.begin() + dest_size, buffer_.end());
+    buffer_.resize(buffer_.size() - dest_size);
+  }
+  return true;
+}
+
+bool URLRequestFetchJob::GetMimeType(std::string* mime_type) const {
+  if (!response_info_)
+    return false;
+
+  return response_info_->headers->GetMimeType(mime_type);
+}
+
+void URLRequestFetchJob::GetResponseInfo(net::HttpResponseInfo* info) {
+  if (response_info_)
+    *info = *response_info_;
+}
+
+int URLRequestFetchJob::GetResponseCode() const {
+  if (!response_info_)
+    return -1;
+
+  return response_info_->headers->response_code();
 }
 
 void URLRequestFetchJob::OnURLFetchComplete(const net::URLFetcher* source) {
-  if (!source->GetStatus().is_success())
-    NotifyDone(net::URLRequestStatus(net::URLRequestStatus::FAILED,
-                                     source->GetResponseCode()));
-
-  NotifyHeadersComplete();
-}
-
-
-int URLRequestFetchJob::Initialize(const net::CompletionCallback& callback) {
-  if (buffer_)
-    buffer_->Release();
-  return net::OK;
-}
-
-int URLRequestFetchJob::Write(net::IOBuffer* buffer,
-                              int num_bytes,
-                              const net::CompletionCallback& calback) {
-  buffer_ = new net::DrainableIOBuffer(buffer, num_bytes);
-  set_expected_content_size(num_bytes);
-  return num_bytes;
-}
-
-int URLRequestFetchJob::Finish(const net::CompletionCallback& callback) {
-  return net::OK;
+  finished_ = true;
 }
 
 }  // namespace atom

atom/browser/net/url_request_fetch_job.h

@@ -5,50 +5,49 @@
 #ifndef ATOM_BROWSER_NET_URL_REQUEST_FETCH_JOB_H_
 #define ATOM_BROWSER_NET_URL_REQUEST_FETCH_JOB_H_
 
-#include "base/memory/ref_counted.h"
+#include <vector>
+
 #include "base/memory/weak_ptr.h"
-#include "net/base/io_buffer.h"
-#include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_fetcher_delegate.h"
-#include "net/url_request/url_fetcher_response_writer.h"
 #include "net/url_request/url_request_job.h"
 
 namespace atom {
 
 class URLRequestFetchJob : public net::URLRequestJob,
-                           public net::URLFetcherDelegate,
-                           public net::URLFetcherResponseWriter {
+                           public net::URLFetcherDelegate {
  public:
   URLRequestFetchJob(net::URLRequest* request,
                      net::NetworkDelegate* network_delegate,
                      const GURL& url);
 
+  base::WeakPtr<URLRequestFetchJob> GetWeakPtr() {
+    return weak_factory_.GetWeakPtr();
+  };
+
+  void HeadersCompleted();
+  void DataAvailable(net::IOBuffer* buffer, int num_bytes);
+
   // net::URLRequestJob:
   void Start() override;
   void Kill() override;
   bool ReadRawData(net::IOBuffer* buf,
                    int buf_size,
                    int* bytes_read) override;
+  bool GetMimeType(std::string* mime_type) const override;
+  void GetResponseInfo(net::HttpResponseInfo* info) override;
+  int GetResponseCode() const override;
 
   // net::URLFetcherDelegate:
   void OnURLFetchComplete(const net::URLFetcher* source) override;
 
-  // net::URLFetchResponseWriter:
-  int Initialize(const net::CompletionCallback& callback) override;
-  int Write(net::IOBuffer* buffer,
-            int num_bytes,
-            const net::CompletionCallback& callback) override;
-  int Finish(const net::CompletionCallback& callback) override;
-
- protected:
-  virtual ~URLRequestFetchJob();
-
  private:
   GURL url_;
   scoped_ptr<net::URLFetcher> fetcher_;
-  scoped_refptr<net::DrainableIOBuffer> buffer_;
+  std::vector<char> buffer_;
+  scoped_ptr<net::HttpResponseInfo> response_info_;
+  bool finished_;
 
-  base::WeakPtrFactory<URLRequestFetchJob> weak_ptr_factory_;
+  base::WeakPtrFactory<URLRequestFetchJob> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(URLRequestFetchJob);
 };