908031 - Remove nsGeolocation::PendingRequest

Status: RESOLVED FIXED

(Core :: DOM: Geolocation, defect)

Description

[Kan-Ru Chen [:kanru] (UTC+9)]

We could use nsGeolocationRequest directly and use nsGeolocationRequest::IsWatch to find out the type of the request.

Comment 1

[Josh Matthews [:jdm]]

Yes, please.

Comment 2

[Anup Kumar [:reznord]]

Hi, 

I am willing to work on this bug. So, please I request you to assign me this bug.

Thanks in Advance,

Regards,
A.Anup.

Comment 3

[mjh563]

Attachment: patch v1

Here's my attempted fix for this bug. Hope I've understood it correctly...

I'm not sure about this line though, I have a feeling it's not right:
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPendingRequests[i])

I'm not at all familiar with how cycle collection works, but since mPendingRequests is now the same type as mPendingCallbacks and mWatchingCallbacks (ie. nsTArray<nsRefPtr<nsGeolocationRequest> >), I guess the cycle collection for mPendingRequests should be done in the same way?

Comment 4

[Josh Matthews [:jdm]]

Comment on attachment 801193 [details] [diff] [review]
patch v1

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

This looks great, and your hunch is right. The cycle collection traversal is still be correct as written, but it would be more maintainable to let it deal with the array like the others rather than manually iterating through it.

Comment 5

[mjh563]

Attachment: updated patch

Updated patch to fix the cycle collection macros. Ms2ger suggested on IRC to use NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE_4, so that's what I've done.

Comment 6

[Josh Matthews [:jdm]]

Comment on attachment 801276 [details] [diff] [review]
updated patch

Mmm, lovely. Thanks!

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dbe8e0e7c250

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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