[Webkit-unassigned] [Bug 44890] Add AudioContext files
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Sep 22 17:51:06 PDT 2010
https://bugs.webkit.org/show_bug.cgi?id=44890
Kenneth Russell <kbr at google.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #68279|review? |review+
Flag| |
--- Comment #15 from Kenneth Russell <kbr at google.com> 2010-09-22 17:51:06 PST ---
(From update of attachment 68279)
View in context: https://bugs.webkit.org/attachment.cgi?id=68279&action=review
We've gone through the locking semantics of this class as well as AudioNode at length offline, and at this point I think this looks great. Marking r+; a couple of minor issues that aren't really necessary before commit, so leave them up to you.
> WebCore/webaudio/AudioContext.cpp:60
> +const int UndefinedThreadIdentifier = 0xffffffff;
>From looking at ThreadingPthreads.cpp and ThreadingWin.cpp, I think that 0 is the correct value to use here. Regardless it would be good to define that explicitly in WTF.
> WebCore/webaudio/AudioContext.cpp:114
> + printf("%p: AudioContext::~AudioContext()\n", this);
Should use LOG_ERROR or similar.
> WebCore/webaudio/AudioContext.cpp:352
> + m_graphOwnerThread = thisThread;
This could be placed in the "else" clause to avoid mutating this member over and over; not a big deal.
> WebCore/webaudio/AudioContext.cpp:437
> + ASSERT(isAudioThread());
Could use an ASSERT(isGraphOwner()) too.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list