Add a QuicChromiumClock and make QuicClock abstract to match internal code.

Add a QuicChromiumClock and make QuicClock abstract to match internal code.

BUG=

Committed: https://crrev.com/799bbe860fd36a73c92f0bdf7e7f061e0cde54e3
Cr-Commit-Position: refs/heads/master@{#436601}

[Ryan Hamilton, 2016-12-02 23:28:49]

rch@chromium.org changed reviewers:
+ jri@chromium.org

[Ryan Hamilton, 2016-12-03 03:33:01]

The CQ bit was checked by rch@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-03 03:33:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-03 04:41:08]

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

[commit-bot: I haz the power, 2016-12-03 04:41:09]

Dry run: This issue passed the CQ dry run.

[Jana, 2016-12-03 19:32:19]

Looks good! A couple of comments.

https://codereview.chromium.org/2547233002/diff/1/net/quic/platform/api/quic_...
File net/quic/platform/api/quic_clock.cc (right):

https://codereview.chromium.org/2547233002/diff/1/net/quic/platform/api/quic_...
net/quic/platform/api/quic_clock.cc:13: QuicTime
QuicClock::ConvertWallTimeToQuicTime(
Since all methods except for this one are pure virtual, I'm tempted to try and
make QuicClock an Interface class...  you make this method a utility method
outside of QuicClock, and make this class QuicClockInterface?

https://codereview.chromium.org/2547233002/diff/1/net/quic/platform/api/quic_...
File net/quic/platform/api/quic_clock.h (right):

https://codereview.chromium.org/2547233002/diff/1/net/quic/platform/api/quic_...
net/quic/platform/api/quic_clock.h:22: // Returns the approximate current time
as a QuicTime object.
Can you also define more precisely what "approximate" means, so that an impl
knows what to implement?

https://codereview.chromium.org/2547233002/diff/1/net/quic/platform/api/quic_...
net/quic/platform/api/quic_clock.h:26: // Note: this use significant resources
please use only if needed.
This note should probably say "this may use significant resources".

[Ryan Hamilton, 2016-12-04 03:01:11]

On 2016/12/03 19:32:19, Jana wrote:
> Looks good! A couple of comments.
> 
>
https://codereview.chromium.org/2547233002/diff/1/net/quic/platform/api/quic_...
> File net/quic/platform/api/quic_clock.cc (right):
> 
>
https://codereview.chromium.org/2547233002/diff/1/net/quic/platform/api/quic_...
> net/quic/platform/api/quic_clock.cc:13: QuicTime
> QuicClock::ConvertWallTimeToQuicTime(
> Since all methods except for this one are pure virtual, I'm tempted to try and
> make QuicClock an Interface class...  you make this method a utility method
> outside of QuicClock, and make this class QuicClockInterface?
> 
>
https://codereview.chromium.org/2547233002/diff/1/net/quic/platform/api/quic_...
> File net/quic/platform/api/quic_clock.h (right):
> 
>
https://codereview.chromium.org/2547233002/diff/1/net/quic/platform/api/quic_...
> net/quic/platform/api/quic_clock.h:22: // Returns the approximate current time
> as a QuicTime object.
> Can you also define more precisely what "approximate" means, so that an impl
> knows what to implement?
> 
>
https://codereview.chromium.org/2547233002/diff/1/net/quic/platform/api/quic_...
> net/quic/platform/api/quic_clock.h:26: // Note: this use significant resources
> please use only if needed.
> This note should probably say "this may use significant resources".

These are all great suggestions! But since this CL syncs quic_clock.{h,cc} with
the internal version, I'll cook up a followup CL there. Cool?

[Ryan Hamilton, 2016-12-05 23:57:01]

On 2016/12/04 03:01:11, Ryan Hamilton wrote:
> 
> These are all great suggestions! But since this CL syncs quic_clock.{h,cc}
with
> the internal version, I'll cook up a followup CL there. Cool?

Ping?

[Jana, 2016-12-06 06:14:36]

On 2016/12/05 23:57:01, Ryan Hamilton wrote:
> On 2016/12/04 03:01:11, Ryan Hamilton wrote:
> > 
> > These are all great suggestions! But since this CL syncs quic_clock.{h,cc}
> with
> > the internal version, I'll cook up a followup CL there. Cool?
> 
> Ping?

Ah sorry! I totally didn't see your response come through. SGTM.

[Jana, 2016-12-06 06:14:45]

The CQ bit was checked by jri@chromium.org

[Jana, 2016-12-06 06:14:46]

lgtm

[commit-bot: I haz the power, 2016-12-06 06:15:07]

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

[commit-bot: I haz the power, 2016-12-06 07:16:49]

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

[commit-bot: I haz the power, 2016-12-06 07:16:50]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ryan Hamilton, 2016-12-06 14:41:46]

The CQ bit was checked by rch@chromium.org

[commit-bot: I haz the power, 2016-12-06 14:41:58]

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

[commit-bot: I haz the power, 2016-12-06 15:30:12]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1481035306794640, "parent_rev":
"546062d56498c35169866453a7d18ed8a5523340", "commit_rev":
"4a2585f19aa3d5b6d6135c30ad0828d74a3c7eab"}

[commit-bot: I haz the power, 2016-12-06 15:30:37]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-12-06 15:32:29]

Description was changed from

==========
Add a QuicChromiumClock and make QuicClock abstract to match internal code.

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

to

==========
Add a QuicChromiumClock and make QuicClock abstract to match internal code.

BUG=

Committed: https://crrev.com/799bbe860fd36a73c92f0bdf7e7f061e0cde54e3
Cr-Commit-Position: refs/heads/master@{#436601}
==========

[commit-bot: I haz the power, 2016-12-06 15:32:30]

Patchset 1 (id:??) landed as
https://crrev.com/799bbe860fd36a73c92f0bdf7e7f061e0cde54e3
Cr-Commit-Position: refs/heads/master@{#436601}