[webkit-reviews] review granted: [Bug 22380] Fix WorkerContext refcounting : [Attachment 25311] proposed patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Nov 20 09:24:59 PST 2008
Darin Adler <darin at apple.com> has granted Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 22380: Fix WorkerContext refcounting
https://bugs.webkit.org/show_bug.cgi?id=22380
Attachment 25311: proposed patch
https://bugs.webkit.org/attachment.cgi?id=25311&action=review
------- Additional Comments from Darin Adler <darin at apple.com>
> + , m_script(new WorkerScriptController(this))
For future consideration: We want to try to avoid bare "new" calls almost
everywhere. For single-owner classes, I think a create function that returns
auto_ptr is a good design. We can change OwnPtr so it has a constructor that
takes an auto_ptr, if it doesn't already have one. For reference counted
classes a create function that returns PassRefPtr is obviously our approach.
> + workerContext->clearScript();
> + ASSERT(workerContext->refCount() == 1);
I think you should add a comment here explaining why there's a guarantee that
no one else is holding a reference. Our usual aim with reference-counted
objects is to make the "shutdown" operation be something other than
destruction, and always allow someone to prolong the lifetime of the object a
bit, just to keep a pointer valid. I know we don't always use reference
counting that way, but I think it merits a comment when we're not.
By the way, can you use the hasOneRef() function here instead?
r=me
More information about the webkit-reviews
mailing list