Under MSW, don't set the state to State_Cancelled as soon as Cancel()
was called, as the request was still used from the other threads
afterwards, resulting in race conditions and crashes.
Fix this by just removing the SetState(State_Cancelled) call from the
main thread, as it was redundant anyhow. This also makes the behaviour
correspond to the documentation, which indicates that Cancel() works
asynchronously.
Also ensure, for all backends, that we actually cancel the request only
once, even if public Cancel() is called multiple times. This required
renaming the existing wxWebRequestImpl::Cancel() to DoCancel().
It's up to the application code to decide how it handles the HTTP status
codes it gets back from server, there is no need to have a special
method for handling them in wxWebRequest itself.
We also shouldn't skip downloading the response body just because it was
unsuccessful, we may still need it (e.g. it's very common to return the
detailed error description in a JSON object in the message body when
returning some 4xx error), so don't do it in wxMSW implementation and
add a test verifying that we still get the expected body even for an
error status.
Also improve wxWebRequest::State values documentation.
This is shorter and doesn't imply that just the name (and not the full
path) is being returned.
Also rename wxWebResponse::GetFileName() to GetDataFile() for the same
reasons and for consistency. And document this previously undocumented
method.
Check that current state is State_Idle in wxWebRequest itself only once
instead of doing it in 2 (out of 3) wxWebRequestImpl implementations.
Also assert if this is not the case instead of silently doing nothing
which would surely be more difficult to debug.
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.
Don't force the application code to deal with wxObjectDataPtr<> or,
worse, calling {Inc,Dec}Ref() manually by hiding it inside the wx
objects themselves and giving the value-like semantics to them.
There should be no real changes in the behaviour, but the API does
change significantly. Notably, wxWebRequest is not a wxEvtHandler itself
any longer, as this would be incompatible with the value semantics, and
an event handler needs to be specified when creating it, so that it
could be notified about the request state changes.
Having wxWebSessionFactory part of the public API implies keeping
compatibility with the possible ways of implementing it which is too
restrictive for no good reason, so move this class to the private header
and don't document it nor wxWebSession::RegisterFactory() (which is now
private).
They're not necessary to use this class and we may consider exporting
them later, possibly with a better API and more tests, if really needed.
Also do change their API slightly by leaving only a single function and
returning the value instead of using an out parameter for it to make it
simpler to use.
It doesn't make much sense to specify the conversion here, it would
ideally be taken from the response Content-Type header itself and
currently is just assumed to be UTF-8 anyhow.
Also implement fallback to Latin-1 to avoid losing the data entirely if
it's not in UTF-8.
Check that the stream is valid, if specified at all, and return false if
it isn't -- or if no size was specified and determining stream size
failed.
Check for SetData() success in the test to provide better diagnostics in
case the file it uses is not found (as is the case when running the test
from another directory, for example).
Also pass wxSharedPtr<> by const reference instead of by value to avoid
unnecessary copies.