Note: There are a few cases of duplicates in user autocompletion which are being worked on.

nsScriptLoader should use nsRefPtr to hold nsScriptLoadRequest

RESOLVED FIXED in mozilla2.0b7

Status

()

Core
DOM
--
trivial
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: hsivonen, Assigned: hsivonen)

Tracking

Trunk
mozilla2.0b7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

nsScriptLoader should use nsRefPtr to hold nsScriptLoadRequest. It's currently using nsCOMPtr and nsCOMArray.
Created attachment 476220 [details] [diff] [review]
Make nsScriptLoader use nsRefPtr/nsTArray for holding nsScriptLoadRequest
Assignee: nobody → hsivonen
Status: NEW → ASSIGNED
Attachment #476220 - Flags: review?(jonas)
Comment on attachment 476220 [details] [diff] [review]
Make nsScriptLoader use nsRefPtr/nsTArray for holding nsScriptLoadRequest

>+  while (mEnabled && i < mAsyncRequests.Length()) {
>     if (!mAsyncRequests[i]->mLoading) {
>-      request = mAsyncRequests[i];
>-      mAsyncRequests.RemoveObjectAt(i);
>+      request.swap(mAsyncRequests[i]);
>+      mAsyncRequests.RemoveElementAt(i);
>       ProcessRequest(request);
>       continue;
>     }
>     ++i;
>   }

This is somewhat odd since you're actually swapping in the old requests into the array before you remove it. I think it works, but it would IMHO be more understandable to do

request = mAsyncRequests[i].forget();

Same in the loop below.

r=me with that fixed.

Thanks for doing this cleanup.
Attachment #476220 - Flags: review?(jonas) → review+
Comment on attachment 476220 [details] [diff] [review]
Make nsScriptLoader use nsRefPtr/nsTArray for holding nsScriptLoadRequest

Requesting approval: This is low-risk cleanup. Taking this on the branch would make branch maintanance easier if we end up having to patch nsScriptLoader.
Attachment #476220 - Flags: approval2.0?

Updated

7 years ago
Attachment #476220 - Flags: approval2.0? → approval2.0+
http://hg.mozilla.org/mozilla-central/rev/aacd84d91b66
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Target Milestone: mozilla2.0b8 → mozilla2.0b7
Depends on: 737754
You need to log in before you can comment on or make changes to this bug.