[webkit-reviews] review granted: [Bug 133298] Add mock DTMFSender support : [Attachment 232282] Implemented mock RTCDTMFSender

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Kiran
<kiran.guduru at samsung.com>'s request for review:
Bug 133298: Add mock DTMFSender support
https://bugs.webkit.org/show_bug.cgi?id=133298

Attachment 232282: Implemented mock RTCDTMFSender
https://bugs.webkit.org/attachment.cgi?id=232282&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=232282&action=review


> Source/WebCore/platform/mock/RTCDTMFSenderHandlerMock.cpp:55
> +	   while (m_toneBuffer.length() > 0) {
> +	       m_client->didPlayTone(m_toneBuffer.substring(0, 1));
> +	       m_toneBuffer.remove(0, 1);
> +	   }

This is an extremely inefficient way to do this. If this processing was done
asynchronously it would be necessary to modify m_toneBuffer each time through
the loop, but nothing can access m_toneBuffer while this runs so something like
this would be better:

    for (size_t i = 0; i < m_toneBuffer.length(); ++i)
	m_client->didPlayTone(m_toneBuffer[i]);
    m_toneBuffer = emptyString();
    m_client->didPlayTone(emptyString());

> Source/WebCore/platform/mock/RTCDTMFSenderHandlerMock.cpp:56
> +	   if (m_toneBuffer.isEmpty())

Why this test, m_toneBuffer is empty because you just emptied it.