Closed Bug 1312144 Opened 5 years ago Closed 5 years ago

Remove nsISupportsArray usage from xpfe

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: erahm, Assigned: erahm)

References

Details

Attachments

(2 files)

nsISupportsArray is deprecated. We shoudl switch usage in xpfe over to nsIMutableArray. Filing in xpcom as we're just switching types and I couldn't find an xpfe component.

MozReview-Commit-ID: JT585Zg4iJY
Attachment #8803577 - Flags: review?(nfroyd)
This swaps out nsIMutableArray for nsISupportsArray.

MozReview-Commit-ID: 9iZynpMcq6A
Attachment #8803578 - Flags: review?(nfroyd)
Comment on attachment 8803577 [details] [diff] [review]
Part 0: Cleanup formatting

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

rs=me; the diff is pretty unreadable. :(

::: xpfe/components/directory/nsDirectoryViewer.cpp
@@ +911,5 @@
>  nsHTTPIndex::FireTimer(nsITimer* aTimer, void* aClosure)
>  {
>    nsHTTPIndex *httpIndex = static_cast<nsHTTPIndex *>(aClosure);
> +  if (!httpIndex)
> +    return;

If we're going to change it, we might as well add braces.  Or are you trying to stick with local style here?
Attachment #8803577 - Flags: review?(nfroyd) → review+
Comment on attachment 8803578 [details] [diff] [review]
Part 1: Stop using nsISupportsArray in nsDirectoryViewer

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

I feel like these should really be nsCOMArray<nsIRDFResource>, but whatever.
Attachment #8803578 - Flags: review?(nfroyd) → review+
(In reply to Nathan Froyd [:froydnj] from comment #3)
> Comment on attachment 8803577 [details] [diff] [review]
> Part 0: Cleanup formatting
> 
> Review of attachment 8803577 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> rs=me; the diff is pretty unreadable. :(
> 
> ::: xpfe/components/directory/nsDirectoryViewer.cpp
> @@ +911,5 @@
> >  nsHTTPIndex::FireTimer(nsITimer* aTimer, void* aClosure)
> >  {
> >    nsHTTPIndex *httpIndex = static_cast<nsHTTPIndex *>(aClosure);
> > +  if (!httpIndex)
> > +    return;
> 
> If we're going to change it, we might as well add braces.  Or are you trying
> to stick with local style here?

More or less sticking with the style, the cleanup was basically highlighting to function in vim and hitting '=' (reindent selection). Then I broke the ifs onto newlines.
(In reply to Nathan Froyd [:froydnj] from comment #4)
> Comment on attachment 8803578 [details] [diff] [review]
> Part 1: Stop using nsISupportsArray in nsDirectoryViewer
> 
> Review of attachment 8803578 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I feel like these should really be nsCOMArray<nsIRDFResource>, but whatever.

My sentiments exactly. I don't think it's worth the effort to change this code too much (and there's logic that seems to count on the array itself being nullptr, so it'd have to be a nsCOMArray*).
https://hg.mozilla.org/mozilla-central/rev/70280cfa7c11
https://hg.mozilla.org/mozilla-central/rev/3eb7745dd704
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.