[webkit-reviews] review denied: [Bug 30935] Simplify LocalStorageThread : [Attachment 42161] Patch v1

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Oct 29 17:27:52 PDT 2009


Darin Adler <darin at apple.com> has denied Jeremy Orlow <jorlow at chromium.org>'s
request for review:
Bug 30935: Simplify LocalStorageThread
https://bugs.webkit.org/show_bug.cgi?id=30935

Attachment 42161: Patch v1
https://bugs.webkit.org/attachment.cgi?id=42161&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +    // Even in weird, exceptional cases, don't wait on a non-existant thread
to terminate.

Spelling error here. It's "nonexistent" rather than "non-existant".

> +    void *returnValue;

WebKit coding style is void* for this, not void *.

>      // FIXME: Rename this class to StorageThread
> -    class LocalStorageThread : public ThreadSafeShared<LocalStorageThread> {

> +    class LocalStorageThread {

Should derive from Noncopyable for multiple reasons. One is that it will make
this derive from FastAllocBase and save some work on the FastAllocBase.

By the way, you could still have made this have a create function and a private
constructor, just using PassOwnPtr instead of PassRefPtr. The benefit would be
that we'd avoid having a "bare new" outside a create function. Might be good
long term to have that style.

Otherwise, looks good. I'm going to say review- because I think the Noncopyable
is fairly important.


More information about the webkit-reviews mailing list