Move delay_based_bwe_ into CongestionController

Move delay_based_bwe_ into CongestionController

BUG=webrtc:7058
Review-Url: https://codereview.webrtc.org/2725823002
Cr-Commit-Position: refs/heads/master@{#17146}
Committed: https://chromium.googlesource.com/external/webrtc/+/5bbf43f9d480f89fa57bbdbff0b028a413202695

[elad.alon_webrtc.org, 2017-03-01 10:25:00]

Description was changed from

==========
Move delay_based_bwe_ into CongestionController

BUG=
==========

to

==========
Move delay_based_bwe_ into CongestionController

BUG=
==========

[elad.alon_webrtc.org, 2017-03-01 10:25:01]

elad.alon@webrtc.org changed reviewers:
+ michaelt@webrtc.org, minyue@webrtc.org, ossu@webrtc.org, solenberg@webrtc.org,
sprang@webrtc.org, stefan@webrtc.org

[elad.alon_webrtc.org, 2017-03-02 11:48:55]

Description was changed from

==========
Move delay_based_bwe_ into CongestionController

BUG=
==========

to

==========
Move delay_based_bwe_ into CongestionController

BUG=None
==========

[elad.alon_webrtc.org, 2017-03-02 11:51:47]

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
File webrtc/modules/congestion_controller/congestion_controller_unittest.cc
(right):

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/congestion_controller_unittest.cc:42: //
TODO(elad.alon): This is not meant to be landed like this. Ask reviewers
Soliciting feedback. :-)

[stefan-webrtc, 2017-03-02 15:58:07]

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
File webrtc/modules/congestion_controller/congestion_controller_unittest.cc
(right):

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/congestion_controller_unittest.cc:106:
class MockBitrateControllerAdapter : public MockBitrateController {
I would prefer not to have this mock, and to not expose the bitrate controller
in the constructor at all.

It should be possible to write the test so that we can see that the delay-based
bwe does what it should without this. For instance, insert packets at a fixed
rate that is as now and verify that the target bitrate is high enough. Then
insert packets where the rate is lower, and where the delay is growing, and
verify that the target bitrate has decreased.

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/congestion_controller_unittest.cc:337: }
Keep this loop, and also add a similar after:

while (clock_.TimeInMilliseconds() - start_time_ms < kRunTimeMs) {
  // insert late packets by increasing the arrival time more than the send time.
}

Then verified decraesed bitrate.

[minyue-webrtc, 2017-03-02 19:53:59]

Very good in general. Some questions, some of which may be your planned future
work, but I do not mind making it happen in this CL (if not big)

I have not looked at the UT carefully. will do later.

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/audio/audio_send_s...
File webrtc/audio/audio_send_stream.cc (right):

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/audio/audio_send_s...
webrtc/audio/audio_send_stream.cc:89: congestion_controller->pacer(),
congestion_controller, packet_router,
should the 2nd arg be an FeedbackAdaptor?

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
File webrtc/modules/congestion_controller/include/congestion_controller.h
(right):

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/include/congestion_controller.h:175: Clock*
const clock_;
if you can be nice, add "const"

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/include/congestion_controller.h:176:
Observer* const observer_;
here too

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/include/congestion_controller.h:177:
RtcEventLog* const event_log_;
const RtcEventLog* const

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/include/congestion_controller.h:178:
PacketRouter* const packet_router_;
here too

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/include/congestion_controller.h:188:
rtc::CriticalSection critsect_;
should we rename this to differentiate from bwe_lock_

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
File webrtc/modules/congestion_controller/transport_feedback_adapter.h (right):

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/transport_feedback_adapter.h:53: Clock*
const clock_;
const

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/transport_feedback_adapter.h:56:
std::vector<PacketFeedback> last_packet_feedback_vector_;
where is the list<FeedbackObserver>

[minyue-webrtc, 2017-03-02 19:55:50]

On 2017/03/02 19:53:59, minyue-webrtc wrote:
> Very good in general. Some questions, some of which may be your planned future
> work, but I do not mind making it happen in this CL (if not big)
> 
> I have not looked at the UT carefully. will do later.
> 
>
https://codereview.webrtc.org/2725823002/diff/40001/webrtc/audio/audio_send_s...
> File webrtc/audio/audio_send_stream.cc (right):
> 
>
https://codereview.webrtc.org/2725823002/diff/40001/webrtc/audio/audio_send_s...
> webrtc/audio/audio_send_stream.cc:89: congestion_controller->pacer(),
> congestion_controller, packet_router,
> should the 2nd arg be an FeedbackAdaptor?
> 
>
https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
> File webrtc/modules/congestion_controller/include/congestion_controller.h
> (right):
> 
>
https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
> webrtc/modules/congestion_controller/include/congestion_controller.h:175:
Clock*
> const clock_;
> if you can be nice, add "const"
> 
>
https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
> webrtc/modules/congestion_controller/include/congestion_controller.h:176:
> Observer* const observer_;
> here too
> 
>
https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
> webrtc/modules/congestion_controller/include/congestion_controller.h:177:
> RtcEventLog* const event_log_;
> const RtcEventLog* const
> 
>
https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
> webrtc/modules/congestion_controller/include/congestion_controller.h:178:
> PacketRouter* const packet_router_;
> here too
> 
>
https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
> webrtc/modules/congestion_controller/include/congestion_controller.h:188:
> rtc::CriticalSection critsect_;
> should we rename this to differentiate from bwe_lock_
> 
>
https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
> File webrtc/modules/congestion_controller/transport_feedback_adapter.h
(right):
> 
>
https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
> webrtc/modules/congestion_controller/transport_feedback_adapter.h:53: Clock*
> const clock_;
> const
> 
>
https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
> webrtc/modules/congestion_controller/transport_feedback_adapter.h:56:
> std::vector<PacketFeedback> last_packet_feedback_vector_;
> where is the list<FeedbackObserver>

btw, if you want to make the wiring of the adaptor in another CL, please upload
and let us see it asap.

[elad.alon_webrtc.org, 2017-03-08 18:25:09]

elad.alon@webrtc.org changed reviewers:
+ terelius@webrtc.org

[elad.alon_webrtc.org, 2017-03-08 18:25:10]

Ready for review (except for the analyzer; I'll fix and ping Björn Terelius
separately).

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/audio/audio_send_s...
File webrtc/audio/audio_send_stream.cc (right):

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/audio/audio_send_s...
webrtc/audio/audio_send_stream.cc:89: congestion_controller->pacer(),
congestion_controller, packet_router,
On 2017/03/02 19:53:58, minyue-webrtc(OOO_until_Mar19) wrote:
> should the 2nd arg be an FeedbackAdaptor?

As agreed with Stefan, CongestionController is now the
TransportFeedbackObserver. For now, I am passing it as a
TransportFeedbackObserver, but a later CL will (probably) change that to being
passed as a full-fledged CongestionController (rather than exposing the subset
of capabilities embodied in TransportFeedbackObserver), thereby allowing some
further cleanup.

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
File webrtc/modules/congestion_controller/include/congestion_controller.h
(right):

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/include/congestion_controller.h:175: Clock*
const clock_;
On 2017/03/02 19:53:58, minyue-webrtc(OOO_until_Mar19) wrote:
> if you can be nice, add "const"

This one is a bit less straight-forward than you think, because it's propagated
to many other classes, whose constructors, etc., expect a |Clock*|, not a |const
Clock*|. I'll do that in a separate CL, so as not to overwhelm this CL with
unrelated changes.

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/include/congestion_controller.h:176:
Observer* const observer_;
On 2017/03/02 19:53:58, minyue-webrtc(OOO_until_Mar19) wrote:
> here too

Done.

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/include/congestion_controller.h:177:
RtcEventLog* const event_log_;
On 2017/03/02 19:53:58, minyue-webrtc(OOO_until_Mar19) wrote:
> const RtcEventLog* const

Can't, because RtcEventLog::LogDelayBasedBweUpdate() isn't const.

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/include/congestion_controller.h:178:
PacketRouter* const packet_router_;
On 2017/03/02 19:53:58, minyue-webrtc(OOO_until_Mar19) wrote:
> here too

Similar issue (due to PacketRouter::SendFeedback() this time).

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/include/congestion_controller.h:188:
rtc::CriticalSection critsect_;
On 2017/03/02 19:53:58, minyue-webrtc(OOO_until_Mar19) wrote:
> should we rename this to differentiate from bwe_lock_

Stefan, what do you suggest? Perhaps |network_lock_|?

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
File webrtc/modules/congestion_controller/transport_feedback_adapter.h (right):

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/transport_feedback_adapter.h:53: Clock*
const clock_;
On 2017/03/02 19:53:58, minyue-webrtc(OOO_until_Mar19) wrote:
> const

Done.

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/transport_feedback_adapter.h:56:
std::vector<PacketFeedback> last_packet_feedback_vector_;
On 2017/03/02 19:53:58, minyue-webrtc(OOO_until_Mar19) wrote:
> where is the list<FeedbackObserver>

Upcoming CL. This CL only deals with moving the BWE out of the adapter and into
the CongestionController.

[elad.alon_webrtc.org, 2017-03-08 18:34:20]

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/modules/congestion...
File webrtc/modules/congestion_controller/congestion_controller.cc (right):

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/congestion_controller.cc:354: return
transport_feedback_adapter_.GetTransportFeedbackVector();
Stefan, are we worried because there's nobody making sure that this is never
called from a thread other than the worker thread, on which
OnTransportFeedback() is called? Do we want a lock? A thread-checker? If
thread-checker, would we be satisfied with only doing this for
GetTransportFeedbackVector() and OnTransportFeedback(), so as to hopefully avoid
the UT problems we saw last time we attempted to add thread-checking to all of
the adapter?

[elad.alon_webrtc.org, 2017-03-08 19:03:55]

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
File webrtc/modules/congestion_controller/include/congestion_controller.h
(right):

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/include/congestion_controller.h:175: Clock*
const clock_;
On 2017/03/08 18:25:09, elad.alon wrote:
> On 2017/03/02 19:53:58, minyue-webrtc(OOO_until_Mar19) wrote:
> > if you can be nice, add "const"
> 
> This one is a bit less straight-forward than you think, because it's
propagated
> to many other classes, whose constructors, etc., expect a |Clock*|, not a
|const
> Clock*|. I'll do that in a separate CL, so as not to overwhelm this CL with
> unrelated changes.

Done separately in the following CL: https://codereview.webrtc.org/2735423002

[stefan-webrtc, 2017-03-09 09:41:02]

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
File webrtc/modules/congestion_controller/include/congestion_controller.h
(right):

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/include/congestion_controller.h:188:
rtc::CriticalSection critsect_;
On 2017/03/08 18:25:09, elad.alon wrote:
> On 2017/03/02 19:53:58, minyue-webrtc(OOO_until_Mar19) wrote:
> > should we rename this to differentiate from bwe_lock_
> 
> Stefan, what do you suggest? Perhaps |network_lock_|?

It protects access to members accessed both on incoming feedback and on the
process thread.  Naming is hard, but maybe network_state_lock_?

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/include/congestion_controller.h:194:
std::unique_ptr<DelayBasedBwe> delay_based_bwe_ GUARDED_BY(&bwe_lock_);
Why is &bwe_lock_ needed here but not &critsect_ above?

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/modules/congestion...
File webrtc/modules/congestion_controller/congestion_controller.cc (right):

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/congestion_controller.cc:354: return
transport_feedback_adapter_.GetTransportFeedbackVector();
On 2017/03/08 18:34:20, elad.alon wrote:
> Stefan, are we worried because there's nobody making sure that this is never
> called from a thread other than the worker thread, on which
> OnTransportFeedback() is called? Do we want a lock? A thread-checker? If
> thread-checker, would we be satisfied with only doing this for
> GetTransportFeedbackVector() and OnTransportFeedback(), so as to hopefully
avoid
> the UT problems we saw last time we attempted to add thread-checking to all of
> the adapter?

A thread checker might be good. We can start with adding the thread checker here
and expand it later?

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/modules/congestion...
File webrtc/modules/congestion_controller/congestion_controller_unittest.cc
(right):

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/congestion_controller_unittest.cc:422:
return;  // Would crash the tests later otherwise.
Change line 420 to ASSERT_TRUE instead. That basically returns if the assertion
doesn't hold, thereby avoiding crashes.

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/congestion_controller_unittest.cc:428:
while (clock_.TimeInMilliseconds() - start_time_ms < 2 * kRunTimeMs) {
Perhaps break this loop out into a function with delay as a parameter and use it
above too?

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/congestion_controller_unittest.cc:429:
delay += 50;  // Delay has to increase, or it's just an added RTT.
"a longer RTT"?

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/modules/congestion...
File
webrtc/modules/congestion_controller/congestion_controller_unittests_helper.h
(right):

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/congestion_controller_unittests_helper.h:19:
void ComparePacketVectors(const std::vector<PacketFeedback>& truth,
ComparePacketFeedbackVectors maybe?

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/tools/event_log_vi...
File webrtc/tools/event_log_visualizer/analyzer.cc (left):

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/tools/event_log_vi...
webrtc/tools/event_log_visualizer/analyzer.cc:1134: // BitrateController.
Good to get this fixed!

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/tools/event_log_vi...
File webrtc/tools/event_log_visualizer/analyzer.cc (right):

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/tools/event_log_vi...
webrtc/tools/event_log_visualizer/analyzer.cc:1150: // 
feedback_adapter.InitBwe();  // TODO(elad.alon): Fix before landing.
I guess you can simply remove this now?

[elad.alon_webrtc.org, 2017-03-09 12:03:54]

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
File webrtc/modules/congestion_controller/include/congestion_controller.h
(right):

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/include/congestion_controller.h:188:
rtc::CriticalSection critsect_;
On 2017/03/09 09:41:02, stefan-webrtc wrote:
> On 2017/03/08 18:25:09, elad.alon wrote:
> > On 2017/03/02 19:53:58, minyue-webrtc(OOO_until_Mar19) wrote:
> > > should we rename this to differentiate from bwe_lock_
> > 
> > Stefan, what do you suggest? Perhaps |network_lock_|?
> 
> It protects access to members accessed both on incoming feedback and on the
> process thread.  Naming is hard, but maybe network_state_lock_?

Done.

https://codereview.webrtc.org/2725823002/diff/40001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/include/congestion_controller.h:194:
std::unique_ptr<DelayBasedBwe> delay_based_bwe_ GUARDED_BY(&bwe_lock_);
On 2017/03/09 09:41:02, stefan-webrtc wrote:
> Why is &bwe_lock_ needed here but not &critsect_ above

It works with either. I happened to have the ampersand because I copy/pasted
(typing is annoying...) from a different file. Ampersand removed.

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/modules/congestion...
File webrtc/modules/congestion_controller/congestion_controller.cc (right):

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/congestion_controller.cc:354: return
transport_feedback_adapter_.GetTransportFeedbackVector();
On 2017/03/09 09:41:02, stefan-webrtc wrote:
> On 2017/03/08 18:34:20, elad.alon wrote:
> > Stefan, are we worried because there's nobody making sure that this is never
> > called from a thread other than the worker thread, on which
> > OnTransportFeedback() is called? Do we want a lock? A thread-checker? If
> > thread-checker, would we be satisfied with only doing this for
> > GetTransportFeedbackVector() and OnTransportFeedback(), so as to hopefully
> avoid
> > the UT problems we saw last time we attempted to add thread-checking to all
of
> > the adapter?
> 
> A thread checker might be good. We can start with adding the thread checker
here
> and expand it later?

Done.

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/modules/congestion...
File webrtc/modules/congestion_controller/congestion_controller_unittest.cc
(right):

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/congestion_controller_unittest.cc:422:
return;  // Would crash the tests later otherwise.
On 2017/03/09 09:41:02, stefan-webrtc wrote:
> Change line 420 to ASSERT_TRUE instead. That basically returns if the
assertion
> doesn't hold, thereby avoiding crashes.

Done.

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/congestion_controller_unittest.cc:428:
while (clock_.TimeInMilliseconds() - start_time_ms < 2 * kRunTimeMs) {
On 2017/03/09 09:41:02, stefan-webrtc wrote:
> Perhaps break this loop out into a function with delay as a parameter and use
it
> above too?

Done.

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/congestion_controller_unittest.cc:429:
delay += 50;  // Delay has to increase, or it's just an added RTT.
On 2017/03/09 09:41:02, stefan-webrtc wrote:
> "a longer RTT"?

"Longer" is confusing to me, because it might be understood as buildup. I went
with "// Delay has to increase, or it's just RTT.", which also has the benefit
of fitting in one line (now that moving this to a function has increased
indentation).

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/modules/congestion...
File
webrtc/modules/congestion_controller/congestion_controller_unittests_helper.h
(right):

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/modules/congestion...
webrtc/modules/congestion_controller/congestion_controller_unittests_helper.h:19:
void ComparePacketVectors(const std::vector<PacketFeedback>& truth,
On 2017/03/09 09:41:02, stefan-webrtc wrote:
> ComparePacketFeedbackVectors maybe?

Done.

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/tools/event_log_vi...
File webrtc/tools/event_log_visualizer/analyzer.cc (right):

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/tools/event_log_vi...
webrtc/tools/event_log_visualizer/analyzer.cc:1150: // 
feedback_adapter.InitBwe();  // TODO(elad.alon): Fix before landing.
On 2017/03/09 09:41:02, stefan-webrtc wrote:
> I guess you can simply remove this now?

I've not had time to look into this; just commented out and moved on. I'll get
back to it later. Thanks.

[stefan-webrtc, 2017-03-09 12:15:44]

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/tools/event_log_vi...
File webrtc/tools/event_log_visualizer/analyzer.cc (right):

https://codereview.webrtc.org/2725823002/diff/60001/webrtc/tools/event_log_vi...
webrtc/tools/event_log_visualizer/analyzer.cc:1150: // 
feedback_adapter.InitBwe();  // TODO(elad.alon): Fix before landing.
On 2017/03/09 12:03:54, elad.alon wrote:
> On 2017/03/09 09:41:02, stefan-webrtc wrote:
> > I guess you can simply remove this now?
> 
> I've not had time to look into this; just commented out and moved on. I'll get
> back to it later. Thanks.

I'm pretty sure the BWE wasn't needed in this method, it's just that we had to
initialize it to avoid crashing...

Now it's removed from the feedback adapter, so that solves this hack.

[stefan-webrtc, 2017-03-09 12:16:56]

If you have a BUG which you can refer to that would be preferable. For instance
the feature you are working on that made us realize this was necessary.

[elad.alon_webrtc.org, 2017-03-09 12:45:43]

Description was changed from

==========
Move delay_based_bwe_ into CongestionController

BUG=None
==========

to

==========
Move delay_based_bwe_ into CongestionController

BUG=webrtc:7058
==========

[elad.alon_webrtc.org, 2017-03-09 12:47:25]

On 2017/03/09 12:16:56, stefan-webrtc wrote:
> If you have a BUG which you can refer to that would be preferable. For
instance
> the feature you are working on that made us realize this was necessary.

OK, done.

[stefan-webrtc, 2017-03-09 13:06:06]

lgtm

[elad.alon_webrtc.org, 2017-03-09 13:25:51]

The CQ bit was checked by elad.alon@webrtc.org

[commit-bot: I haz the power, 2017-03-09 13:25:57]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[terelius, 2017-03-09 13:34:46]

Nice. lgtm

[commit-bot: I haz the power, 2017-03-09 13:37:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-09 13:37:22]

Try jobs failed on following builders:
  linux_ubsan on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_ubsan/builds/10428)

[elad.alon_webrtc.org, 2017-03-09 14:13:32]

The CQ bit was checked by elad.alon@webrtc.org

[elad.alon_webrtc.org, 2017-03-09 14:13:33]

The patchset sent to the CQ was uploaded after l-g-t-m from stefan@webrtc.org,
terelius@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2725823002/#ps120001 (title:
"UT thread-checking.")

[commit-bot: I haz the power, 2017-03-09 14:13:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-03-09 14:40:09]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1489068812795920,
"parent_rev": "45b5fe549f9bea7a4abc9932937b6c35ad5adc54", "commit_rev":
"5bbf43f9d480f89fa57bbdbff0b028a413202695"}

[commit-bot: I haz the power, 2017-03-09 14:40:11]

Description was changed from

==========
Move delay_based_bwe_ into CongestionController

BUG=webrtc:7058
==========

to

==========
Move delay_based_bwe_ into CongestionController

BUG=webrtc:7058

Review-Url: https://codereview.webrtc.org/2725823002
Cr-Commit-Position: refs/heads/master@{#17146}
Committed:
https://chromium.googlesource.com/external/webrtc/+/5bbf43f9d480f89fa57bbdbff...
==========

[commit-bot: I haz the power, 2017-03-09 14:40:13]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/external/webrtc/+/5bbf43f9d480f89fa57bbdbff...

[nisse-webrtc, 2017-03-09 16:23:48]

A revert of this CL (patchset #7 id:120001) has been created in
https://codereview.webrtc.org/2743553003/ by nisse@webrtc.org.

The reason for reverting is: Deleting the GetTransportFeedbackObserver methods
breaks an internal project. 

Keeping the method and letting it return |this| will likely work..