Closed Bug 908031 Opened 6 years ago Closed 6 years ago

Remove nsGeolocation::PendingRequest

Categories

(Core :: DOM: Geolocation, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: kanru, Assigned: mjh563)

Details

(Whiteboard: [mentor=jdm][lang=c++])

Attachments

(1 file, 1 obsolete file)

We could use nsGeolocationRequest directly and use nsGeolocationRequest::IsWatch to find out the type of the request.
Yes, please.
Whiteboard: [mentor=jdm][lang=c++]
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.
Assignee: nobody → allamsetty.anup
Assignee: allamsetty.anup → nobody
Attached patch patch v1 (obsolete) — Splinter Review
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?
Assignee: nobody → mjh563
Status: NEW → ASSIGNED
Attachment #801193 - Flags: feedback?(josh)
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.
Attachment #801193 - Flags: feedback?(josh) → feedback+
Attached patch updated patchSplinter Review
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.
Attachment #801193 - Attachment is obsolete: true
Attachment #801276 - Flags: review?(josh)
Comment on attachment 801276 [details] [diff] [review]
updated patch

Mmm, lovely. Thanks!
Attachment #801276 - Flags: review?(josh) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/dbe8e0e7c250
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.