Commit 3f90035e authored by Harald Alvestrand's avatar Harald Alvestrand Committed by Commit Bot

Implement DTMF [[ToneBuffer]] in the blink layer

This CL makes the Blink layer keep a copy of the tone buffer
and update it on insertDTMF and ontonechange events only; this
makes it possible to expose the state of the tone buffer at the
time the event is fired to the Javascript callback.

It removes a queueing step inside the DTMF sender, because
that queueing step destroyed the consistency.

Bug: chromium:816475
Change-Id: I5aa68396299a67d6cea1e8a17d364f553514c291
Reviewed-on: https://chromium-review.googlesource.com/1213084Reviewed-by: default avatarGuido Urdaneta <guidou@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Commit-Queue: Harald Alvestrand <hta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589910}
parent 2e841144
...@@ -30,18 +30,20 @@ class RtcDtmfSenderHandler::Observer : ...@@ -30,18 +30,20 @@ class RtcDtmfSenderHandler::Observer :
~Observer() override {} ~Observer() override {}
void OnToneChange(const std::string& tone) override { void OnToneChange(const std::string& tone,
const std::string& tone_buffer) override {
main_thread_->PostTask( main_thread_->PostTask(
FROM_HERE, FROM_HERE,
base::BindOnce( base::BindOnce(
&RtcDtmfSenderHandler::Observer::OnToneChangeOnMainThread, this, &RtcDtmfSenderHandler::Observer::OnToneChangeOnMainThread, this,
tone)); tone, tone_buffer));
} }
void OnToneChangeOnMainThread(const std::string& tone) { void OnToneChangeOnMainThread(const std::string& tone,
const std::string& tone_buffer) {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(thread_checker_.CalledOnValidThread());
if (handler_) if (handler_)
handler_->OnToneChange(tone); handler_->OnToneChange(tone, tone_buffer);
} }
base::ThreadChecker thread_checker_; base::ThreadChecker thread_checker_;
...@@ -85,12 +87,14 @@ bool RtcDtmfSenderHandler::InsertDTMF(const blink::WebString& tones, ...@@ -85,12 +87,14 @@ bool RtcDtmfSenderHandler::InsertDTMF(const blink::WebString& tones,
static_cast<int>(interToneGap)); static_cast<int>(interToneGap));
} }
void RtcDtmfSenderHandler::OnToneChange(const std::string& tone) { void RtcDtmfSenderHandler::OnToneChange(const std::string& tone,
const std::string& tone_buffer) {
if (!webkit_client_) { if (!webkit_client_) {
LOG(ERROR) << "WebRTCDTMFSenderHandlerClient not set."; LOG(ERROR) << "WebRTCDTMFSenderHandlerClient not set.";
return; return;
} }
webkit_client_->DidPlayTone(blink::WebString::FromUTF8(tone)); webkit_client_->DidPlayTone(blink::WebString::FromUTF8(tone),
blink::WebString::FromUTF8(tone_buffer));
} }
} // namespace content } // namespace content
...@@ -38,7 +38,7 @@ class CONTENT_EXPORT RtcDtmfSenderHandler ...@@ -38,7 +38,7 @@ class CONTENT_EXPORT RtcDtmfSenderHandler
long duration, long duration,
long interToneGap) override; long interToneGap) override;
void OnToneChange(const std::string& tone); void OnToneChange(const std::string& tone, const std::string& tone_buffer);
private: private:
scoped_refptr<webrtc::DtmfSenderInterface> dtmf_sender_; scoped_refptr<webrtc::DtmfSenderInterface> dtmf_sender_;
......
...@@ -4605,8 +4605,6 @@ crbug.com/806249 virtual/video-surface-layer/external/wpt/html/semantics/embedde ...@@ -4605,8 +4605,6 @@ crbug.com/806249 virtual/video-surface-layer/external/wpt/html/semantics/embedde
crbug.com/806249 virtual/video-surface-layer/external/wpt/html/semantics/embedded-content/media-elements/autoplay-default-feature-policy.https.sub.html [ Skip ] crbug.com/806249 virtual/video-surface-layer/external/wpt/html/semantics/embedded-content/media-elements/autoplay-default-feature-policy.https.sub.html [ Skip ]
crbug.com/806249 virtual/video-surface-layer/external/wpt/html/semantics/embedded-content/media-elements/autoplay-disabled-by-feature-policy.https.sub.html [ Skip ] crbug.com/806249 virtual/video-surface-layer/external/wpt/html/semantics/embedded-content/media-elements/autoplay-disabled-by-feature-policy.https.sub.html [ Skip ]
# Sheriff 2018-02-26
crbug.com/816475 [ Win7 Linux ] external/wpt/webrtc/RTCDTMFSender-ontonechange.https.html [ Failure Pass ]
crbug.com/816914 [ Mac ] fast/canvas/canvas-drawImage-live-video.html [ Failure Pass ] crbug.com/816914 [ Mac ] fast/canvas/canvas-drawImage-live-video.html [ Failure Pass ]
crbug.com/817167 http/tests/devtools/oopif/oopif-cookies-refresh.js [ Failure Timeout Pass ] crbug.com/817167 http/tests/devtools/oopif/oopif-cookies-refresh.js [ Failure Timeout Pass ]
...@@ -4700,8 +4698,6 @@ crbug.com/840881 virtual/service-worker-servicification/external/wpt/service-wor ...@@ -4700,8 +4698,6 @@ crbug.com/840881 virtual/service-worker-servicification/external/wpt/service-wor
crbug.com/831993 [ Linux ] virtual/gpu-rasterization/images/cross-fade-overflow-position.html [ Pass Timeout ] crbug.com/831993 [ Linux ] virtual/gpu-rasterization/images/cross-fade-overflow-position.html [ Pass Timeout ]
# Sheriff 2018-04-13 # Sheriff 2018-04-13
crbug.com/832842 [ Win7 Linux ] virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/RTCDTMFSender-ontonechange.https.html [ Failure ]
crbug.com/832842 [ Mac ] virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/RTCDTMFSender-ontonechange.https.html [ Failure Pass ]
crbug.com/833655 [ Linux ] media/controls/closed-captions-dynamic-update.html [ Skip ] crbug.com/833655 [ Linux ] media/controls/closed-captions-dynamic-update.html [ Skip ]
crbug.com/833655 [ Linux ] virtual/new-remote-playback-pipeline/media/controls/closed-captions-dynamic-update.html [ Skip ] crbug.com/833655 [ Linux ] virtual/new-remote-playback-pipeline/media/controls/closed-captions-dynamic-update.html [ Skip ]
crbug.com/833655 [ Linux ] virtual/video-surface-layer/media/controls/closed-captions-dynamic-update.html [ Skip ] crbug.com/833655 [ Linux ] virtual/video-surface-layer/media/controls/closed-captions-dynamic-update.html [ Skip ]
......
...@@ -109,7 +109,7 @@ function test_tone_change_events(testFunc, toneChanges, desc) { ...@@ -109,7 +109,7 @@ function test_tone_change_events(testFunc, toneChanges, desc) {
const now = Date.now(); const now = Date.now();
const duration = now - lastEventTime; const duration = now - lastEventTime;
assert_approx_equals(duration, expectedDuration, 250, assert_approx_equals(duration, expectedDuration, 400,
`Expect tonechange event for "${tone}" to be fired approximately after ${expectedDuration} milliseconds`); `Expect tonechange event for "${tone}" to be fired approximately after ${expectedDuration} milliseconds`);
lastEventTime = now; lastEventTime = now;
......
...@@ -36,7 +36,8 @@ class WebRTCDTMFSenderHandlerClient { ...@@ -36,7 +36,8 @@ class WebRTCDTMFSenderHandlerClient {
public: public:
virtual ~WebRTCDTMFSenderHandlerClient() = default; virtual ~WebRTCDTMFSenderHandlerClient() = default;
virtual void DidPlayTone(const WebString& tone) = 0; virtual void DidPlayTone(const WebString& tone,
const WebString& tone_buffer) = 0;
}; };
} // namespace blink } // namespace blink
......
...@@ -58,10 +58,7 @@ RTCDTMFSender::RTCDTMFSender(ExecutionContext* context, ...@@ -58,10 +58,7 @@ RTCDTMFSender::RTCDTMFSender(ExecutionContext* context,
std::unique_ptr<WebRTCDTMFSenderHandler> handler) std::unique_ptr<WebRTCDTMFSenderHandler> handler)
: ContextLifecycleObserver(context), : ContextLifecycleObserver(context),
handler_(std::move(handler)), handler_(std::move(handler)),
stopped_(false), stopped_(false) {
scheduled_event_timer_(context->GetTaskRunner(TaskType::kNetworking),
this,
&RTCDTMFSender::ScheduledEventTimerFired) {
handler_->SetClient(this); handler_->SetClient(this);
} }
...@@ -79,7 +76,7 @@ bool RTCDTMFSender::canInsertDTMF() const { ...@@ -79,7 +76,7 @@ bool RTCDTMFSender::canInsertDTMF() const {
} }
String RTCDTMFSender::toneBuffer() const { String RTCDTMFSender::toneBuffer() const {
return handler_->CurrentToneBuffer(); return tone_buffer_;
} }
void RTCDTMFSender::insertDTMF(const String& tones, void RTCDTMFSender::insertDTMF(const String& tones,
...@@ -129,10 +126,14 @@ void RTCDTMFSender::insertDTMF(const String& tones, ...@@ -129,10 +126,14 @@ void RTCDTMFSender::insertDTMF(const String& tones,
"Could not send provided tones, '" + tones + "'."); "Could not send provided tones, '" + tones + "'.");
return; return;
} }
tone_buffer_ = handler_->CurrentToneBuffer();
} }
void RTCDTMFSender::DidPlayTone(const WebString& tone) { void RTCDTMFSender::DidPlayTone(const WebString& tone,
ScheduleDispatchEvent(RTCDTMFToneChangeEvent::Create(tone)); const WebString& tone_buffer) {
tone_buffer_ = tone_buffer;
Member<Event> event = RTCDTMFToneChangeEvent::Create(tone);
DispatchEvent(*event.Release());
} }
const AtomicString& RTCDTMFSender::InterfaceName() const { const AtomicString& RTCDTMFSender::InterfaceName() const {
...@@ -148,27 +149,7 @@ void RTCDTMFSender::ContextDestroyed(ExecutionContext*) { ...@@ -148,27 +149,7 @@ void RTCDTMFSender::ContextDestroyed(ExecutionContext*) {
handler_->SetClient(nullptr); handler_->SetClient(nullptr);
} }
void RTCDTMFSender::ScheduleDispatchEvent(Event* event) {
scheduled_events_.push_back(event);
if (!scheduled_event_timer_.IsActive())
scheduled_event_timer_.StartOneShot(TimeDelta(), FROM_HERE);
}
void RTCDTMFSender::ScheduledEventTimerFired(TimerBase*) {
if (stopped_)
return;
HeapVector<Member<Event>> events;
events.swap(scheduled_events_);
HeapVector<Member<Event>>::iterator it = events.begin();
for (; it != events.end(); ++it)
DispatchEvent(*it->Release());
}
void RTCDTMFSender::Trace(blink::Visitor* visitor) { void RTCDTMFSender::Trace(blink::Visitor* visitor) {
visitor->Trace(scheduled_events_);
EventTargetWithInlineData::Trace(visitor); EventTargetWithInlineData::Trace(visitor);
ContextLifecycleObserver::Trace(visitor); ContextLifecycleObserver::Trace(visitor);
} }
......
...@@ -75,18 +75,14 @@ class RTCDTMFSender final : public EventTargetWithInlineData, ...@@ -75,18 +75,14 @@ class RTCDTMFSender final : public EventTargetWithInlineData,
std::unique_ptr<WebRTCDTMFSenderHandler>); std::unique_ptr<WebRTCDTMFSenderHandler>);
void Dispose(); void Dispose();
void ScheduleDispatchEvent(Event*);
void ScheduledEventTimerFired(TimerBase*);
// WebRTCDTMFSenderHandlerClient // WebRTCDTMFSenderHandlerClient
void DidPlayTone(const WebString&) override; void DidPlayTone(const WebString& tone,
const WebString& tone_buffer) override;
std::unique_ptr<WebRTCDTMFSenderHandler> handler_; std::unique_ptr<WebRTCDTMFSenderHandler> handler_;
bool stopped_; bool stopped_;
String tone_buffer_;
TaskRunnerTimer<RTCDTMFSender> scheduled_event_timer_;
HeapVector<Member<Event>> scheduled_events_;
}; };
} // namespace blink } // namespace blink
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment