Closed Bug 870125 Opened 13 years ago Closed 13 years ago

Add a stop method to DOMCursor

Categories

(Core :: DOM: Device Interfaces, defect)

Product:
Core
Component:
DOM: Device Interfaces
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED WONTFIX

People

(Reporter: reuben, Unassigned)

Details

We need a stop/cancel/abort/something method in DOMCursor to signal to the API that content is not going to call continue() on that cursor anymore. For example, if you change the contacts sorting in the Contacts app, it'll create a new request with the new sorting options, but we'll keep some data around because we don't know if content is going to use the cursor. Mounir, does this look OK?
Flags: needinfo?(mounir)
We talked with some google folks a few days ago and this issue came up with indexedDB cursors. We all basically hated the idea of a stop/close method ;)
(In reply to ben turner [:bent] from comment #1) > We talked with some google folks a few days ago and this issue came up with > indexedDB cursors. We all basically hated the idea of a stop/close method ;) So what's the solution? How long should we keep the cursor and cached objects around?
(In reply to ben turner [:bent] from comment #1) > We talked with some google folks a few days ago and this issue came up with > indexedDB cursors. We all basically hated the idea of a stop/close method ;) Yep, I don't like it either. I was wondering if we could see the cursor being GC'd and use that, but apparently that's hard to do in Gecko.
I would tend to say that .stop() is fine mostly because I do not believe we are intending to keep DOMCursor forever. However, this might not solve your problem because developers might still not call .stop() and simply stop using the cursor and you never know if the cursor will be re-used later. I would be fine with a system that tries to be right most of the time and simply send an error event if things go too bad and trying to be right would just be too painful. Mostly because, again, we do not intend to keep those interfaces forever as far as I know.
Flags: needinfo?(mounir)
(In reply to Gregor Wagner [:gwagner] from comment #2) > So what's the solution? How long should we keep the cursor and cached > objects around? There isn't really one. If you rely on a close method then you've already sort of lost because developers could forget to call it. On the other hand it sucks to hold things longer than we need to... We can know when the transaction closes, right? Can we just assume that the cursor can't be used after that? Is that good enough?
(In reply to ben turner [:bent] from comment #5) > (In reply to Gregor Wagner [:gwagner] from comment #2) > > So what's the solution? How long should we keep the cursor and cached > > objects around? > > We can know when the transaction closes, right? Can we just assume that the > cursor can't be used after that? Is that good enough? That could work for IndexedDB, but Contacts doesn't expose transactions.
Maybe just do a timeout and clean-up everything after 2 min without a continue call?
I think it's fine to add a .stop() method to DOMCursor. But we certainly can't rely on that authors will call it. I think the "simplest" solution is to have the child process send a message to the parent process once we have cached, say, 20 items ahead. At that point the parent process can stop producing results. However at a later point the child process needs to be able to send a message to the parent again saying "please start sending results again, I'm below 20". So definitely not trivial.
(In reply to Jonas Sicking (:sicking) from comment #8) > I think it's fine to add a .stop() method to DOMCursor. But we certainly > can't rely on that authors will call it. > > I think the "simplest" solution is to have the child process send a message > to the parent process once we have cached, say, 20 items ahead. At that > point the parent process can stop producing results. However at a later > point the child process needs to be able to send a message to the parent > again saying "please start sending results again, I'm below 20". > > So definitely not trivial. That's what we do now, so it's actually quite simple, at least for Contacts :)
If that's what we're already doing, then I don't think it's important that we do anything more here. We could add a .stop() function which immediately sent a "stop sending me any more results" message to the parent process and then causes any more calls to .continue() to throw. However it doesn't seem like that will save us any significant amounts of resources, since it sounds like we're already not spending any significant amount of resources as soon as the page stops reading.
We're not spending time sending stuff to the child, but we do keep the cache around forever if it doesn't call continue again. stop would then throw away the cache in the child and the parent.
Only until the object is GC'ed. Presumably if the application isn't planning on calling .continue() any more, it'll drop all references to the cursor. Unless we have data showing that the cached contact entries actually add up to a problematic amount of memory, I don't think we should worry about it.
(In reply to Jonas Sicking (:sicking) from comment #12) > Only until the object is GC'ed. Presumably if the application isn't planning > on calling .continue() any more, it'll drop all references to the cursor. We keep the cache in ContactManager, not in the cursor, and there's no easy way to get rid of it when the cursor gets destroyed. > Unless we have data showing that the cached contact entries actually add up > to a problematic amount of memory, I don't think we should worry about it. It hasn't showed up yet, and since stop() would only marginally solve this problem, this is wontfix for now.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → WONTFIX
Oh, we should definitely ensure that we free resources when the cursor is GCed. You can possibly add an callback that's called when that happens.
You need to log in before you can comment on or make changes to this bug.