Moved RtcEventLog files from call/ to logging/ (new top level dir)

Moved RtcEventLog files from call/ to logging/

The RtcEventLog headers need to be accessible from any place which needs
logging, and the implementation needs access to data structures that are
logged.

After a discussion in the code review, we all agreed to move the RtcEventLog implementation into its own top level directory - which I called "logging/" in expectation that other types of logging may have similar requirements. The directory contains two main build targets - "rtc_event_log_api", which is just rtc_event_log.h, that has no external dependencies and can be used from anywhere, and "rtc_event_log_impl" which contains the rest of the implementation and has many dependencies (more in the future).

The "api" target can be referenced from anywhere, while the "impl" target is only needed at the place of instantiation (currently Call, soon to be moved to PeerConnection by https://codereview.webrtc.org/2353033005/).

This change allows using RtcEventLog in the p2p/ directory, so that we
can log STUN pings and ICE state transitions.

BUG=webrtc:6393
R=kjellander@webrtc.org, kwiberg@webrtc.org, solenberg@webrtc.org, stefan@webrtc.org, terelius@webrtc.org

Committed: https://chromium.googlesource.com/external/webrtc/+/cc91d284e43e266f97edb999eb2ebfc8a094beac

[skvlad, 2016-09-29 03:16:17]

skvlad@webrtc.org changed reviewers:
+ kjellander@webrtc.org, solenberg@webrtc.org, stefan@webrtc.org

[skvlad, 2016-09-29 03:16:18]

Please take a look. This should be just a refactoring.

[stefan-webrtc, 2016-09-29 07:10:44]

Could you explain the thinking behind making RtcEventLog an API? I would have
expected it to perhaps go under base/.

kjellander, solenberg, what has been said about implementations under api/? It
currently looks like we're mixing headers only with both headers and
implementations.

[kjellander_webrtc, 2016-09-29 07:22:22]

On 2016/09/29 07:10:44, stefan-webrtc (holmer) wrote:
> Could you explain the thinking behind making RtcEventLog an API? I would have
> expected it to perhaps go under base/.
> 
> kjellander, solenberg, what has been said about implementations under api/? It
> currently looks like we're mixing headers only with both headers and
> implementations.

Right, I was just going to say the same. If doing refactorings like this, we
should adopt the Slim and Modular approach and only put headers in api/ (the
existing implementations in there are supposed to moved out: check blocking bugs
for https://bugs.chromium.org/p/webrtc/issues/detail?id=5716). See also
go/slim-webrtc for more details.

[skvlad, 2016-09-29 07:30:24]

On 2016/09/29 07:22:22, kjellander_webrtc wrote:
> On 2016/09/29 07:10:44, stefan-webrtc (holmer) wrote:
> > Could you explain the thinking behind making RtcEventLog an API? I would
have
> > expected it to perhaps go under base/.
> > 
> > kjellander, solenberg, what has been said about implementations under api/?
It
> > currently looks like we're mixing headers only with both headers and
> > implementations.
> 
> Right, I was just going to say the same. If doing refactorings like this, we
> should adopt the Slim and Modular approach and only put headers in api/ (the
> existing implementations in there are supposed to moved out: check blocking
bugs
> for https://bugs.chromium.org/p/webrtc/issues/detail?id=5716). See also
> go/slim-webrtc for more details.

Agreed on not putting implementations in api/ - however, there is currently no
other place where it could go. Like peerconnection.cc, the implementation of
RtcEventLog depends on modules/ (for the RTP parser), call.h (for state enums),
and potentially other directories in the future. If we want to keep the current
approach where RtcEventLog takes entire configuration structures (as in
https://cs.chromium.org/chromium/src/third_party/webrtc/call/rtc_event_log.cc...),
then the implementation needs access to these structures. The only place where
this is possible (without splitting it into multiple files) is api/. If we are
to keep only headers in api/, then we need to create another top level directory
- perhaps named api_impl? - that will contain implementations.

In other words, if not in api/ - where would files like peerconnection.cc go?

[the sun, 2016-09-29 09:54:21]

On 2016/09/29 07:30:24, skvlad wrote:
> On 2016/09/29 07:22:22, kjellander_webrtc wrote:
> > On 2016/09/29 07:10:44, stefan-webrtc (holmer) wrote:
> > > Could you explain the thinking behind making RtcEventLog an API? I would
> have
> > > expected it to perhaps go under base/.
> > > 
> > > kjellander, solenberg, what has been said about implementations under
api/?
> It
> > > currently looks like we're mixing headers only with both headers and
> > > implementations.
> > 
> > Right, I was just going to say the same. If doing refactorings like this, we
> > should adopt the Slim and Modular approach and only put headers in api/ (the
> > existing implementations in there are supposed to moved out: check blocking
> bugs
> > for https://bugs.chromium.org/p/webrtc/issues/detail?id=5716). See also
> > go/slim-webrtc for more details.
> 
> Agreed on not putting implementations in api/ - however, there is currently no
> other place where it could go. Like peerconnection.cc, the implementation of
> RtcEventLog depends on modules/ (for the RTP parser), call.h (for state
enums),
> and potentially other directories in the future. If we want to keep the
current
> approach where RtcEventLog takes entire configuration structures (as in
>
https://cs.chromium.org/chromium/src/third_party/webrtc/call/rtc_event_log.cc...),
> then the implementation needs access to these structures. The only place where
> this is possible (without splitting it into multiple files) is api/. If we are
> to keep only headers in api/, then we need to create another top level
directory
> - perhaps named api_impl? - that will contain implementations.
> 
> In other words, if not in api/ - where would files like peerconnection.cc go?

Slim WebRTC only attempts to clarify the relationship between public APIs and
their implementations, so how to exactly organize the api/ folder, and the
implementation code, is something that we'll have to figure out along the way.
As you have noticed.

In this case, it is quite clear to me that RtcEventLog shouldn't be in the api/
folder at all, because you cannot currently create a logger and supply that from
the outside. RtcEventLog::Create() is called in call.cc. 

Still, it does make sense to move RtcEventLog out from call/, for instance into
its own rtc_event_log/ top folder. In order to break the circular relationship,
it looks like we'd benefit from splitting it into two targets, akin to the Slim
WebRTC structure: an rtc_event_log_api target which contains just
rtc_event_log.h and an rtc_event_log_impl target which contains the rest (excl
tests, mock). The modules using event log can depend on the _api target and the
rtc_event_log_impl can depend on the modules (and the _api target).

This structure will make sense also going forward, when we *do* want to move the
_api target into api/ and offer the ability to supply a custom logger to the
PeerConnection or PeerConnectionFactory on construction (don't know which makes
most sense OTOH).

So:
webrtc/rtc_event_log/
  :rtc_event_log_api
    rtc_event_log.h
  :rtc_event_log_impl
    -> rtc_event_log_api
    -> misc others
    rtc_event_log.cc
    ...

It may be confusing to have an _api target that's not in api/ though. Perhaps
name the target rtc_event_log_private_api instead?

WDYT?

[stefan-webrtc, 2016-09-29 09:58:48]

+cc terelius

[hlundin-webrtc, 2016-09-29 11:42:57]

henrik.lundin@webrtc.org changed reviewers:
+ henrik.lundin@webrtc.org

[hlundin-webrtc, 2016-09-29 11:42:58]

+ cc:ivoc

[hlundin-webrtc, 2016-09-29 11:43:54]

henrik.lundin@webrtc.org changed reviewers:
- henrik.lundin@webrtc.org

[the sun, 2016-09-29 16:08:33]

Oh, and btw, even if RtcEventLog instantiation is moved to peerconnection.cc,
the above still is true - the implementations of PeerConnectionInterface should
be moved out from api/, perhaps into a new folder webrtc/peerconnection/.

[skvlad, 2016-09-29 20:20:47]

Description was changed from

==========
Moved RtcEventLog files from call/ to api/

The RtcEventLog headers need to be accessible from any place which needs
logging, and the implementation needs access to data structures that are
logged. Turns out that the only such place right now is the api/
directory. This CL moves all of RtcEventLog implementatation there.

This change allows using RtcEventLog in the p2p/ directory, so that we
can log STUN pings and ICE state transitions.

BUG=webrtc:6393
==========

to

==========
Moved RtcEventLog files from call/ to logging/

The RtcEventLog headers need to be accessible from any place which needs
logging, and the implementation needs access to data structures that are
logged. 

After a discussion in the code review, we all agreed to move the RtcEventLog
implementation into its own top level directory - which I called "logging/" in
expectation that other types of logging may have similar requirements. The
directory contains two main build targets - "rtc_event_log_api", which is just
rtc_event_log.h, that has no external dependencies and can be used from
anywhere, and "rtc_event_log_impl" which contains the rest of the implementation
and has many dependencies (more in the future).

The "api" target can be referenced from anywhere, while the "impl" target is
only needed at the place of instantiation (currently Call, soon to be moved to
PeerConnection by https://codereview.webrtc.org/2353033005/).

This change allows using RtcEventLog in the p2p/ directory, so that we
can log STUN pings and ICE state transitions.

BUG=webrtc:6393
==========

[skvlad, 2016-09-29 20:20:47]

skvlad@webrtc.org changed reviewers:
+ terelius@webrtc.org

[skvlad, 2016-09-29 20:21:35]

I've moved RtcEventLog into its own top level directory (here called 'logging'
as other log types may have similar requirements). Please take another look.

[the sun, 2016-09-30 09:40:08]

Nice! Thanks for doing this.

Further cleanup could include:
- Create an rtc_event_log_test target which could include the mock_rtc_event_log
(likely useful unit tests for components that can log), as well as the event log
parser.
- Move RtcEventLog::ParseRtcEventLog() into the _test target too.

But I think this is an improvement as is. LGTM

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/logging/rtc_event...
File webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc (right):

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/logging/rtc_event...
webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc:35: #ifdef
WEBRTC_ANDROID_PLATFORM_BUILD
Add RTC_PUSH_IGNORING_WUNDEF() / RTC_POP_IGNORING_WUNDEF() ?

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/logging/rtc_event...
File webrtc/logging/rtc_event_log/rtc_event_log_unittest_helper.cc (right):

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/logging/rtc_event...
webrtc/logging/rtc_event_log/rtc_event_log_unittest_helper.cc:23: #ifdef
WEBRTC_ANDROID_PLATFORM_BUILD
RTC_PUSH_IGNORING_WUNDEF() / RTC_POP_IGNORING_WUNDEF() ?

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/test/fuzzers/cong...
File webrtc/test/fuzzers/congestion_controller_feedback_fuzzer.cc (right):

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/test/fuzzers/cong...
webrtc/test/fuzzers/congestion_controller_feedback_fuzzer.cc:34:
CongestionController cc(&clock, &observer, &observer, nullptr);
Uhm, can we leave the null object discussion in the other CL? ;)

[kjellander_webrtc, 2016-09-30 10:57:05]

lgtm with mine and solenberg's comments addressed.
Great to see such nice work adapting the ideas of the Slim and Modular effort,
kudos!

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/logging/rtc_event...
File webrtc/logging/rtc_event_log/ringbuffer_unittest.cc (right):

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/logging/rtc_event...
webrtc/logging/rtc_event_log/ringbuffer_unittest.cc:16: #include
"webrtc/base/random.h"
nit: sort this first in the list (or does it have to be last?)

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/modules/audio_cod...
File webrtc/modules/audio_coding/DEPS (right):

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/DEPS:5:
"+webrtc/logging/rtc_event_log/rtc_event_log.h",
I don't think it makes sense to be this fine-grained.
+webrtc/logging/rtc_event_log
should be sufficient.

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/modules/bitrate_c...
File webrtc/modules/bitrate_controller/DEPS (right):

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/modules/bitrate_c...
webrtc/modules/bitrate_controller/DEPS:4:
"+webrtc/logging/rtc_event_log/rtc_event_log.h",
Make this +webrtc/logging/rtc_event_log

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/DEPS (right):

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/DEPS:5: "+webrtc/logging/rtc_event_log/rtc_event_log.h",
+webrtc/logging/rtc_event_log

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/voice_engine/DEPS
File webrtc/voice_engine/DEPS (right):

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/voice_engine/DEPS...
webrtc/voice_engine/DEPS:5: "+webrtc/logging/rtc_event_log/rtc_event_log.h",
+webrtc/logging/rtc_event_log

[kwiberg-webrtc, 2016-09-30 11:34:57]

kwiberg@webrtc.org changed reviewers:
+ kwiberg@webrtc.org

[kwiberg-webrtc, 2016-09-30 11:34:58]

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/logging/rtc_event...
File webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc (right):

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/logging/rtc_event...
webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc:35: #ifdef
WEBRTC_ANDROID_PLATFORM_BUILD
On 2016/09/30 09:40:08, the sun wrote:
> Add RTC_PUSH_IGNORING_WUNDEF() / RTC_POP_IGNORING_WUNDEF() ?

I'm not sure that it's *always* necessary to do this for the .pb.h files.
compile it with clang and see if you get errors...

[skvlad, 2016-09-30 18:50:55]

Thanks for the feedback! I still need an owner of
modules/{audio_coding,bitrate_controller,rtp_rtcp} to review this. stefan@, you
are an owner of all three - would you mind reviewing this?

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/logging/rtc_event...
File webrtc/logging/rtc_event_log/ringbuffer_unittest.cc (right):

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/logging/rtc_event...
webrtc/logging/rtc_event_log/ringbuffer_unittest.cc:16: #include
"webrtc/base/random.h"
On 2016/09/30 10:57:05, kjellander_webrtc wrote:
> nit: sort this first in the list (or does it have to be last?)

Done.

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/logging/rtc_event...
File webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc (right):

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/logging/rtc_event...
webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc:35: #ifdef
WEBRTC_ANDROID_PLATFORM_BUILD
On 2016/09/30 11:34:58, kwiberg-webrtc wrote:
> On 2016/09/30 09:40:08, the sun wrote:
> > Add RTC_PUSH_IGNORING_WUNDEF() / RTC_POP_IGNORING_WUNDEF() ?
> 
> I'm not sure that it's *always* necessary to do this for the .pb.h files.
> compile it with clang and see if you get errors...

It seems to be compiling fine on iOS and Mac bots that are using clang, so it
doesn't seem to be required in this case.

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/modules/audio_cod...
File webrtc/modules/audio_coding/DEPS (right):

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/modules/audio_cod...
webrtc/modules/audio_coding/DEPS:5:
"+webrtc/logging/rtc_event_log/rtc_event_log.h",
On 2016/09/30 10:57:05, kjellander_webrtc wrote:
> I don't think it makes sense to be this fine-grained.
> +webrtc/logging/rtc_event_log
> should be sufficient.

Ok, shortened it to +webrtc/logging/rtc_event_log here and everywhere else.

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/test/fuzzers/cong...
File webrtc/test/fuzzers/congestion_controller_feedback_fuzzer.cc (right):

https://codereview.webrtc.org/2380683005/diff/100001/webrtc/test/fuzzers/cong...
webrtc/test/fuzzers/congestion_controller_feedback_fuzzer.cc:34:
CongestionController cc(&clock, &observer, &observer, nullptr);
On 2016/09/30 09:40:08, the sun wrote:
> Uhm, can we leave the null object discussion in the other CL? ;)

Sorry, this wasn't intentional :) I'm working on 3 related CLs simultaneously
and must have been on the wrong branch.

[terelius, 2016-10-03 09:45:15]

lgtm

[stefan-webrtc, 2016-10-03 10:47:06]

Let's give kwiberg a chance to review audio_coding since he's a true OWNER
there.

lgtm for bitrate_controller and rtp_rtcp.

[kwiberg-webrtc, 2016-10-03 11:54:40]

lgtm for audio_coding

[skvlad, 2016-10-03 23:12:47]

The CQ bit was checked by skvlad@webrtc.org

[skvlad, 2016-10-03 23:12:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
solenberg@webrtc.org, kjellander@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2380683005/#ps120001 (title:
"Code review feedback")

[commit-bot: I haz the power, 2016-10-03 23:12:57]

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, 2016-10-03 23:14:06]

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

[commit-bot: I haz the power, 2016-10-03 23:14:08]

Try jobs failed on following builders:
  android_clang_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_clang_dbg/build...)
  ios64_gyp_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_gyp_dbg/builds/1118)
  ios_arm64_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/13513)
  ios_arm64_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_rel/builds/13436)
  ios_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/18764)
  ios_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/17435)
  linux_libfuzzer_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_libfuzzer_rel/bui...)
  linux_ubsan_vptr on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_ubsan_vptr/builds...)

[skvlad, 2016-10-04 01:11:23]

The CQ bit was checked by skvlad@webrtc.org

[skvlad, 2016-10-04 01:11:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
solenberg@webrtc.org, terelius@webrtc.org, kjellander@webrtc.org,
stefan@webrtc.org, kwiberg@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2380683005/#ps140001 (title:
"Rebase to master")

[commit-bot: I haz the power, 2016-10-04 01:11:32]

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

[skvlad, 2016-10-04 01:31:36]

Description was changed from

==========
Moved RtcEventLog files from call/ to logging/

The RtcEventLog headers need to be accessible from any place which needs
logging, and the implementation needs access to data structures that are
logged. 

After a discussion in the code review, we all agreed to move the RtcEventLog
implementation into its own top level directory - which I called "logging/" in
expectation that other types of logging may have similar requirements. The
directory contains two main build targets - "rtc_event_log_api", which is just
rtc_event_log.h, that has no external dependencies and can be used from
anywhere, and "rtc_event_log_impl" which contains the rest of the implementation
and has many dependencies (more in the future).

The "api" target can be referenced from anywhere, while the "impl" target is
only needed at the place of instantiation (currently Call, soon to be moved to
PeerConnection by https://codereview.webrtc.org/2353033005/).

This change allows using RtcEventLog in the p2p/ directory, so that we
can log STUN pings and ICE state transitions.

BUG=webrtc:6393
==========

to

==========
Moved RtcEventLog files from call/ to logging/

The RtcEventLog headers need to be accessible from any place which needs
logging, and the implementation needs access to data structures that are
logged.

After a discussion in the code review, we all agreed to move the RtcEventLog
implementation into its own top level directory - which I called "logging/" in
expectation that other types of logging may have similar requirements. The
directory contains two main build targets - "rtc_event_log_api", which is just
rtc_event_log.h, that has no external dependencies and can be used from
anywhere, and "rtc_event_log_impl" which contains the rest of the implementation
and has many dependencies (more in the future).

The "api" target can be referenced from anywhere, while the "impl" target is
only needed at the place of instantiation (currently Call, soon to be moved to
PeerConnection by https://codereview.webrtc.org/2353033005/).

This change allows using RtcEventLog in the p2p/ directory, so that we
can log STUN pings and ICE state transitions.

BUG=webrtc:6393
R=kjellander@webrtc.org, kwiberg@webrtc.org, solenberg@webrtc.org,
stefan@webrtc.org, terelius@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/cc91d284e43e266f97edb999e...
==========

[commit-bot: I haz the power, 2016-10-04 01:31:38]

Description was changed from

==========
Moved RtcEventLog files from call/ to logging/

The RtcEventLog headers need to be accessible from any place which needs
logging, and the implementation needs access to data structures that are
logged.

After a discussion in the code review, we all agreed to move the RtcEventLog
implementation into its own top level directory - which I called "logging/" in
expectation that other types of logging may have similar requirements. The
directory contains two main build targets - "rtc_event_log_api", which is just
rtc_event_log.h, that has no external dependencies and can be used from
anywhere, and "rtc_event_log_impl" which contains the rest of the implementation
and has many dependencies (more in the future).

The "api" target can be referenced from anywhere, while the "impl" target is
only needed at the place of instantiation (currently Call, soon to be moved to
PeerConnection by https://codereview.webrtc.org/2353033005/).

This change allows using RtcEventLog in the p2p/ directory, so that we
can log STUN pings and ICE state transitions.

BUG=webrtc:6393
R=kjellander@webrtc.org, kwiberg@webrtc.org, solenberg@webrtc.org,
stefan@webrtc.org, terelius@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/cc91d284e43e266f97edb999e...
==========

to

==========
Moved RtcEventLog files from call/ to logging/

The RtcEventLog headers need to be accessible from any place which needs
logging, and the implementation needs access to data structures that are
logged.

After a discussion in the code review, we all agreed to move the RtcEventLog
implementation into its own top level directory - which I called "logging/" in
expectation that other types of logging may have similar requirements. The
directory contains two main build targets - "rtc_event_log_api", which is just
rtc_event_log.h, that has no external dependencies and can be used from
anywhere, and "rtc_event_log_impl" which contains the rest of the implementation
and has many dependencies (more in the future).

The "api" target can be referenced from anywhere, while the "impl" target is
only needed at the place of instantiation (currently Call, soon to be moved to
PeerConnection by https://codereview.webrtc.org/2353033005/).

This change allows using RtcEventLog in the p2p/ directory, so that we
can log STUN pings and ICE state transitions.

BUG=webrtc:6393
R=kjellander@webrtc.org, kwiberg@webrtc.org, solenberg@webrtc.org,
stefan@webrtc.org, terelius@webrtc.org

Committed: https://crrev.com/cc91d284e43e266f97edb999eb2ebfc8a094beac
Cr-Commit-Position: refs/heads/master@{#14485}
==========

[skvlad, 2016-10-04 01:31:38]

Committed patchset #8 (id:140001) manually as
cc91d284e43e266f97edb999eb2ebfc8a094beac (presubmit successful).