Take raw pointer and not wxSharedPtr<> in SetData()

Using shared pointer seems to be ill-advised here, the stream shouldn't
be shared as it's going to be used by wxWebRequest itself and can't be
used by the application code in parallel, so the ownership transfer
semantics is more appropriate.

We could take a wxScopedPtr<> instead, but wx API takes ownership of raw
pointers everywhere else, so do it here too.

Incidentally fix a bug with calling IsOk() on a possibly null pointer.

include/wx/private/webrequest.h

@@ -56,7 +56,7 @@ public:
 
     void SetData(const wxString& text, const wxString& contentType, const wxMBConv& conv = wxConvUTF8);
 
-    bool SetData(const wxSharedPtr<wxInputStream>& dataStream, const wxString& contentType, wxFileOffset dataSize = wxInvalidOffset);
+    bool SetData(wxScopedPtr<wxInputStream>& dataStream, const wxString& contentType, wxFileOffset dataSize = wxInvalidOffset);
 
     void SetIgnoreServerErrorStatus(bool ignore) { m_ignoreServerErrorStatus = ignore; }
 
@@ -99,7 +99,7 @@ protected:
     wxWebRequest::Storage m_storage;
     wxWebRequestHeaderMap m_headers;
     wxFileOffset m_dataSize;
-    wxSharedPtr<wxInputStream> m_dataStream;
+    wxScopedPtr<wxInputStream> m_dataStream;
 
     wxWebRequestImpl(wxWebSession& session, wxEvtHandler* handler, int id);
 

include/wx/webrequest.h

@@ -134,7 +134,7 @@ public:
 
     void SetData(const wxString& text, const wxString& contentType, const wxMBConv& conv = wxConvUTF8);
 
-    bool SetData(const wxSharedPtr<wxInputStream>& dataStream, const wxString& contentType, wxFileOffset dataSize = wxInvalidOffset);
+    bool SetData(wxInputStream* dataStream, const wxString& contentType, wxFileOffset dataSize = wxInvalidOffset);
 
     void SetIgnoreServerErrorStatus(bool ignore);
 

interface/wx/webrequest.h

@@ -269,10 +269,23 @@ public:
         @c GET and the given @a dataStream will be posted as the body of
         this request.
 
+        Example of use:
+        @code
+        std::unique_ptr<wxInputStream> stream(new wxFileInputStream("some_file.dat"));
+        if ( !stream->IsOk() ) {
+            // Handle error (due to e.g. file not found) here.
+            ...
+            return;
+        }
+        request.SetData(stream.release(), "application/octet-stream")
+        @endcode
+
         @param dataStream
             The data in this stream will be posted as the request body. The
-            stream may be empty, which will result in sending 0 bytes of data,
+            pointer may be @NULL, which will result in sending 0 bytes of data,
-            but if not empty, should be valid.
+            but if not empty, should be valid, i.e. wxInputStream::IsOk() must
+            return @true. This object takes ownership of the passed in pointer
+            and will delete it, i.e. the pointer must be heap-allocated.
         @param contentType
             The value of HTTP "Content-Type" header, e.g.
             "application/octet-stream".
@@ -284,7 +297,7 @@ public:
             dataSize is not specified and the attempt to determine stream size
             failed; @true in all the other cases.
     */
-    bool SetData(const wxSharedPtr<wxInputStream>& dataStream,
+    bool SetData(wxInputStream* dataStream,
         const wxString& contentType, wxFileOffset dataSize = wxInvalidOffset);
 
     /**

src/common/webrequest.cpp

@@ -97,17 +97,23 @@ bool wxWebRequestImpl::IsActiveState(wxWebRequest::State state)
 void wxWebRequestImpl::SetData(const wxString& text, const wxString& contentType, const wxMBConv& conv)
 {
     m_dataText = text.mb_str(conv);
-    SetData(wxSharedPtr<wxInputStream>(new wxMemoryInputStream(m_dataText, m_dataText.length())), contentType);
+
+    wxScopedPtr<wxInputStream>
+        stream(new wxMemoryInputStream(m_dataText, m_dataText.length()));
+    SetData(stream, contentType);
 }
 
-bool wxWebRequestImpl::SetData(const wxSharedPtr<wxInputStream>& dataStream, const wxString& contentType, wxFileOffset dataSize)
+bool
+wxWebRequestImpl::SetData(wxScopedPtr<wxInputStream>& dataStream,
+                          const wxString& contentType,
+                          wxFileOffset dataSize)
 {
-    if ( !dataStream->IsOk() )
+    m_dataStream.reset(dataStream.release());
-        return false;
 
-    m_dataStream = dataStream;
+    if ( m_dataStream )
-    if ( m_dataStream.get() )
     {
+        wxCHECK_MSG( m_dataStream->IsOk(), false, "can't use invalid stream" );
+
         if ( dataSize == wxInvalidOffset )
         {
             // Determine data size
@@ -342,13 +348,16 @@ void wxWebRequest::SetData(const wxString& text, const wxString& contentType, co
 }
 
 bool
-wxWebRequest::SetData(const wxSharedPtr<wxInputStream>& dataStream,
+wxWebRequest::SetData(wxInputStream* dataStream,
                       const wxString& contentType,
                       wxFileOffset dataSize)
 {
+    // Ensure that the stream is destroyed even we return below.
+    wxScopedPtr<wxInputStream> streamPtr(dataStream);
+
     wxCHECK_IMPL( false );
 
-    return m_impl->SetData(dataStream, contentType, dataSize);
+    return m_impl->SetData(streamPtr, contentType, dataSize);
 }
 
 void wxWebRequest::SetIgnoreServerErrorStatus(bool ignore)

tests/net/webrequest.cpp

@@ -178,10 +178,10 @@ TEST_CASE_METHOD(RequestFixture, "WebRequest", "[net][webrequest]")
     SECTION("PUT file data")
     {
         Create("/put");
-        wxSharedPtr<wxInputStream> is(new wxFileInputStream("horse.png"));
+        wxScopedPtr<wxInputStream> is(new wxFileInputStream("horse.png"));
         REQUIRE( is->IsOk() );
 
-        request.SetData(is, "image/png");
+        request.SetData(is.release(), "image/png");
         request.SetMethod("PUT");
         Run();
     }