Misc changes in the http portions:

1. Fixed a bunch of parameters that should be passing by const reference.
   Some of these were trying to pass by value and 'move' when it isn't going to
   be more efficient and isn't worth it.
2. Updated a bunch of places passing shared_ptr by value.
3. Removed some unnecessary constructors/assignment operators.
4. Removed pointless streambuf check from desktop client implementation.
5. Updated some comments to be more clear.

Author: stgates

Release/include/cpprest/http_client.h

@@ -1,12 +1,12 @@
 /***
 * ==++==
 *
-* Copyright (c) Microsoft Corporation. All rights reserved. 
+* Copyright (c) Microsoft Corporation. All rights reserved.
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 * http://www.apache.org/licenses/LICENSE-2.0
-* 
+*
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -49,7 +49,7 @@ typedef void* native_handle;}}}
 #if defined(_MSC_VER) && (_MSC_VER >= 1800)
 #include <ppltasks.h>
 namespace pplx = Concurrency;
-#else 
+#else
 #include "pplx/pplxtasks.h"
 #endif
 
@@ -66,7 +66,7 @@ namespace pplx = Concurrency;
 #include "cpprest/oauth2.h"
 
 /// The web namespace contains functionality common to multiple protocols like HTTP and WebSockets.
-namespace web 
+namespace web
 {
 /// Declarations and functionality for the HTTP protocol.
 namespace http
@@ -88,7 +88,7 @@ namespace details {
 #else
         class winhttp_client;
 #endif // __cplusplus_winrt
-}  
+}
 #endif // _MS_WINDOWS
 
 /// <summary>
@@ -98,7 +98,7 @@ namespace details {
 class http_client_config
 {
 public:
-    http_client_config() : 
+    http_client_config() :
         m_guarantee_order(false),
         m_timeout(utility::seconds(30)),
         m_chunksize(0)
@@ -217,7 +217,7 @@ class http_client_config
     /// Set the timeout
     /// </summary>
     /// <param name="timeout">The timeout (in seconds) used for each send and receive operation on the client.</param>
-    void set_timeout(utility::seconds timeout)
+    void set_timeout(const utility::seconds &timeout)
     {
         m_timeout = timeout;
     }
@@ -284,7 +284,7 @@ class http_client_config
     }
 
     /// <summary>
-    /// Sets the request buffering property. 
+    /// Sets the request buffering property.
     /// If true, in cases where the request body/stream doesn't support seeking the request data will be buffered.
     /// This can help in situations where an authentication challenge might be expected.
     /// </summary>
@@ -296,12 +296,12 @@ class http_client_config
     }
 #endif
 #endif
-    
+
     /// <summary>
     /// Sets a callback to enable custom setting of winhttp options
     /// </summary>
     /// <param name="callback">A user callback allowing for customization of the request</param>
-    void set_nativehandle_options(std::function<void(native_handle)> callback)
+    void set_nativehandle_options(const std::function<void(native_handle)> &callback)
     {
          m_set_user_nativehandle_options = callback;
     }
@@ -369,7 +369,7 @@ class http_client
     /// Note the destructor doesn't necessarily close the connection and release resources.
     /// The connection is reference counted with the http_responses.
     /// </summary>
-    ~http_client() {}
+    ~http_client() _noexcept {}
 
     /// <summary>
     /// Gets the base uri
@@ -398,7 +398,7 @@ class http_client
     /// Adds an HTTP pipeline stage to the client.
     /// </summary>
     /// <param name="stage">A shared pointer to a pipeline stage.</param>
-    void add_handler(std::shared_ptr<http::http_pipeline_stage> stage)
+    void add_handler(const std::shared_ptr<http::http_pipeline_stage> &stage)
     {
         m_pipeline->append(stage);
     }
@@ -409,17 +409,17 @@ class http_client
     /// <param name="request">Request to send.</param>
     /// <param name="token">Cancellation token for cancellation of this request operation.</param>
     /// <returns>An asynchronous operation that is completed once a response from the request is received.</returns>
-    _ASYNCRTIMP pplx::task<http_response> request(http_request request, pplx::cancellation_token token = pplx::cancellation_token::none());
+    _ASYNCRTIMP pplx::task<http_response> request(http_request request, const pplx::cancellation_token &token = pplx::cancellation_token::none());
 
     /// <summary>
     /// Asynchronously sends an HTTP request.
     /// </summary>
     /// <param name="mtd">HTTP request method.</param>
     /// <param name="token">Cancellation token for cancellation of this request operation.</param>
     /// <returns>An asynchronous operation that is completed once a response from the request is received.</returns>
-    pplx::task<http_response> request(method mtd, pplx::cancellation_token token = pplx::cancellation_token::none())
+    pplx::task<http_response> request(const method &mtd, const pplx::cancellation_token &token = pplx::cancellation_token::none())
     {
-        http_request msg(std::move(mtd));
+        http_request msg(mtd);
         return request(msg, token);
     }
 
@@ -431,11 +431,11 @@ class http_client
     /// <param name="token">Cancellation token for cancellation of this request operation.</param>
     /// <returns>An asynchronous operation that is completed once a response from the request is received.</returns>
     pplx::task<http_response> request(
-        method mtd, 
+        const method &mtd,
         const utility::string_t &path_query_fragment,
-        pplx::cancellation_token token = pplx::cancellation_token::none())
+        const pplx::cancellation_token &token = pplx::cancellation_token::none())
     {
-        http_request msg(std::move(mtd));
+        http_request msg(mtd);
         msg.set_request_uri(path_query_fragment);
         return request(msg, token);
     }
@@ -449,12 +449,12 @@ class http_client
     /// <param name="token">Cancellation token for cancellation of this request operation.</param>
     /// <returns>An asynchronous operation that is completed once a response from the request is received.</returns>
     pplx::task<http_response> request(
-        method mtd, 
-        const utility::string_t &path_query_fragment, 
+        const method &mtd,
+        const utility::string_t &path_query_fragment,
         const json::value &body_data,
-        pplx::cancellation_token token = pplx::cancellation_token::none())
+        const pplx::cancellation_token &token = pplx::cancellation_token::none())
     {
-        http_request msg(std::move(mtd));
+        http_request msg(mtd);
         msg.set_request_uri(path_query_fragment);
         msg.set_body(body_data);
         return request(msg, token);
@@ -469,16 +469,17 @@ class http_client
     /// <param name="body_data">String containing the text to use in the message body.</param>
     /// <param name="token">Cancellation token for cancellation of this request operation.</param>
     /// <returns>An asynchronous operation that is completed once a response from the request is received.</returns>
+    // TODO overload???Not sure...
     pplx::task<http_response> request(
-        method mtd, 
+        const method &mtd,
         const utility::string_t &path_query_fragment,
         const utility::string_t &body_data,
-        utility::string_t content_type = _XPLATSTR("text/plain"),
-        pplx::cancellation_token token = pplx::cancellation_token::none())
+        const utility::string_t &content_type = _XPLATSTR("text/plain"),
+        const pplx::cancellation_token &token = pplx::cancellation_token::none())
     {
-        http_request msg(std::move(mtd));
+        http_request msg(mtd);
         msg.set_request_uri(path_query_fragment);
-        msg.set_body(body_data, std::move(content_type));
+        msg.set_body(body_data, content_type);
         return request(msg, token);
     }
 
@@ -490,11 +491,12 @@ class http_client
     /// <param name="body_data">String containing the text to use in the message body.</param>
     /// <param name="token">Cancellation token for cancellation of this request operation.</param>
     /// <returns>An asynchronous operation that is completed once a response from the request is received.</returns>
+    // TODO overload not usre???
     pplx::task<http_response> request(
-        method mtd, 
+        const method &mtd,
         const utility::string_t &path_query_fragment,
         const utility::string_t &body_data,
-        pplx::cancellation_token token)
+        const pplx::cancellation_token &token)
     {
         return request(mtd, path_query_fragment, body_data, _XPLATSTR("text/plain"), token);
     }
@@ -510,15 +512,15 @@ class http_client
     /// <param name="token">Cancellation token for cancellation of this request operation.</param>
     /// <returns>A task that is completed once a response from the request is received.</returns>
     pplx::task<http_response> request(
-        method mtd, 
+        const method &mtd,
         const utility::string_t &path_query_fragment,
-        concurrency::streams::istream body,
-        utility::string_t content_type = _XPLATSTR("application/octet-stream"),
-        pplx::cancellation_token token = pplx::cancellation_token::none())
+        const concurrency::streams::istream &body,
+        const utility::string_t &content_type = _XPLATSTR("application/octet-stream"),
+        const pplx::cancellation_token &token = pplx::cancellation_token::none())
     {
-        http_request msg(std::move(mtd));
+        http_request msg(mtd);
         msg.set_request_uri(path_query_fragment);
-        msg.set_body(body, std::move(content_type));
+        msg.set_body(body, content_type);
         return request(msg, token);
     }
 
@@ -531,10 +533,10 @@ class http_client
     /// <param name="token">Cancellation token for cancellation of this request operation.</param>
     /// <returns>A task that is completed once a response from the request is received.</returns>
     pplx::task<http_response> request(
-        method mtd, 
+        const method &mtd,
         const utility::string_t &path_query_fragment,
-        concurrency::streams::istream body,
-        pplx::cancellation_token token)
+        const concurrency::streams::istream &body,
+        const pplx::cancellation_token &token)
     {
         return request(mtd, path_query_fragment, body, _XPLATSTR("application/octet-stream"), token);
     }
@@ -552,16 +554,16 @@ class http_client
     /// <returns>A task that is completed once a response from the request is received.</returns>
     /// <remarks>Winrt requires to provide content_length.</remarks>
     pplx::task<http_response> request(
-        method mtd, 
+        const method &mtd,
         const utility::string_t &path_query_fragment,
-        concurrency::streams::istream body,
+        const concurrency::streams::istream &body,
         size_t content_length,
-        utility::string_t content_type= _XPLATSTR("application/octet-stream"),
-        pplx::cancellation_token token = pplx::cancellation_token::none())
+        const utility::string_t &content_type = _XPLATSTR("application/octet-stream"),
+        const pplx::cancellation_token &token = pplx::cancellation_token::none())
     {
-        http_request msg(std::move(mtd));
+        http_request msg(mtd);
         msg.set_request_uri(path_query_fragment);
-        msg.set_body(body, content_length, std::move(content_type));
+        msg.set_body(body, content_length, content_type);
         return request(msg, token);
     }
 
@@ -576,22 +578,22 @@ class http_client
     /// <returns>A task that is completed once a response from the request is received.</returns>
     /// <remarks>Winrt requires to provide content_length.</remarks>
     pplx::task<http_response> request(
-        method mtd, 
+        const method &mtd,
         const utility::string_t &path_query_fragment,
-        concurrency::streams::istream body,
+        const concurrency::streams::istream &body,
         size_t content_length,
-        pplx::cancellation_token token)
+        const pplx::cancellation_token &token)
     {
         return request(mtd, path_query_fragment, body, content_length, _XPLATSTR("application/octet-stream"), token);
     }
 
 private:
 
     void build_pipeline(uri base_uri, http_client_config client_config);
-    
+
     std::shared_ptr<::web::http::http_pipeline> m_pipeline;
 };
 
-}}} // namespaces
+}}}
 
 #endif  /* _CASA_HTTP_CLIENT_H */

Release/include/cpprest/http_client_impl.h

@@ -1,7 +1,7 @@
 /***
 * ==++==
 *
-* Copyright (c) Microsoft Corporation. All rights reserved. 
+* Copyright (c) Microsoft Corporation. All rights reserved.
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
@@ -40,7 +40,7 @@ using namespace Platform;
 using namespace Microsoft::WRL;
 #endif
 
-using namespace web; 
+using namespace web;
 using namespace concurrency;
 #else
 #include <boost/asio.hpp>
@@ -162,7 +162,7 @@ class request_context
     void report_exception(std::exception_ptr exceptionPtr)
     {
         auto response_impl = m_response._get_impl();
-        
+
         // If cancellation has been triggered then ignore any errors.
         if(m_request._cancellation_token().is_canceled())
         {
@@ -221,10 +221,10 @@ class request_context
 
 protected:
 
-    request_context(std::shared_ptr<_http_client_communicator> client, http_request &request)
-        : m_http_client(client), 
+    request_context(const std::shared_ptr<_http_client_communicator> &client, const http_request &request)
+        : m_http_client(client),
         m_request(request),
-        m_exceptionPtr(), 
+        m_exceptionPtr(),
         m_uploaded(0),
         m_downloaded(0)
     {
@@ -254,7 +254,7 @@ class _http_client_communicator
     virtual ~_http_client_communicator() {}
 
     // Asynchronously send a HTTP request and process the response.
-    void async_send_request(std::shared_ptr<request_context> request)
+    void async_send_request(const std::shared_ptr<request_context> &request)
     {
         if(m_client_config.guarantee_order())
         {
@@ -314,7 +314,7 @@ class _http_client_communicator
     virtual unsigned long open() = 0;
 
     // HTTP client implementations must implement send_request.
-    virtual void send_request(_In_ std::shared_ptr<request_context> request) = 0;
+    virtual void send_request(_In_ const std::shared_ptr<request_context> &request) = 0;
 
     // URI to connect to.
     const http::uri m_uri;
@@ -328,7 +328,7 @@ class _http_client_communicator
     pplx::extensibility::critical_section_t m_open_lock;
 
     // Wraps opening the client around sending a request.
-    void open_and_send_request(std::shared_ptr<request_context> request)
+    void open_and_send_request(const std::shared_ptr<request_context> &request)
     {
         // First see if client needs to be opened.
         auto error = open_if_required();
@@ -370,16 +370,14 @@ class _http_client_communicator
         return error;
     }
 
-    void push_request(_In_opt_ std::shared_ptr<request_context> request)
+    void push_request(const std::shared_ptr<request_context> &request)
     {
-        if (request == nullptr) return;
-
         pplx::extensibility::scoped_critical_section_t l(m_open_lock);
 
         if(++m_scheduled == 1)
         {
             // Schedule a task to start sending.
-            pplx::create_task([this, request]() -> void
+            pplx::create_task([this, request]()
             {
                 open_and_send_request(request);
             });
@@ -403,7 +401,7 @@ inline void request_context::finish()
         _ASSERTE(m_request._cancellation_token() != pplx::cancellation_token::none());
         m_request._cancellation_token().deregister_callback(m_cancellationRegistration);
     }
-    
+
     m_http_client->finish_request();
 }