Test RtcpParser rewritten to use rtcp packet classes

Test RtcpParser rewritten to use rtcp packet classes
instead of rtcp_utility

BUG=webrtc:5260
R=sprang@webrtc.org, stefan@webrtc.org

Committed: https://crrev.com/ba6f7be234cc31c2b9da791b58af77843fe048a6
Cr-Commit-Position: refs/heads/master@{#14050}

[danilchap, 2016-08-23 11:32:04]

Description was changed from

==========
Test RtcpParser changed to use rtcp classes Parse
==========

to

==========
Test RtcpParser rewritten to use rtcp packet classes
instead of rtcp_utility
==========

[danilchap, 2016-08-31 16:09:02]

Patchset #6 (id:100001) has been deleted

[danilchap, 2016-09-01 13:41:14]

Description was changed from

==========
Test RtcpParser rewritten to use rtcp packet classes
instead of rtcp_utility
==========

to

==========
Test RtcpParser rewritten to use rtcp packet classes
instead of rtcp_utility

BUG=webrtc:5260
==========

[danilchap, 2016-09-01 13:54:13]

danilchap@webrtc.org changed reviewers:
+ sprang@webrtc.org, stefan@webrtc.org

[sprang_webrtc, 2016-09-02 07:43:22]

Looks great! just some nits

https://codereview.webrtc.org/2070673002/diff/120001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc (right):

https://codereview.webrtc.org/2070673002/diff/120001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc:311: EXPECT_EQ(0u,
parser()->sender_report()->report_blocks().size());
nit: What does the style guide say, 0u or 0U? Just for consistency..

https://codereview.webrtc.org/2070673002/diff/120001/webrtc/test/rtcp_packet_...
File webrtc/test/rtcp_packet_parser.cc (right):

https://codereview.webrtc.org/2070673002/diff/120001/webrtc/test/rtcp_packet_...
webrtc/test/rtcp_packet_parser.cc:26: next_packet = header.NextPacket()) {
The dcheck inside the test statement of the for-loop looks kinda weird? Why not
just RTC_DCHECK_LE first thing in the body?

https://codereview.webrtc.org/2070673002/diff/120001/webrtc/test/rtcp_packet_...
webrtc/test/rtcp_packet_parser.cc:31: switch (header.type()) {
Header type is just an uin8_t, right? Can we add a default clause for
unrecognized values? With a warning or an ADD_FAILURE() if this just for test
and there should be no unknowns.

[danilchap, 2016-09-02 08:15:53]

https://codereview.webrtc.org/2070673002/diff/120001/webrtc/modules/rtp_rtcp/...
File webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc (right):

https://codereview.webrtc.org/2070673002/diff/120001/webrtc/modules/rtp_rtcp/...
webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc:311: EXPECT_EQ(0u,
parser()->sender_report()->report_blocks().size());
On 2016/09/02 07:43:21, språng wrote:
> nit: What does the style guide say, 0u or 0U? Just for consistency..

style doesn't say anything specific, but examples there use upper case suffix.
Habbit adjusted.

https://codereview.webrtc.org/2070673002/diff/120001/webrtc/test/rtcp_packet_...
File webrtc/test/rtcp_packet_parser.cc (right):

https://codereview.webrtc.org/2070673002/diff/120001/webrtc/test/rtcp_packet_...
webrtc/test/rtcp_packet_parser.cc:26: next_packet = header.NextPacket()) {
On 2016/09/02 07:43:21, språng wrote:
> The dcheck inside the test statement of the for-loop looks kinda weird? Why
not
> just RTC_DCHECK_LE first thing in the body?

Done.
Failed to find reasons I had for putting DCHECK into that unnatural place.

https://codereview.webrtc.org/2070673002/diff/120001/webrtc/test/rtcp_packet_...
webrtc/test/rtcp_packet_parser.cc:31: switch (header.type()) {
On 2016/09/02 07:43:21, språng wrote:
> Header type is just an uin8_t, right? Can we add a default clause for
> unrecognized values? With a warning or an ADD_FAILURE() if this just for test
> and there should be no unknowns.

Done.
ADD_FAILURE would crash fuzzers fast, so added logging.

[sprang_webrtc, 2016-09-02 09:09:10]

lgtm

[stefan-webrtc, 2016-09-02 09:38:20]

lgtm

[danilchap, 2016-09-02 09:41:29]

The CQ bit was checked by danilchap@webrtc.org

[commit-bot: I haz the power, 2016-09-02 09:41:32]

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-09-02 09:54:42]

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

[commit-bot: I haz the power, 2016-09-02 09:54:46]

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

[danilchap, 2016-09-02 16:29:28]

Description was changed from

==========
Test RtcpParser rewritten to use rtcp packet classes
instead of rtcp_utility

BUG=webrtc:5260
==========

to

==========
Test RtcpParser rewritten to use rtcp packet classes
instead of rtcp_utility

BUG=webrtc:5260
R=sprang@webrtc.org, stefan@webrtc.org

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

[danilchap, 2016-09-02 16:29:30]

Committed patchset #9 (id:180001) manually as
ba6f7be234cc31c2b9da791b58af77843fe048a6 (presubmit successful).

[commit-bot: I haz the power, 2016-09-02 16:29:30]

Description was changed from

==========
Test RtcpParser rewritten to use rtcp packet classes
instead of rtcp_utility

BUG=webrtc:5260
R=sprang@webrtc.org, stefan@webrtc.org

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

to

==========
Test RtcpParser rewritten to use rtcp packet classes
instead of rtcp_utility

BUG=webrtc:5260
R=sprang@webrtc.org, stefan@webrtc.org

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