1377101 - Bug 1376951 Let's use CheckedUint32 more broadly in XHR code

Status: RESOLVED FIXED

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

Comment 1

[Andrea Marchesini [:baku]]

Attachment: checked_xhr.patch

Comment 2

[:Nika Layzell (on leave)]

Comment on attachment 8882151 [details] [diff] [review]
checked_xhr.patch

Review of attachment 8882151 [details] [diff] [review]:
-----------------------------------------------------------------

Patch looks good, but I don't see what this has to do with CheckedUint32. It looks to me like this code is simply improving our error handling in XHR code.

::: dom/xhr/XMLHttpRequestMainThread.cpp
@@ -316,5 @@
>    mBlobSet = nullptr;
>    mResultArrayBuffer = nullptr;
>    mArrayBufferBuilder.reset();
>    mResultJSON.setUndefined();
> -  mDataAvailable = 0;

I appreciate that we have so many unused variables ^.^

@@ +1722,5 @@
>      if (xmlHttpRequest->mArrayBufferBuilder.capacity() == 0)
>        xmlHttpRequest->mArrayBufferBuilder.setCapacity(std::max(count, XML_HTTP_REQUEST_ARRAYBUFFER_MIN_SIZE));
>  
> +    if (NS_WARN_IF(!xmlHttpRequest->mArrayBufferBuilder.append(
> +                      reinterpret_cast<const uint8_t*>(fromRawSegment), count,

nit: Can you move count down a line? It's kinda hiding off to the right here.

Comment 3

[Andrea Marchesini [:baku]]

> Patch looks good, but I don't see what this has to do with CheckedUint32. It
> looks to me like this code is simply improving our error handling in XHR
> code.

Right, because at the beginning I was using CheckedInt for mDataAvailable. But then I realize that we don't use this variable at all.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c04091fe27f7
Add some return value checks in XHR, r=mystor

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c04091fe27f7