Closed Bug 819936 Opened 7 years ago Closed 7 years ago

remove overload of NS_NewArrayEnumerator() for nsISupportsArray

Categories

(Core :: XPCOM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla20

People

(Reporter: tbsaunde, Assigned: tbsaunde)

References

Details

Attachments

(1 file)

No description provided.
Comment on attachment 690366 [details] [diff] [review]
remove NS_NewArrayEnumerator overload for nsISupportsArray

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

Sorry, I don't know much about the nsEnumeratorUtils.h enumerators...  Can you please ask someone else review this patch?  Thanks!
Attachment #690366 - Flags: review?(ehsan)
Comment on attachment 690366 [details] [diff] [review]
remove NS_NewArrayEnumerator overload for nsISupportsArray

whoever can avoid reviews for less time, or make the mistake of admitting to know this stuff is it :)
Attachment #690366 - Flags: review?(neil)
Attachment #690366 - Flags: review?(bzbarsky)
Attachment #690366 - Flags: review?(benjamin)
This is pretty low on my list for the next week or so, honestly.
(In reply to Boris Zbarsky (:bz) from comment #4)
> This is pretty low on my list for the next week or so, honestly.

that sounds like about the right place for it, especially considering that it doesn't really block anything, and is fairly unlikely to be bit rotted.
Comment on attachment 690366 [details] [diff] [review]
remove NS_NewArrayEnumerator overload for nsISupportsArray

Have you run this though try?

>     RequestMapEntry *e = static_cast<RequestMapEntry *>(hdr);
>+ static_cast<nsCOMArray<nsIRequest>*>(arg)->AppendObject(e->mKey);
Nit: wrong indentation.

>+    nsCOMArray<nsIRequest> requests;
>+    if (!requests.SetCapacity(mRequests.entryCount)) {
Just use nsCOMArray<nsIRequest> requests(mRequests.entryCount);
[This pattern happens several times in the patch, in once case with the wrong indentation.]

>         if (NS_SUCCEEDED(rv))
>         {
>+        nsCOMPtr<nsISimpleEnumerator> tmp;
Nit: wrong indentation.

>+            rv = NS_NewUnionEnumerator(getter_AddRefs(tmp), set, dsCmds);
>+            set = tmp;
Nit: use swap

>+  nsCOMPtr<nsISimpleEnumerator> child, anonArcs;
> 	if (isWellknownContainerURI(aSource))
> 	{
>-		array->AppendElement(kNC_Child);
>+		NS_NewSingletonEnumerator(getter_AddRefs(child), kNC_Child);
Bah, sometimes you kept the tabs, sometimes you converted to spaces...
(In reply to neil@parkwaycc.co.uk from comment #6)
> Comment on attachment 690366 [details] [diff] [review]
> remove NS_NewArrayEnumerator overload for nsISupportsArray
> 
> Have you run this though try?

maybe, I forget if it was in one of the bunches of patches I pushed or not.

> >+    nsCOMArray<nsIRequest> requests;
> >+    if (!requests.SetCapacity(mRequests.entryCount)) {
> Just use nsCOMArray<nsIRequest> requests(mRequests.entryCount);
> [This pattern happens several times in the patch, in once case with the
> wrong indentation.]

ok, sorry about the whitespace issues :(

> >+            rv = NS_NewUnionEnumerator(getter_AddRefs(tmp), set, dsCmds);
> >+            set = tmp;
> Nit: use swap
> 
> >+  nsCOMPtr<nsISimpleEnumerator> child, anonArcs;
> > 	if (isWellknownContainerURI(aSource))
> > 	{
> >-		array->AppendElement(kNC_Child);
> >+		NS_NewSingletonEnumerator(getter_AddRefs(child), kNC_Child);
> Bah, sometimes you kept the tabs, sometimes you converted to spaces...

s/me/vim on my behalf/ suggestions on tools to check / vim config stuff to make whitespace correct more of the time greatfully accepted.  Whitespace doesn't effect my reading of it at all so while I try and check its correct it gets pretty tedious.
Comment on attachment 690366 [details] [diff] [review]
remove NS_NewArrayEnumerator overload for nsISupportsArray

>+++ b/netwerk/base/src/nsLoadGroup.cpp
>+AppendRequestsToCOMArray(PLDHashTable *table, PLDHashEntryHdr *hdr,
>+ static_cast<nsCOMArray<nsIRequest>*>(arg)->AppendObject(e->mKey);

Fix the indent, please.

>+++ b/rdf/base/src/nsCompositeDataSource.cpp
> CompositeDataSourceImpl::GetAllCmds(nsIRDFResource* source,
>+        nsCOMPtr<nsISimpleEnumerator> tmp;

And here.

>+++ b/xpfe/components/directory/nsDirectoryViewer.cpp
> nsHTTPIndex::ArcLabelsOut(nsIRDFResource *aSource, nsISimpleEnumerator **_retval)
>+  nsCOMPtr<nsISimpleEnumerator> child, anonArcs;

And here.

r=me with those fixed.
Attachment #690366 - Flags: review?(bzbarsky) → review+
Attachment #690366 - Flags: review?(neil)
Attachment #690366 - Flags: review?(benjamin)
Attachment #690366 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/08e6e5bc8b5f
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
This left a straggling unused "nsresult rv;" in InMemoryDataSource::GetAllResources(), here:
  https://hg.mozilla.org/mozilla-central/rev/08e6e5bc8b5f#l3.39

...so I pushed a trivial followup to remove it:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/a6ec2ed06504
Depends on: 843363
You need to log in before you can comment on or make changes to this bug.