diff --git a/include/wx/private/webrequest.h b/include/wx/private/webrequest.h index 1b18d583c6..4f502b9af5 100644 --- a/include/wx/private/webrequest.h +++ b/include/wx/private/webrequest.h @@ -106,6 +106,13 @@ public: wxEvtHandler* GetHandler() const { return m_handler; } + // Called to notify about the state change in the main thread by SetState() + // (which can itself be called from a different one). + // + // It also releases a reference added when switching to the active state by + // SetState() when leaving it. + // + // TODO-C++11: make private when we don't need StateEventProcessor any more. void ProcessStateEvent(wxWebRequest::State state, const wxString& failMsg); protected: diff --git a/src/common/webrequest.cpp b/src/common/webrequest.cpp index 189b6d1c16..20579d29c5 100644 --- a/src/common/webrequest.cpp +++ b/src/common/webrequest.cpp @@ -175,15 +175,6 @@ struct StateEventProcessor const wxString& failMsg) : m_request(request), m_state(state), m_failMsg(failMsg) { - // Ensure that the request object stays alive until this event is - // processed. - m_request.IncRef(); - } - - StateEventProcessor(const StateEventProcessor& other) - : m_request(other.m_request), m_state(other.m_state), m_failMsg(other.m_failMsg) - { - m_request.IncRef(); } void operator()() @@ -191,11 +182,6 @@ struct StateEventProcessor m_request.ProcessStateEvent(m_state, m_failMsg); } - ~StateEventProcessor() - { - m_request.DecRef(); - } - wxWebRequestImpl& m_request; const wxWebRequest::State m_state; const wxString m_failMsg; @@ -230,6 +216,17 @@ void wxWebRequestImpl::SetState(wxWebRequest::State state, const wxString & fail wxLogTrace(wxTRACE_WEBREQUEST, "Request %p: state %s => %s", this, StateName(m_state), StateName(state)); + if ( state == wxWebRequest::State_Active ) + { + // The request is now in progress (maybe not for the first time if the + // old state was State_Unauthorized and not State_Idle), ensure that it + // stays alive until it terminates, even if wxWebRequest object itself + // is deleted. + // + // Note that matching DecRef() is done by ProcessStateEvent() later. + IncRef(); + } + m_state = state; // Trigger the event in the main thread except when switching to the active @@ -343,6 +340,7 @@ void wxWebRequestImpl::ProcessStateEvent(wxWebRequest::State state, const wxStri wxWebRequestEvent evt(wxEVT_WEBREQUEST_STATE, GetId(), state, wxWebResponse(response), failMsg); + bool release = false; switch ( state ) { case wxWebRequest::State_Idle: @@ -350,7 +348,17 @@ void wxWebRequestImpl::ProcessStateEvent(wxWebRequest::State state, const wxStri break; case wxWebRequest::State_Active: + break; + case wxWebRequest::State_Unauthorized: + // This one is tricky: we might not be done with the request yet, + // but we don't know if this is the case or not, i.e. if the + // application will call wxWebAuthChallenge::SetCredentials() or + // not later. So we release it now, as we assume that it still + // keeps a reference to the original request if it intends to do it + // anyhow, i.e. this won't actually destroy the request object in + // this case. + release = true; break; case wxWebRequest::State_Completed: @@ -365,6 +373,8 @@ void wxWebRequestImpl::ProcessStateEvent(wxWebRequest::State state, const wxStri case wxWebRequest::State_Cancelled: if ( response ) response->Finalize(); + + release = true; break; } @@ -374,6 +384,10 @@ void wxWebRequestImpl::ProcessStateEvent(wxWebRequest::State state, const wxStri // could have been deleted or moved away by the event handler. if ( !dataFile.empty() && wxFileExists(dataFile) ) wxRemoveFile(dataFile); + + // This may destroy this object if it's not used from elsewhere any longer. + if ( release ) + DecRef(); } // diff --git a/tests/net/webrequest.cpp b/tests/net/webrequest.cpp index 5051b66386..32c2f8b981 100644 --- a/tests/net/webrequest.cpp +++ b/tests/net/webrequest.cpp @@ -104,7 +104,7 @@ public: break; case wxWebRequest::State_Completed: - if ( request.GetStorage() == wxWebRequest::Storage_File ) + if ( request.IsOk() && request.GetStorage() == wxWebRequest::Storage_File ) { wxFileName fn(evt.GetDataFile()); CHECK( fn.GetSize() == expectedFileSize ); @@ -438,6 +438,26 @@ TEST_CASE_METHOD(RequestFixture, REQUIRE( request.GetState() == wxWebRequest::State_Cancelled ); } +TEST_CASE_METHOD(RequestFixture, + "WebRequest::Destroy", "[net][webrequest]") +{ + if ( !InitBaseURL() ) + return; + + Create("/base64/U3RpbGwgYWxpdmUh"); + request.Start(); + + // Destroy the original request: this shouldn't prevent it from running to + // the completion! + request = wxWebRequest(); + + RunLoopWithTimeout(); + + CHECK( stateFromEvent == wxWebRequest::State_Completed ); + CHECK( statusFromEvent == 200 ); + CHECK( responseStringFromEvent == "Still alive!" ); +} + // This test is not run by default and has to be explicitly selected to run. TEST_CASE_METHOD(RequestFixture, "WebRequest::Manual", "[.]")