565500 - nsPrefetchNode::GetStatus sets its PRUint16 outparam to a nsresult value instead of an enum from nsIDOMLoadStatus

Status: RESOLVED DUPLICATE of bug 562403

(Core :: DOM: Core & HTML, defect)

Description

[Daniel Holbert [:dholbert]]

AFAICT, nsPrefetchNode::GetStatus is supposed to return one of the nsIDOMLoadStatus enumerated values in its PRUint16 outparam. (e.g. nsIDOMLoadStatus::REQUESTED)

However, right now it doesn't use any of those enumerated values, and instead it sets its outparam to one of the following:
   - 0                                                   (line 841 & 861)
   - NS_ERROR_NOT_AVAILABLE                              (line 857)
   - PRUint16(httpStatus) where httpStatus is a nsresult (line 866)
http://mxr.mozilla.org/mozilla-central/source/uriloader/prefetch/nsPrefetchService.cpp?mark=841-841,857-857,861-861,866-866#837

The nsresult ones are particularly troubling, because IIUC, nsresult "error" codes vs "OK" codes are only differentiated based on whether the highest bit is set or not. And that highest bit gets truncated when converting to a PRUint16.

Note that this bug triggers the following compile warning:
> uriloader/prefetch/nsPrefetchService.cpp:857: warning: large integer implicitly truncated to unsigned type

Comment 1

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to comment #0)
>    - PRUint16(httpStatus) where httpStatus is a nsresult (line 866)
httpStatus isn't a nsresult, this one is actually correct.

Comment 3

[Daniel Holbert [:dholbert]]

(In reply to comment #1)
> httpStatus isn't a nsresult

Oops, you're right that it's not an nsresult there -- it's a PRUint32 -- but that one's still not correct.  We're still truncating a 32-bit value to be 16-bit, and we're converting between "spaces" of enumerated types.  AFAICT, httpStatus is a 32-bit HTTP response code (e.g. "200"), as documented in nsIHttpChannel.idl [1].  Whereas, as noted in comment 0, the status returned by nsPrefetchNode is a 16-bit  enum value from nsIDOMLoadStatus.idl[2], in the range of 0 through 4.

Anyway, moving this discussion to the other bug (bug 562403), sorry for the dupe.

[1] http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/public/nsIHttpChannel.idl?mark=183-189#177
[2] http://mxr.mozilla.org/mozilla-central/source/dom/interfaces/offline/nsIDOMLoadStatus.idl?mark=49-54#51

Comment 4

[Daniel Holbert [:dholbert]]

(Actually, comment 3 might be wrong -- looks like callers do (at least sometimes) expect aStatus to end up with an HTTP response code, as I just described on the duplicate, in bug 562403 comment 6)