[webkit-reviews] review denied: [Bug 67866] HRTFDatabaseLoader should not call WTF::waitForThreadCompletion() more than once : [Attachment 106926] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Sep 9 15:21:33 PDT 2011

David Levin <levin at chromium.org> has denied Chris Rogers <crogers at google.com>'s
request for review:
Bug 67866: HRTFDatabaseLoader should not call WTF::waitForThreadCompletion()
more than once

Attachment 106926: Patch

------- Additional Comments from David Levin <levin at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=106926&action=review

> Source/WebCore/platform/audio/HRTFDatabaseLoader.cpp:127
> +    // WTF::waitForThreadCompletion() should not be called twice for the
same thread.

It feels odd to have the WTF:: prefix here but not in the code. It takes me a
little more time to pair the two. (I'd just leave it off in the comment.)

> Source/WebCore/platform/audio/HRTFDatabaseLoader.h:84
> +    Mutex m_threadLock;

Ideally there would be some comment like what is here:

I think it is only guarding  m_databaseLoaderThread but in one instance in the
code, it is also guarding "m_startedLoadingDatabase = true;" which I don't
think is needed (and is confusing because that variable isn't guarded when used

Why do we even have m_startedLoadingDatabase when it seems like the value of
m_databaseLoaderThread serves the same purpose?

More information about the webkit-reviews mailing list