Mojo: Refactor URLLoader interface.

mojo/services/network/url_loader_impl.cc

 // Copyright 2014 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.
 
 #include "mojo/services/network/url_loader_impl.h"
 
 #include "base/message_loop/message_loop.h"
 #include "mojo/common/common_type_converters.h"
 #include "mojo/services/network/network_context.h"
 #include "net/base/io_buffer.h"
@@ skipping 28 matching lines @@
   url_request->GetMimeType(&mime_type);
   response->mime_type = mime_type;
 
   std::string charset;
   url_request->GetCharset(&charset);
   response->charset = charset;
 
   return response.Pass();
 }
 
+NetworkErrorPtr MakeNetworkError(int error_code) {
+  NetworkErrorPtr error = NetworkError::New();
+  error->code = error_code;
+  error->description = net::ErrorToString(error_code);
+  return error.Pass();
+}
+
 }  // namespace
 
 // Keeps track of a pending two-phase write on a DataPipeProducerHandle.
 class URLLoaderImpl::PendingWriteToDataPipe :
     public base::RefCountedThreadSafe<PendingWriteToDataPipe> {
  public:
   explicit PendingWriteToDataPipe(ScopedDataPipeProducerHandle handle)
       : handle_(handle.Pass()),
         buffer_(NULL) {
   }
@@ skipping 37 matching lines @@
       : net::WrappedIOBuffer(pending_write->buffer()),
         pending_write_(pending_write) {
   }
  private:
   virtual ~DependentIOBuffer() {}
   scoped_refptr<PendingWriteToDataPipe> pending_write_;
 };
 
 URLLoaderImpl::URLLoaderImpl(NetworkContext* context)
     : context_(context),
+      response_body_buffer_size_(0),
       auto_follow_redirects_(true),
       weak_ptr_factory_(this) {
 }
 
 URLLoaderImpl::~URLLoaderImpl() {
 }
 
 void URLLoaderImpl::OnConnectionError() {
   delete this;
 }
 
 void URLLoaderImpl::Start(URLRequestPtr request,
-                          ScopedDataPipeProducerHandle response_body_stream) {
-  // Do not allow starting another request.
+                          const Callback<void(URLResponsePtr)>& callback) {
   if (url_request_) {
-    SendError(net::ERR_UNEXPECTED);
-    url_request_.reset();
-    response_body_stream_.reset();
+    SendError(net::ERR_UNEXPECTED, callback);
     return;
   }
 
   if (!request) {
-    SendError(net::ERR_INVALID_ARGUMENT);
+    SendError(net::ERR_INVALID_ARGUMENT, callback);
     return;
   }
 
-  response_body_stream_ = response_body_stream.Pass();
-
-  GURL url(request->url);
   url_request_.reset(
-      new net::URLRequest(url,
+      new net::URLRequest(GURL(request->url),
                           net::DEFAULT_PRIORITY,
                           this,
                           context_->url_request_context()));
   url_request_->set_method(request->method);
   if (request->headers) {
     net::HttpRequestHeaders headers;
     for (size_t i = 0; i < request->headers.size(); ++i)
       headers.AddHeaderFromString(request->headers[i].To<base::StringPiece>());
     url_request_->SetExtraRequestHeaders(headers);
   }
   if (request->bypass_cache)
     url_request_->SetLoadFlags(net::LOAD_BYPASS_CACHE);
   // TODO(darin): Handle request body.
 
+  callback_ = callback;
+  response_body_buffer_size_ = request->response_body_buffer_size;
   auto_follow_redirects_ = request->auto_follow_redirects;
 
   url_request_->Start();
 }
 
-void URLLoaderImpl::FollowRedirect() {
+void URLLoaderImpl::FollowRedirect(
+    const Callback<void(URLResponsePtr)>& callback) {
+  if (!url_request_) {
+    SendError(net::ERR_UNEXPECTED, callback);
+    return;
+  }
+
   if (auto_follow_redirects_) {
     DLOG(ERROR) << "Spurious call to FollowRedirect";
+    SendError(net::ERR_UNEXPECTED, callback);
+    return;
+  }
+
+  // TODO(darin): Verify that it makes sense to call FollowDeferredRedirect.
+  url_request_->FollowDeferredRedirect();
+}
+
+void URLLoaderImpl::QueryStatus(
+    const Callback<void(URLLoaderStatusPtr)>& callback) {
+  URLLoaderStatusPtr status(URLLoaderStatus::New());
+  if (url_request_) {
+    status->is_loading = url_request_->is_pending();
+    if (!url_request_->status().is_success())
+      status->error = MakeNetworkError(url_request_->status().error());
   } else {
-    if (url_request_)
-      url_request_->FollowDeferredRedirect();
+    status->is_loading = false;
   }
+  // TODO(darin): Populate more status fields.
+  callback.Run(status.Pass());
 }
 
 void URLLoaderImpl::OnReceivedRedirect(net::URLRequest* url_request,
                                        const GURL& new_url,
                                        bool* defer_redirect) {
   DCHECK(url_request == url_request_.get());
   DCHECK(url_request->status().is_success());
 
+  if ((*defer_redirect = !auto_follow_redirects_))

[Aaron Boodman, 2014/07/10 20:05:00]

Do you need the extra parens?

+    return;
+
   URLResponsePtr response = MakeURLResponse(url_request);
-  std::string redirect_method =
+  response->redirect_method =
       net::URLRequest::ComputeMethodForRedirect(url_request->method(),
                                                 response->status_code);
-  client()->OnReceivedRedirect(
-      response.Pass(), new_url.spec(), redirect_method);
+  response->redirect_url = new_url.spec();
 
-  *defer_redirect = !auto_follow_redirects_;
+  SendResponse(response.Pass());
 }
 
 void URLLoaderImpl::OnResponseStarted(net::URLRequest* url_request) {
   DCHECK(url_request == url_request_.get());
 
   if (!url_request->status().is_success()) {
-    SendError(url_request->status().error());
+    SendError(url_request->status().error(), callback_);
+    callback_ = Callback<void(URLResponsePtr)>();

[Aaron Boodman, 2014/07/10 20:05:00]

It looks like if an empty mojo::Callback gets Run() its a silent no-op. Perhaps
it should be a DCHECK instead. Or you should add a DHCECK that it's not empty at
the place that callback_ is Run() here.

     return;
   }
 
   // TODO(darin): Add support for optional MIME sniffing.
 
-  client()->OnReceivedResponse(MakeURLResponse(url_request));
+  DataPipe data_pipe;
+  // TODO(darin): Honor given buffer size.
+
+  URLResponsePtr response = MakeURLResponse(url_request);
+  response->body = data_pipe.consumer_handle.Pass();
+  response_body_stream_ = data_pipe.producer_handle.Pass();
+
+  SendResponse(response.Pass());
 
   // Start reading...
   ReadMore();
 }
 
 void URLLoaderImpl::OnReadCompleted(net::URLRequest* url_request,
                                     int bytes_read) {
+  DCHECK(url_request == url_request_.get());
+
   if (url_request->status().is_success()) {
     DidRead(static_cast<uint32_t>(bytes_read), false);
   } else {
     pending_write_ = NULL;  // This closes the data pipe.
-    // TODO(darin): Perhaps we should communicate this error to our client.
   }
 }
 
-void URLLoaderImpl::SendError(int error_code) {
-  NetworkErrorPtr error(NetworkError::New());
-  error->code = error_code;
-  error->description = net::ErrorToString(error_code);
-  client()->OnReceivedError(error.Pass());
+void URLLoaderImpl::SendError(
+    int error_code,
+    const Callback<void(URLResponsePtr)>& callback) {
+  URLResponsePtr response(URLResponse::New());
+  if (url_request_)
+    response->url = url_request_->url().spec();
+  response->error = MakeNetworkError(error_code);
+  callback.Run(response.Pass());
+}
+
+void URLLoaderImpl::SendResponse(URLResponsePtr response) {
+  Callback<void(URLResponsePtr)> callback;
+  std::swap(callback_, callback);
+  callback.Run(response.Pass());
 }
 
 void URLLoaderImpl::OnResponseBodyStreamReady(MojoResult result) {
   // TODO(darin): Handle a bad |result| value.
   ReadMore();
 }
 
 void URLLoaderImpl::WaitToReadMore() {
   handle_watcher_.Start(response_body_stream_.get(),
                         MOJO_HANDLE_SIGNAL_WRITABLE,
@@ skipping 31 matching lines @@
   // Drop our reference to the buffer.
   buf = NULL;
 
   if (url_request_->status().is_io_pending()) {
     // Wait for OnReadCompleted.
   } else if (url_request_->status().is_success() && bytes_read > 0) {
     DidRead(static_cast<uint32_t>(bytes_read), true);
   } else {
     pending_write_->Complete(0);
     pending_write_ = NULL;  // This closes the data pipe.
-    if (bytes_read == 0) {
-      client()->OnReceivedEndOfResponseBody();
-    } else {
-      DCHECK(!url_request_->status().is_success());
-      SendError(url_request_->status().error());
-    }
+    DCHECK(bytes_read == 0 || !url_request_->status().is_success());

[Aaron Boodman, 2014/07/10 20:05:00]

Nit: Recommend separate DCHECKs so that you can see which one failed more
easily.

   }
 }
 
 void URLLoaderImpl::DidRead(uint32_t num_bytes, bool completed_synchronously) {
   DCHECK(url_request_->status().is_success());
 
   response_body_stream_ = pending_write_->Complete(num_bytes);
   pending_write_ = NULL;
 
   if (completed_synchronously) {
     base::MessageLoop::current()->PostTask(
         FROM_HERE,
         base::Bind(&URLLoaderImpl::ReadMore, weak_ptr_factory_.GetWeakPtr()));
   } else {
     ReadMore();
   }
 }
 
 }  // namespace mojo