Closed Bug 846852 Opened 7 years ago Closed 7 years ago

Preallocate source data for file:// URIs

Categories

(Core :: ImageLib, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: joe, Assigned: joe)

Details

Attachments

(1 file)

Big images take a lot longer to load from file:// than http:// because we don't preallocate the source data array for file:// URIs. This patch does that.
Attachment #720054 - Flags: review?(seth)
Comment on attachment 720054 [details] [diff] [review]
preallocate on file://

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

Looks good. My only concern is making GetContentSize not bother with the new file channel-related code if the http channel-related code succeeds. I suggest how you might do it below, but regardless of your approach, r=me with that change.

::: image/src/ImageFactory.cpp
@@ +143,5 @@
> +
> +uint32_t
> +GetContentSize(nsIRequest* aRequest)
> +{
> +  int32_t len = 0;

Don't declare this here.

@@ +152,5 @@
> +    nsAutoCString contentLength;
> +    nsresult rv = httpChannel->GetResponseHeader(NS_LITERAL_CSTRING("content-length"),
> +                                                 contentLength);
> +    if (NS_SUCCEEDED(rv)) {
> +      len = contentLength.ToInteger(&rv);

Just "return std::max(contentLength.ToInteger(&rv), 0);".

@@ +165,5 @@
> +    if (NS_SUCCEEDED(rv)) {
> +      int64_t filesize;
> +      rv = file->GetFileSize(&filesize);
> +      if (NS_SUCCEEDED(rv)) {
> +        len = SaturateToInt32(filesize);

Just "return std::max(SaturateToInt32(filesize), 0);".

@@ +173,5 @@
> +
> +  if (len < 0)
> +    return 0;
> +
> +  return len;

Replace the last four lines with "return 0" and maybe a comment about how this is the fallback case.
Attachment #720054 - Flags: review?(seth) → review+
http://hg.mozilla.org/integration/mozilla-inbound/rev/1f764bcd4e5d
Assignee: nobody → joe
Target Milestone: --- → mozilla22
https://hg.mozilla.org/mozilla-central/rev/1f764bcd4e5d
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.