STEPS TO REPRODUCE:
1. Load any remote image in a debug build, e.g.:
2. Hold down Ctrl + R (to perform many repeated reloads)
This warning gets printed in my terminal:
WARNING: NS_ENSURE_TRUE(mState == STATE_TRANSFERRING) failed: file ../../../../mozilla/netwerk/base/src/nsSocketTransport2.cpp, line 1889
No warning. This is a perfectly normal use-case (interrupting an in-progress load with a reload), and normal use-cases shouldn't trigger warning-spam.
(This same warning gets printed for lots of other sorts of reloads, too - e.g. if I load mozilla.org and reload while it's loading. It's simplest with a single image, though, because there's less other noise going by in the terminal.)
I was able to reproduce only with mState == STATE_CLOSED (== 0). We might extend the check to also include STATE_CLOSED but only while mNetAddr has a valid (or simply not invalid) family set.
Or, when state is not STATE_RESOLVING and mNetAddr is not invalid. Because only during the RESOLVING phase the structure may change.
The common trigger for this seems to be an asynchronous nsITransportStatusEvent that calls GetPeerAddr() after the connection has closed.
The code should not be using NS_ENSURE_TRUE() which produces the WARNING - calling GetPeerAddr on an unconnected socket will return an error but it isn't a violation to make the call. I will change that to be a normal conditional plus a LOG message.
I will also enhance this to match the idl comment which says GetPeerAddr() is valid once the connection has been established (which includes after it is closed) by setting a flag when we get to TRANSFERRING and allowing access to mNetAddr anytime after that point.
patch to follow.
Created attachment 591863 [details] [diff] [review]
Comment on attachment 591863 [details] [diff] [review]
SOCKET_LOG((" advancing to STATE_TRANSFERRING\n"));
mPollFlags = (PR_POLL_READ | PR_POLL_WRITE | PR_POLL_EXCEPT);
mPollTimeout = mTimeouts[TIMEOUT_READ_WRITE];
mState = STATE_TRANSFERRING;
+ mNetAddrIsSet = true;
Please add a comment here about why you set this to true only now, i.e. that this is the earliest time where we can be sure what host we're establishing the connection to due to failover.
sidenote, this is why I don't like the NS_ENSURE_* macros