Mojo: Refactor URLLoader interface.

This change simplifies the URLLoader interface, eliminating the URLLoaderClient interface. The URLResponse now includes the DataPipeConsumerHandle for the response body stream as well as information about a possible redirect response.

One nice thing about this change is that you can now pass around the URLResponsePtr without also having to pass around the DataPipeConsumerHandle for the response body stream.

This didn't enable me to eliminate ResponseDetails as I think that structure should include not only the URLResponse but also the URLLoader used to generate the response. (That enables the LaunchInstance to get destroyed immediately after delegating the response to the handler app as it no longer needs to stick around to keep the URLLoader alive.) The recipient of the URLLoader might be interested in calling the new QueryStatus method to find out more information about the URL load (e.g., Blink wants to know the encoded size of a response body, which may not be available at the time when response headers are received).

Review URL: https://codereview.chromium.org/373373002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@282535 0039d316-1c4b-4281-b951-d872f2087c98

mojo/examples/html_viewer/html_document_view.cc

@@ -104,15 +104,13 @@ void HTMLDocumentView::AttachToNode(view_manager::Node* node) {
   web_view_->resize(gfx::Size(node->bounds().size()));
 }
 
-void HTMLDocumentView::Load(URLResponsePtr response,
-                            ScopedDataPipeConsumerHandle response_body_stream) {
+void HTMLDocumentView::Load(URLResponsePtr response) {
   DCHECK(web_view_);
 
   GURL url(response->url);
 
   WebURLRequestExtraData* extra_data = new WebURLRequestExtraData;
   extra_data->synthetic_response = response.Pass();
-  extra_data->synthetic_response_body_stream = response_body_stream.Pass();
 
   blink::WebURLRequest web_request;
   web_request.initialize();

mojo/examples/html_viewer/html_document_view.h

@@ -36,8 +36,7 @@ class HTMLDocumentView : public blink::WebViewClient,
 
   void AttachToNode(view_manager::Node* node);
 
-  void Load(URLResponsePtr response,
-            ScopedDataPipeConsumerHandle response_body_stream);
+  void Load(URLResponsePtr response);
 
  private:
   // WebViewClient methods:

mojo/examples/html_viewer/html_viewer.cc

@@ -47,11 +47,9 @@ class HTMLViewer : public ApplicationDelegate,
     blink::shutdown();
   }
 
-  void Load(URLResponsePtr response,
-            ScopedDataPipeConsumerHandle response_body_stream) {
+  void Load(URLResponsePtr response) {
     // Need to wait for OnRootAdded.
     response_ = response.Pass();
-    response_body_stream_ = response_body_stream.Pass();
     MaybeLoad();
   }
 
@@ -82,7 +80,7 @@ class HTMLViewer : public ApplicationDelegate,
 
   void MaybeLoad() {
     if (document_view_ && response_.get())
-      document_view_->Load(response_.Pass(), response_body_stream_.Pass());
+      document_view_->Load(response_.Pass());
   }
 
   scoped_ptr<BlinkPlatformImpl> blink_platform_impl_;
@@ -91,7 +89,6 @@ class HTMLViewer : public ApplicationDelegate,
   // TODO(darin): Figure out proper ownership of this instance.
   HTMLDocumentView* document_view_;
   URLResponsePtr response_;
-  ScopedDataPipeConsumerHandle response_body_stream_;
 
   DISALLOW_COPY_AND_ASSIGN(HTMLViewer);
 };
@@ -102,8 +99,7 @@ void NavigatorImpl::Navigate(
     navigation::ResponseDetailsPtr response_details) {
   printf("In HTMLViewer, rendering url: %s\n",
       response_details->response->url.data());
-  viewer_->Load(response_details->response.Pass(),
-                response_details->response_body_stream.Pass());
+  viewer_->Load(response_details->response.Pass());
 }
 
 }

mojo/examples/html_viewer/weburlloader_impl.cc

@@ -47,7 +47,6 @@ WebURLLoaderImpl::WebURLLoaderImpl(NetworkService* network_service)
     : client_(NULL),
       weak_factory_(this) {
   network_service->CreateURLLoader(Get(&url_loader_));
-  url_loader_.set_client(this);
 }
 
 WebURLLoaderImpl::~WebURLLoaderImpl() {
@@ -64,25 +63,25 @@ void WebURLLoaderImpl::loadSynchronously(
 void WebURLLoaderImpl::loadAsynchronously(const blink::WebURLRequest& request,
                                           blink::WebURLLoaderClient* client) {
   client_ = client;
+  url_ = request.url();
 
   URLRequestPtr url_request(URLRequest::New());
-  url_request->url = request.url().spec();
+  url_request->url = url_.spec();
   url_request->auto_follow_redirects = false;
   // TODO(darin): Copy other fields.
 
   if (request.extraData()) {
     WebURLRequestExtraData* extra_data =
         static_cast<WebURLRequestExtraData*>(request.extraData());
-    response_body_stream_ = extra_data->synthetic_response_body_stream.Pass();
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE,
         base::Bind(&WebURLLoaderImpl::OnReceivedResponse,
                    weak_factory_.GetWeakPtr(),
                    base::Passed(&extra_data->synthetic_response)));
   } else {
-    DataPipe pipe;
-    url_loader_->Start(url_request.Pass(), pipe.producer_handle.Pass());
-    response_body_stream_ = pipe.consumer_handle.Pass();
+    url_loader_->Start(url_request.Pass(),
+                       base::Bind(&WebURLLoaderImpl::OnReceivedResponse,
+                                  weak_factory_.GetWeakPtr()));
   }
 }
 
@@ -90,53 +89,63 @@ void WebURLLoaderImpl::cancel() {
   url_loader_.reset();
   response_body_stream_.reset();
 
-  NetworkErrorPtr network_error(NetworkError::New());
-  network_error->code = net::ERR_ABORTED;
+  URLResponsePtr failed_response(URLResponse::New());
+  failed_response->url = url_.spec();
+  failed_response->error = NetworkError::New();
+  failed_response->error->code = net::ERR_ABORTED;
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
-      base::Bind(&WebURLLoaderImpl::OnReceivedError,
+      base::Bind(&WebURLLoaderImpl::OnReceivedResponse,
                  weak_factory_.GetWeakPtr(),
-                 base::Passed(&network_error)));
+                 base::Passed(&failed_response)));
 }
 
 void WebURLLoaderImpl::setDefersLoading(bool defers_loading) {
   NOTIMPLEMENTED();
 }
 
-void WebURLLoaderImpl::OnReceivedRedirect(URLResponsePtr url_response,
-                                          const String& new_url,
-                                          const String& new_method) {
-  blink::WebURLRequest new_request;
-  new_request.initialize();
-  new_request.setURL(GURL(new_url));
-
-  client_->willSendRequest(this, new_request, ToWebURLResponse(url_response));
-  // TODO(darin): Check if new_request was rejected.
-
-  url_loader_->FollowRedirect();
-}
-
 void WebURLLoaderImpl::OnReceivedResponse(URLResponsePtr url_response) {
-  client_->didReceiveResponse(this, ToWebURLResponse(url_response));
+  url_ = GURL(url_response->url);
 
-  // Start streaming data
-  ReadMore();
+  if (url_response->error) {
+    OnReceivedError(url_response.Pass());
+  } else if (url_response->redirect_url) {
+    OnReceivedRedirect(url_response.Pass());
+  } else {
+    client_->didReceiveResponse(this, ToWebURLResponse(url_response));
+
+    // Start streaming data
+    response_body_stream_ = url_response->body.Pass();
+    ReadMore();
+  }
 }
 
-void WebURLLoaderImpl::OnReceivedError(NetworkErrorPtr error) {
+void WebURLLoaderImpl::OnReceivedError(URLResponsePtr url_response) {
   blink::WebURLError web_error;
   web_error.domain = blink::WebString::fromUTF8(net::kErrorDomain);
-  web_error.reason = error->code;
-  web_error.unreachableURL = GURL();  // TODO(darin): Record this.
+  web_error.reason = url_response->error->code;
+  web_error.unreachableURL = GURL(url_response->url);
   web_error.staleCopyInCache = false;
-  web_error.isCancellation = error->code == net::ERR_ABORTED ? true : false;
+  web_error.isCancellation =
+      url_response->error->code == net::ERR_ABORTED ? true : false;
 
   client_->didFail(this, web_error);
 }
 
-void WebURLLoaderImpl::OnReceivedEndOfResponseBody() {
-  // This is the signal that the response body was not truncated.
+void WebURLLoaderImpl::OnReceivedRedirect(URLResponsePtr url_response) {
+  blink::WebURLRequest new_request;
+  new_request.initialize();
+  new_request.setURL(GURL(url_response->redirect_url));
+  new_request.setHTTPMethod(
+      blink::WebString::fromUTF8(url_response->redirect_method));
+
+  client_->willSendRequest(this, new_request, ToWebURLResponse(url_response));
+  // TODO(darin): Check if new_request was rejected.
+
+  url_loader_->FollowRedirect(
+      base::Bind(&WebURLLoaderImpl::OnReceivedResponse,
+                 weak_factory_.GetWeakPtr()));
 }
 
 void WebURLLoaderImpl::ReadMore() {

mojo/examples/html_viewer/weburlloader_impl.h

@@ -23,11 +23,9 @@ class WebURLRequestExtraData : public blink::WebURLRequest::ExtraData {
   virtual ~WebURLRequestExtraData();
 
   URLResponsePtr synthetic_response;
-  ScopedDataPipeConsumerHandle synthetic_response_body_stream;
 };
 
-class WebURLLoaderImpl : public blink::WebURLLoader,
-                         public URLLoaderClient {
+class WebURLLoaderImpl : public blink::WebURLLoader {
  public:
   explicit WebURLLoaderImpl(NetworkService* network_service);
 
@@ -43,19 +41,15 @@ class WebURLLoaderImpl : public blink::WebURLLoader,
   virtual void cancel() OVERRIDE;
   virtual void setDefersLoading(bool defers_loading) OVERRIDE;
 
-  // URLLoaderClient methods:
-  virtual void OnReceivedRedirect(URLResponsePtr response,
-                                  const String& new_url,
-                                  const String& new_method) OVERRIDE;
-  virtual void OnReceivedResponse(URLResponsePtr response) OVERRIDE;
-  virtual void OnReceivedError(NetworkErrorPtr error) OVERRIDE;
-  virtual void OnReceivedEndOfResponseBody() OVERRIDE;
-
+  void OnReceivedResponse(URLResponsePtr response);
+  void OnReceivedError(URLResponsePtr response);
+  void OnReceivedRedirect(URLResponsePtr response);
   void ReadMore();
   void WaitToReadMore();
   void OnResponseBodyStreamReady(MojoResult result);
 
   blink::WebURLLoaderClient* client_;
+  GURL url_;
   URLLoaderPtr url_loader_;
   ScopedDataPipeConsumerHandle response_body_stream_;
   common::HandleWatcher handle_watcher_;

mojo/examples/png_viewer/png_viewer.cc

@@ -63,12 +63,12 @@ class NavigatorImpl : public InterfaceImpl<navigation::Navigator> {
     uint32_t num_bytes = bytes_remaining;
     while (bytes_remaining > 0) {
       MojoResult result = ReadDataRaw(
-          response_details->response_body_stream.get(),
+          response_details->response->body.get(),
           buf,
           &num_bytes,
           MOJO_READ_DATA_FLAG_NONE);
       if (result == MOJO_RESULT_SHOULD_WAIT) {
-        Wait(response_details->response_body_stream.get(),
+        Wait(response_details->response->body.get(),
              MOJO_HANDLE_SIGNAL_READABLE,
              MOJO_DEADLINE_INDEFINITE);
       } else if (result == MOJO_RESULT_OK) {

mojo/examples/wget/wget.cc

@@ -6,68 +6,29 @@
 
 #include "mojo/public/cpp/application/application_delegate.h"
 #include "mojo/public/cpp/application/application_impl.h"
+#include "mojo/public/cpp/utility/run_loop.h"
 #include "mojo/services/public/interfaces/network/network_service.mojom.h"
 #include "mojo/services/public/interfaces/network/url_loader.mojom.h"
 
 namespace mojo {
 namespace examples {
+namespace {
 
-class WGetApp : public ApplicationDelegate, public URLLoaderClient {
+class ResponsePrinter {
  public:
-  virtual void Initialize(ApplicationImpl* app) MOJO_OVERRIDE {
-    app->ConnectToService("mojo:mojo_network_service", &network_service_);
-    Start();
-  }
-
- private:
-  virtual void OnReceivedRedirect(URLResponsePtr response,
-                                  const String& new_url,
-                                  const String& new_method) MOJO_OVERRIDE {
-    PrintResponse(response);
-  }
-
-  virtual void OnReceivedResponse(URLResponsePtr response) MOJO_OVERRIDE {
-    PrintResponse(response);
-    PrintResponseBody();
-    Start();
-  }
-
-  virtual void OnReceivedError(NetworkErrorPtr error) MOJO_OVERRIDE {
-    printf("Got error: %d (%s)\n",
-        error->code, error->description.get().c_str());
-  }
-
-  virtual void OnReceivedEndOfResponseBody() MOJO_OVERRIDE {
-    // Ignored.
-  }
-
-  void Start() {
-    std::string url = PromptForURL();
-    printf("Loading: %s\n", url.c_str());
-
-    network_service_->CreateURLLoader(Get(&url_loader_));
-    url_loader_.set_client(this);
-
-    URLRequestPtr request(URLRequest::New());
-    request->url = url;
-    request->method = "GET";
-    request->auto_follow_redirects = true;
-
-    DataPipe data_pipe;
-    response_body_stream_ = data_pipe.consumer_handle.Pass();
+  void Run(URLResponsePtr response) const {
+    if (response->error) {
+      printf("Got error: %d (%s)\n",
+          response->error->code, response->error->description.get().c_str());
+    } else {
+      PrintResponse(response);
+      PrintResponseBody(response->body.Pass());
+    }
 
-    url_loader_->Start(request.Pass(), data_pipe.producer_handle.Pass());
+    RunLoop::current()->Quit();  // All done!
   }
 
-  std::string PromptForURL() {
-    printf("Enter URL> ");
-    char buf[1024];
-    if (scanf("%1023s", buf) != 1)
-      buf[0] = '\0';
-    return buf;
-  }
-
-  void PrintResponse(const URLResponsePtr& response) {
+  void PrintResponse(const URLResponsePtr& response) const {
     printf(">>> Headers <<< \n");
     printf("  %s\n", response->status_line.get().c_str());
     if (response->headers) {
@@ -76,20 +37,17 @@ class WGetApp : public ApplicationDelegate, public URLLoaderClient {
     }
   }
 
-  void PrintResponseBody() {
+  void PrintResponseBody(ScopedDataPipeConsumerHandle body) const {
     // Read response body in blocking fashion.
     printf(">>> Body <<<\n");
 
     for (;;) {
       char buf[512];
       uint32_t num_bytes = sizeof(buf);
-      MojoResult result = ReadDataRaw(
-          response_body_stream_.get(),
-          buf,
-          &num_bytes,
-          MOJO_READ_DATA_FLAG_NONE);
+      MojoResult result = ReadDataRaw(body.get(), buf, &num_bytes,
+                                      MOJO_READ_DATA_FLAG_NONE);
       if (result == MOJO_RESULT_SHOULD_WAIT) {
-        Wait(response_body_stream_.get(),
+        Wait(body.get(),
              MOJO_HANDLE_SIGNAL_READABLE,
              MOJO_DEADLINE_INDEFINITE);
       } else if (result == MOJO_RESULT_OK) {
@@ -104,10 +62,42 @@ class WGetApp : public ApplicationDelegate, public URLLoaderClient {
 
     printf("\n>>> EOF <<<\n");
   }
+};
+
+}  // namespace
+
+class WGetApp : public ApplicationDelegate {
+ public:
+  virtual void Initialize(ApplicationImpl* app) MOJO_OVERRIDE {
+    app->ConnectToService("mojo:mojo_network_service", &network_service_);
+    Start();
+  }
+
+ private:
+  void Start() {
+    std::string url = PromptForURL();
+    printf("Loading: %s\n", url.c_str());
+
+    network_service_->CreateURLLoader(Get(&url_loader_));
+
+    URLRequestPtr request(URLRequest::New());
+    request->url = url;
+    request->method = "GET";
+    request->auto_follow_redirects = true;
+
+    url_loader_->Start(request.Pass(), ResponsePrinter());
+  }
+
+  std::string PromptForURL() {
+    printf("Enter URL> ");
+    char buf[1024];
+    if (scanf("%1023s", buf) != 1)
+      buf[0] = '\0';
+    return buf;
+  }
 
   NetworkServicePtr network_service_;
   URLLoaderPtr url_loader_;
-  ScopedDataPipeConsumerHandle response_body_stream_;
 };
 
 }  // namespace examples

mojo/services/launcher/launcher.cc

@@ -2,9 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/string_tokenizer.h"
+#include "base/strings/string_util.h"
 #include "mojo/public/cpp/application/application_connection.h"
 #include "mojo/public/cpp/application/application_delegate.h"
 #include "mojo/public/cpp/application/application_impl.h"
@@ -42,7 +44,7 @@ class LauncherConnection : public InterfaceImpl<Launcher> {
   DISALLOW_COPY_AND_ASSIGN(LauncherConnection);
 };
 
-class LaunchInstance : public URLLoaderClient {
+class LaunchInstance {
  public:
   LaunchInstance(LauncherApp* app,
                  const LaunchCallback& callback,
@@ -50,24 +52,14 @@ class LaunchInstance : public URLLoaderClient {
   virtual ~LaunchInstance() {}
 
  private:
-  // Overridden from URLLoaderClient:
-  virtual void OnReceivedRedirect(URLResponsePtr response,
-                                  const String& new_url,
-                                  const String& new_method) OVERRIDE {
-  }
-  virtual void OnReceivedResponse(URLResponsePtr response) OVERRIDE;
-  virtual void OnReceivedError(NetworkErrorPtr error) OVERRIDE {
-    ScheduleDestroy();
-  }
-  virtual void OnReceivedEndOfResponseBody() OVERRIDE {
-    ScheduleDestroy();
-  }
+  void OnReceivedResponse(URLResponsePtr response);
 
   std::string GetContentType(const Array<String>& headers) {
     for (size_t i = 0; i < headers.size(); ++i) {
       base::StringTokenizer t(headers[i], ": ;=");
       while (t.GetNext()) {
-        if (!t.token_is_delim() && t.token() == "Content-Type") {
+        if (!t.token_is_delim() &&
+            LowerCaseEqualsASCII(t.token(), "content-type")) {
           while (t.GetNext()) {
             if (!t.token_is_delim())
               return t.token();
@@ -154,31 +146,33 @@ LaunchInstance::LaunchInstance(LauncherApp* app,
     : app_(app),
       destroy_scheduled_(false),
       callback_(callback) {
-  url_loader_ = app_->CreateURLLoader();
-  url_loader_.set_client(this);
-
   URLRequestPtr request(URLRequest::New());
   request->url = url;
   request->method = "GET";
   request->auto_follow_redirects = true;
 
-  DataPipe data_pipe;
-  response_body_stream_ = data_pipe.consumer_handle.Pass();
-
-  url_loader_->Start(request.Pass(), data_pipe.producer_handle.Pass());
+  url_loader_ = app_->CreateURLLoader();
+  url_loader_->Start(request.Pass(),
+                     base::Bind(&LaunchInstance::OnReceivedResponse,
+                                base::Unretained(this)));
 }
 
 void LaunchInstance::OnReceivedResponse(URLResponsePtr response) {
-  std::string content_type = GetContentType(response->headers);
-  std::string handler_url = app_->GetHandlerForContentType(content_type);
-  if (!handler_url.empty()) {
-    navigation::ResponseDetailsPtr nav_response(
-        navigation::ResponseDetails::New());
-    nav_response->response = response.Pass();
-    nav_response->response_body_stream = response_body_stream_.Pass();
-    String response_url = nav_response->response->url;
-    callback_.Run(handler_url, response_url, nav_response.Pass());
+  if (!response->error) {
+    std::string content_type = GetContentType(response->headers);
+    std::string handler_url = app_->GetHandlerForContentType(content_type);
+    if (handler_url.empty()) {
+      DLOG(WARNING) << "No handler for content type: " << content_type;
+    } else {
+      navigation::ResponseDetailsPtr nav_response(
+          navigation::ResponseDetails::New());
+      nav_response->loader_handle = url_loader_.PassMessagePipe();
+      nav_response->response = response.Pass();
+      String response_url = nav_response->response->url;
+      callback_.Run(handler_url, response_url, nav_response.Pass());
+    }
   }
+  ScheduleDestroy();
 }
 
 }  // namespace launcher

mojo/services/network/url_loader_impl.cc

@@ -46,6 +46,13 @@ URLResponsePtr MakeURLResponse(const net::URLRequest* url_request) {
   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.
@@ -103,6 +110,7 @@ class URLLoaderImpl::DependentIOBuffer : public net::WrappedIOBuffer {
 
 URLLoaderImpl::URLLoaderImpl(NetworkContext* context)
     : context_(context),
+      response_body_buffer_size_(0),
       auto_follow_redirects_(true),
       weak_ptr_factory_(this) {
 }
@@ -115,25 +123,19 @@ void URLLoaderImpl::OnConnectionError() {
 }
 
 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()));
@@ -148,18 +150,42 @@ void URLLoaderImpl::Start(URLRequestPtr request,
     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,
@@ -168,27 +194,41 @@ void URLLoaderImpl::OnReceivedRedirect(net::URLRequest* url_request,
   DCHECK(url_request == url_request_.get());
   DCHECK(url_request->status().is_success());
 
+  if (auto_follow_redirects_)
+    return;
+
+  // Send the redirect response to the client, allowing them to inspect it and
+  // optionally follow the redirect.
+  *defer_redirect = true;
+
   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)>();
     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();
@@ -196,19 +236,29 @@ void URLLoaderImpl::OnResponseStarted(net::URLRequest* url_request) {
 
 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) {
@@ -260,12 +310,6 @@ void URLLoaderImpl::ReadMore() {
   } 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());
-    }
   }
 }
 

mojo/services/network/url_loader_impl.h

@@ -33,8 +33,11 @@ class URLLoaderImpl : public InterfaceImpl<URLLoader>,
   // URLLoader methods:
   virtual void Start(
       URLRequestPtr request,
-      ScopedDataPipeProducerHandle response_body_stream) OVERRIDE;
-  virtual void FollowRedirect() OVERRIDE;
+      const Callback<void(URLResponsePtr)>& callback) OVERRIDE;
+  virtual void FollowRedirect(
+      const Callback<void(URLResponsePtr)>& callback) OVERRIDE;
+  virtual void QueryStatus(
+      const Callback<void(URLLoaderStatusPtr)>& callback) OVERRIDE;
 
   // net::URLRequest::Delegate methods:
   virtual void OnReceivedRedirect(net::URLRequest* url_request,
@@ -44,7 +47,10 @@ class URLLoaderImpl : public InterfaceImpl<URLLoader>,
   virtual void OnReadCompleted(net::URLRequest* url_request, int bytes_read)
       OVERRIDE;
 
-  void SendError(int error);
+  void SendError(
+      int error,
+      const Callback<void(URLResponsePtr)>& callback);
+  void SendResponse(URLResponsePtr response);
   void OnResponseBodyStreamReady(MojoResult result);
   void WaitToReadMore();
   void ReadMore();
@@ -52,9 +58,11 @@ class URLLoaderImpl : public InterfaceImpl<URLLoader>,
 
   NetworkContext* context_;
   scoped_ptr<net::URLRequest> url_request_;
+  Callback<void(URLResponsePtr)> callback_;
   ScopedDataPipeProducerHandle response_body_stream_;
   scoped_refptr<PendingWriteToDataPipe> pending_write_;
   common::HandleWatcher handle_watcher_;
+  uint32 response_body_buffer_size_;
   bool auto_follow_redirects_;
 
   base::WeakPtrFactory<URLLoaderImpl> weak_ptr_factory_;

mojo/services/public/interfaces/navigation/navigation.mojom

@@ -26,8 +26,13 @@ struct NavigationDetails {
 struct ResponseDetails {
   // TODO(beng): consider providing access to URLRequest too. Currently it is
   //             not possible to obtain from the URLLoader.
+
+  // The URLLoader instance that generated the response. This must be kept
+  // alive until the response body has been completely consumed.
+  // TODO(darin): This should be |mojo.URLLoader loader|. See crbug/392693.
+  handle<message_pipe> loader_handle;
+
   mojo.URLResponse response;
-  handle<data_pipe_consumer> response_body_stream;
 };
 
 // Embedders that support navigation of implement this interface.

mojo/services/public/interfaces/network/url_loader.mojom

@@ -16,15 +16,20 @@ struct URLRequest {
   // Additional HTTP request headers.
   string[] headers;
 
-  // The payload for the request body. For HTTP requests, the method must be
-  // set to "POST" or "PUT".
-  handle<data_pipe_consumer> body;
+  // The payload for the request body, represented as a concatenation of data
+  // streams. For HTTP requests, the method must be set to "POST" or "PUT".
+  handle<data_pipe_consumer>[] body;
 
   // The number of bytes to be read from |body|. A Content-Length header of
   // this value will be sent. Set to -1 if length is unknown, which will cause
   // |body| to be uploaded using a chunked encoding.
   int64 body_length = 0;
 
+  // The buffer size of the data pipe returned in URLResponse's |body| member.
+  // A value of 0 indicates that the default buffer size should be used.  This
+  // value is just a suggestion. The URLLoader may choose to ignore this value.
+  uint32 response_body_buffer_size = 0;
+
   // If set to true, then redirects will be automatically followed. Otherwise,
   // when a redirect is encounterd, FollowRedirect must be called to proceed.
   bool auto_follow_redirects = false;
@@ -37,6 +42,13 @@ struct URLRequest {
 };
 
 struct URLResponse {
+  // If the response resulted in a network level error, this field will be set.
+  NetworkError error;
+
+  // The response body stream. Read from this data pipe to receive the bytes of
+  // response body.
+  handle<data_pipe_consumer> body;
+
   // The final URL of the response, after redirects have been followed.
   string url;
 
@@ -54,47 +66,41 @@ struct URLResponse {
 
   // The character set of the response body.
   string charset;
+
+  // These fields are set to non-NULL if this response corresponds to a
+  // redirect.  Call the |FollowRedirect| method on the URLLoader instance to
+  // follow this redirect.
+  string redirect_method;
+  string redirect_url;
+};
+
+struct URLLoaderStatus {
+  // If the loader has failed due to a network level error, this field will be
+  // set.
+  NetworkError error;
+
+  // Set to true if the URLLoader is still working. Set to false once an error
+  // is encountered or the response body is completely copied to the response
+  // body stream.
+  bool is_loading;
+
+  // TODO(darin): Add further details about the stages of loading (e.g.,
+  // "resolving host") that happen prior to receiving bytes.
 };
 
-[Client=URLLoaderClient]
 interface URLLoader {
-  // Start loading the given |request|. When available, the response body will
-  // be copied to |response_body_stream|.
-  //
-  // The client's |OnReceivedResponse| method will run when response meta data
-  // becomes available, or if a redirect response is encountered and
-  // |auto_follow_redirects| is false, the client's |OnRecievedRedirect| method
-  // will called instead.
-  //
-  // NOTE: You may observe data being pushed to |response_body_stream| before
-  // you receive |OnReceivedResponse|.
-  Start(URLRequest request, handle<data_pipe_producer> response_body_stream);
+  // Loads the given |request|, asynchronously producing |response|. Consult
+  // |response| to determine if the request resulted in an error, was
+  // redirected, or has a response body to be consumed.
+  Start(URLRequest request) => (URLResponse response);
 
   // If the request passed to |Start| had |auto_follow_redirects| set to false,
-  // then upon receiving a redirect, |OnReceivedRedirect| will be called. To
-  // follow the indicated redirect, call the |FollowRedirect| method.
-  FollowRedirect();
-};
+  // then upon receiving an URLResponse with a non-NULL |redirect_url| field,
+  // |FollowRedirect| may be called to load the URL indicated by the redirect.
+  FollowRedirect() => (URLResponse response);
 
-interface URLLoaderClient {
-  // This method is called when a redirect is encountered, provided the
-  // request's |auto_follow_redirects| attribute was set to false.
-  OnReceivedRedirect(URLResponse response, string new_url, string new_method);
-
-  // This method is called when response meta data becomes available.
-  OnReceivedResponse(URLResponse response);
-
-  // This method is called when a network level error is encountered. This can
-  // happen before or after OnReceivedResponse, but cannot happen after
-  // OnReceivedEndOfResponseBody.
-  OnReceivedError(NetworkError error);
-
-  // This method is called when the response body has been successfully
-  // downloaded and copied in its entirety to |response_body_stream|.
-  //
-  // NOTE: Because |response_body_stream| is limited in size, this event may be
-  // delayed until |response_body_stream| is consumed.
-  OnReceivedEndOfResponseBody();
+  // Query status about the URLLoader.
+  QueryStatus() => (URLLoaderStatus status);
 };
 
 }

mojo/shell/dynamic_service_loader.cc

@@ -80,19 +80,19 @@ class LocalLoader : public Loader {
 };
 
 // For loading services via the network stack.
-class NetworkLoader : public Loader, public URLLoaderClient {
+class NetworkLoader : public Loader {
  public:
   explicit NetworkLoader(scoped_ptr<DynamicServiceRunner> runner,
                          NetworkService* network_service)
       : Loader(runner.Pass()) {
     network_service->CreateURLLoader(Get(&url_loader_));
-    url_loader_.set_client(this);
   }
 
   virtual void Start(const GURL& url,
                     ScopedMessagePipeHandle service_handle,
                     Context* context) OVERRIDE {
     service_handle_ = service_handle.Pass();
+    context_ = context;
 
     URLRequestPtr request(URLRequest::New());
     request->url = url.spec();
@@ -103,17 +103,9 @@ class NetworkLoader : public Loader, public URLLoaderClient {
       request->bypass_cache = true;
     }
 
-    DataPipe data_pipe;
-    url_loader_->Start(request.Pass(), data_pipe.producer_handle.Pass());
-
-    base::CreateTemporaryFile(&file_);
-    common::CopyToFile(data_pipe.consumer_handle.Pass(),
-                       file_,
-                       context->task_runners()->blocking_pool(),
-                       base::Bind(&Loader::StartService,
-                                  base::Unretained(this),
-                                  file_,
-                                  base::Passed(&service_handle_)));
+    url_loader_->Start(request.Pass(),
+                       base::Bind(&NetworkLoader::OnReceivedResponse,
+                                  base::Unretained(this)));
   }
 
  private:
@@ -122,16 +114,24 @@ class NetworkLoader : public Loader, public URLLoaderClient {
       base::DeleteFile(file_, false);
   }
 
-  // URLLoaderClient methods:
-  virtual void OnReceivedRedirect(URLResponsePtr response,
-                                  const String& new_url,
-                                  const String& new_method) OVERRIDE {
-    // TODO(darin): Handle redirects properly!
+  void OnReceivedResponse(URLResponsePtr response) {
+    if (response->error) {
+      LOG(ERROR) << "Error (" << response->error->code << ": "
+                 << response->error->description << ") while fetching "
+                 << response->url;
+    }
+
+    base::CreateTemporaryFile(&file_);
+    common::CopyToFile(response->body.Pass(),
+                       file_,
+                       context_->task_runners()->blocking_pool(),
+                       base::Bind(&Loader::StartService,
+                                  base::Unretained(this),
+                                  file_,
+                                  base::Passed(&service_handle_)));
   }
-  virtual void OnReceivedResponse(URLResponsePtr response) OVERRIDE {}
-  virtual void OnReceivedError(NetworkErrorPtr error) OVERRIDE {}
-  virtual void OnReceivedEndOfResponseBody() OVERRIDE {}
 
+  Context* context_;
   NetworkServicePtr network_service_;
   URLLoaderPtr url_loader_;
   ScopedMessagePipeHandle service_handle_;

mojo/shell/in_process_dynamic_service_runner.cc

@@ -74,9 +74,12 @@ void InProcessDynamicServiceRunner::Run() {
         break;
       }
     } else {
+      // In the component build, Mojo Apps link against mojo_system_impl.
+#if !defined(COMPONENT_BUILD)
       // Strictly speaking this is not required, but it's very unusual to have
       // an app that doesn't require the basic system library.
       LOG(WARNING) << "MojoSetSystemThunks not found in app library";
+#endif
     }
 
     typedef MojoResult (*MojoMainFunction)(MojoHandle);