Convert rtc_event_log from webrtc::Clock to rtc::TimeMicros.

Convert rtc_event_log from webrtc::Clock to rtc::TimeMicros.

TBR=pthatcher@webrtc.org
BUG=webrtc:6733
Review-Url: https://codereview.webrtc.org/2515653002
Cr-Commit-Position: refs/heads/master@{#15711}
Committed: https://chromium.googlesource.com/external/webrtc/+/306127635e72a31adf351078b17fa0d8ebc479e0

[nisse-webrtc, 2016-11-18 13:17:49]

nisse@webrtc.org changed reviewers:
+ solenberg@webrtc.org, terelius@webrtc.org

[terelius, 2016-11-18 13:44:28]

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
File webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc (right):

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc:302:
fake_clock.AdvanceTime(
Could we make a convenience function for this? A lot of the test code advance
the clock by a certain number of microseconds (or milliseconds) for each
"action".

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc:460:
fake_clock.SetTimeNanos(rtc::kNumNanosecsPerMicrosec * prng.Rand<uint32_t>());
Same thing here; could we add a convenience function to set the fake clock
without having to do unit conversions?

[nisse-webrtc, 2016-11-18 15:03:44]

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
File webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc (right):

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc:302:
fake_clock.AdvanceTime(
On 2016/11/18 13:44:28, terelius wrote:
> Could we make a convenience function for this? A lot of the test code advance
> the clock by a certain number of microseconds (or milliseconds) for each
> "action".

What precisely do you suggest to make this more convenient? I also find the
TimeDelta abstraction a bit clumsy.

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc:460:
fake_clock.SetTimeNanos(rtc::kNumNanosecsPerMicrosec * prng.Rand<uint32_t>());
On 2016/11/18 13:44:28, terelius wrote:
> Same thing here; could we add a convenience function to set the fake clock
> without having to do unit conversions?

Maybe a constructor taking a TimeDelta (or some int type, but we should maintain
some consistency with AdvanceTime).

[the sun, 2016-11-18 15:43:59]

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
File webrtc/logging/rtc_event_log/rtc_event_log.cc (left):

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
webrtc/logging/rtc_event_log/rtc_event_log.cc:95:
RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(RtcEventLogImpl);
Why'd this go?

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
File webrtc/logging/rtc_event_log/rtc_event_log.cc (right):

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
webrtc/logging/rtc_event_log/rtc_event_log.cc:54: explicit RtcEventLogImpl();
no need for explicit

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
File webrtc/logging/rtc_event_log/rtc_event_log_helper_thread.cc (right):

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
webrtc/logging/rtc_event_log/rtc_event_log_helper_thread.cc:229:
std::min(stop_time_, static_cast<int64_t>(rtc::TimeMicros())));
Oh, so theoretically, the other calls to rtc::TimeMicros may cause wrapping? You
should fix TimeMicros() to return int64_t before landing this.

[terelius, 2016-11-18 15:55:33]

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
File webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc (right):

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc:302:
fake_clock.AdvanceTime(
On 2016/11/18 15:03:43, nisse-webrtc wrote:
> On 2016/11/18 13:44:28, terelius wrote:
> > Could we make a convenience function for this? A lot of the test code
advance
> > the clock by a certain number of microseconds (or milliseconds) for each
> > "action".
> 
> What precisely do you suggest to make this more convenient? I also find the
> TimeDelta abstraction a bit clumsy.

A function AdvanceTimeMicroseconds(int64_t) comes to mind. :)

[nisse-webrtc, 2016-11-21 07:58:02]

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
File webrtc/logging/rtc_event_log/rtc_event_log.cc (left):

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
webrtc/logging/rtc_event_log/rtc_event_log.cc:95:
RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(RtcEventLogImpl);
On 2016/11/18 15:43:59, the sun wrote:
> Why'd this go?

Because otherwise it failed to compile... As far as I understood, the expansion
of this macro seemed to collide with the new default constructor
RtcEventLogImpl().

What should I do to be able to keep this?

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
File webrtc/logging/rtc_event_log/rtc_event_log_helper_thread.cc (right):

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
webrtc/logging/rtc_event_log/rtc_event_log_helper_thread.cc:229:
std::min(stop_time_, static_cast<int64_t>(rtc::TimeMicros())));
On 2016/11/18 15:43:59, the sun wrote:
> Oh, so theoretically, the other calls to rtc::TimeMicros may cause wrapping?
You
> should fix TimeMicros() to return int64_t before landing this.

I'll try to land https://codereview.webrtc.org/2514553003/ (doing precisely
that) first. But it's a bit complicated, because that change breaks at least two
downstream projects.

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
File webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc (right):

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc:302:
fake_clock.AdvanceTime(
On 2016/11/18 15:55:33, terelius wrote:
> On 2016/11/18 15:03:43, nisse-webrtc wrote:
> > On 2016/11/18 13:44:28, terelius wrote:
> > > Could we make a convenience function for this? A lot of the test code
> advance
> > > the clock by a certain number of microseconds (or milliseconds) for each
> > > "action".
> > 
> > What precisely do you suggest to make this more convenient? I also find the
> > TimeDelta abstraction a bit clumsy.
> 
> A function AdvanceTimeMicroseconds(int64_t) comes to mind. :) 

Hmm. I can add AdvanceTimeMicros and SetTimeMicros. Not sure why Taylor choose
to copy Chrome's TimeDelta class just for this.

[the sun, 2016-11-21 08:35:02]

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
File webrtc/logging/rtc_event_log/rtc_event_log.cc (left):

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
webrtc/logging/rtc_event_log/rtc_event_log.cc:95:
RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(RtcEventLogImpl);
On 2016/11/21 07:58:02, nisse-webrtc wrote:
> On 2016/11/18 15:43:59, the sun wrote:
> > Why'd this go?
> 
> Because otherwise it failed to compile... As far as I understood, the
expansion
> of this macro seemed to collide with the new default constructor
> RtcEventLogImpl().
> 
> What should I do to be able to keep this?

Oh, sorry for being inattentive. You should use RTC_DISALLOW_COPY_AND_ASSIGN()
instead.

[nisse-webrtc, 2016-11-28 14:26:41]

nisse@webrtc.org changed reviewers:
+ pthatcher@webrtc.org

[nisse-webrtc, 2016-11-28 14:26:42]

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
File webrtc/logging/rtc_event_log/rtc_event_log.cc (left):

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
webrtc/logging/rtc_event_log/rtc_event_log.cc:95:
RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(RtcEventLogImpl);
On 2016/11/21 08:35:01, the sun wrote:
> On 2016/11/21 07:58:02, nisse-webrtc wrote:
> > On 2016/11/18 15:43:59, the sun wrote:
> > > Why'd this go?
> > 
> > Because otherwise it failed to compile... As far as I understood, the
> expansion
> > of this macro seemed to collide with the new default constructor
> > RtcEventLogImpl().
> > 
> > What should I do to be able to keep this?
> 
> Oh, sorry for being inattentive. You should use RTC_DISALLOW_COPY_AND_ASSIGN()
> instead.

Done.

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
File webrtc/logging/rtc_event_log/rtc_event_log_helper_thread.cc (right):

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
webrtc/logging/rtc_event_log/rtc_event_log_helper_thread.cc:229:
std::min(stop_time_, static_cast<int64_t>(rtc::TimeMicros())));
On 2016/11/21 07:58:02, nisse-webrtc wrote:
> On 2016/11/18 15:43:59, the sun wrote:
> > Oh, so theoretically, the other calls to rtc::TimeMicros may cause wrapping?
> You
> > should fix TimeMicros() to return int64_t before landing this.
> 
> I'll try to land https://codereview.webrtc.org/2514553003/ (doing precisely
> that) first. But it's a bit complicated, because that change breaks at least
two
> downstream projects.

Deleting the cast now.

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
File webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc (right):

https://codereview.webrtc.org/2515653002/diff/1/webrtc/logging/rtc_event_log/...
webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc:302:
fake_clock.AdvanceTime(
On 2016/11/21 07:58:02, nisse-webrtc wrote:
> On 2016/11/18 15:55:33, terelius wrote:
> > On 2016/11/18 15:03:43, nisse-webrtc wrote:
> > > On 2016/11/18 13:44:28, terelius wrote:
> > > > Could we make a convenience function for this? A lot of the test code
> > advance
> > > > the clock by a certain number of microseconds (or milliseconds) for each
> > > > "action".
> > > 
> > > What precisely do you suggest to make this more convenient? I also find
the
> > > TimeDelta abstraction a bit clumsy.
> > 
> > A function AdvanceTimeMicroseconds(int64_t) comes to mind. :) 
> 
> Hmm. I can add AdvanceTimeMicros and SetTimeMicros. Not sure why Taylor choose
> to copy Chrome's TimeDelta class just for this.

Done.

[terelius, 2016-11-28 15:27:22]

lgtm. Thanks!

[the sun, 2016-11-28 19:37:35]

lgtm

[nisse-webrtc, 2016-12-02 12:58:48]

Peter, ping?

This cl adds two convenience methods to the FakeClock class.

[nisse-webrtc, 2016-12-20 10:46:34]

Description was changed from

==========
Convert rtc_event_log from webrtc::Clock to rtc::TimeMicros.

BUG=webrtc:6733
==========

to

==========
Convert rtc_event_log from webrtc::Clock to rtc::TimeMicros.

TBR=pthatcher@webrtc.org
BUG=webrtc:6733
==========

[nisse-webrtc, 2016-12-20 10:46:50]

The CQ bit was checked by nisse@webrtc.org

[commit-bot: I haz the power, 2016-12-20 10:46:53]

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-12-20 10:49:22]

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

[commit-bot: I haz the power, 2016-12-20 10:49:23]

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

[nisse-webrtc, 2016-12-20 11:33:52]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2016-12-20 11:33:53]

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

[commit-bot: I haz the power, 2016-12-20 11:33:59]

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-12-20 13:03:59]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1482233632643020,
"parent_rev": "022b54e86a538236b6a3cb97b80dfa1b3e3d5c56", "commit_rev":
"306127635e72a31adf351078b17fa0d8ebc479e0"}

[commit-bot: I haz the power, 2016-12-20 13:04:01]

Description was changed from

==========
Convert rtc_event_log from webrtc::Clock to rtc::TimeMicros.

TBR=pthatcher@webrtc.org
BUG=webrtc:6733
==========

to

==========
Convert rtc_event_log from webrtc::Clock to rtc::TimeMicros.

TBR=pthatcher@webrtc.org
BUG=webrtc:6733

Review-Url: https://codereview.webrtc.org/2515653002
Cr-Commit-Position: refs/heads/master@{#15711}
Committed:
https://chromium.googlesource.com/external/webrtc/+/306127635e72a31adf351078b...
==========

[commit-bot: I haz the power, 2016-12-20 13:04:02]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/external/webrtc/+/306127635e72a31adf351078b...