Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2390653002:
Allow backoff entry serialization if clock is null. (Closed)
DescriptionAllow backoff entry serialization if clock is null.
BUG=652311
Committed: https://crrev.com/2f5f7eb0bc16edd389575519466539f221737a7f
Cr-Commit-Position: refs/heads/master@{#422455}
Patch Set 1 #
Total comments: 4
Patch Set 2 : feedback #
Total comments: 1
Patch Set 3 : feedback #
Messages
Total messages: 25 (14 generated)
olivierrobin@chromium.org changed reviewers: + gambard@chromium.org, johnme@chromium.org, mmenke@chromium.org
The CQ bit was checked by olivierrobin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/10/03 11:24:18, gambard wrote: > lgtm I have a problem with that. Can you please write step by step tutorial for me and my group mates? You are our last hope, if we don't find the solution to that problem, we will address http://essaydune.com/contact-us/ for help.
lgtm with nits https://codereview.chromium.org/2390653002/diff/1/net/base/backoff_entry.h File net/base/backoff_entry.h (right): https://codereview.chromium.org/2390653002/diff/1/net/base/backoff_entry.h#ne... net/base/backoff_entry.h:27: friend class BackoffEntrySerializer; Reviewers in https://codereview.chromium.org/1023473003#msg13 had a preference for avoiding friend classes unless really needed. It might make sense to make GetTimeTicksNow() public instead of friending? Alternatively, if you do make BackoffEntrySerializer a friend class, you should probably make BackoffDurationToReleaseTime private (it's only currently public so BackoffEntrySerializer can use it). https://codereview.chromium.org/2390653002/diff/1/net/base/backoff_entry_seri... File net/base/backoff_entry_serializer.cc (left): https://codereview.chromium.org/2390653002/diff/1/net/base/backoff_entry_seri... net/base/backoff_entry_serializer.cc:32: entry.GetReleaseTime() - entry.tick_clock()->NowTicks(); I believe this is the only caller of tick_clock(). Please remove the method as well.
The CQ bit was checked by olivierrobin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2390653002/diff/1/net/base/backoff_entry.h File net/base/backoff_entry.h (right): https://codereview.chromium.org/2390653002/diff/1/net/base/backoff_entry.h#ne... net/base/backoff_entry.h:27: friend class BackoffEntrySerializer; On 2016/10/03 12:18:22, johnme wrote: > Reviewers in https://codereview.chromium.org/1023473003#msg13 had a preference > for avoiding friend classes unless really needed. It might make sense to make > GetTimeTicksNow() public instead of friending? Alternatively, if you do make > BackoffEntrySerializer a friend class, you should probably make > BackoffDurationToReleaseTime private (it's only currently public so > BackoffEntrySerializer can use it). Done. https://codereview.chromium.org/2390653002/diff/1/net/base/backoff_entry_seri... File net/base/backoff_entry_serializer.cc (left): https://codereview.chromium.org/2390653002/diff/1/net/base/backoff_entry_seri... net/base/backoff_entry_serializer.cc:32: entry.GetReleaseTime() - entry.tick_clock()->NowTicks(); On 2016/10/03 12:18:22, johnme wrote: > I believe this is the only caller of tick_clock(). Please remove the method as > well. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Is there a bug this is needed for? If so, please added it to description, if not, please file one. LGTM, modulo comments. https://codereview.chromium.org/2390653002/diff/20001/net/base/backoff_entry.h File net/base/backoff_entry.h (right): https://codereview.chromium.org/2390653002/diff/20001/net/base/backoff_entry.... net/base/backoff_entry.h:112: base::TimeTicks GetTimeTicksNow() const; Need to reorder the cc file to match the order in the header (i.e., move GetTimeTicksNow above CalculateReleaseTime())
Description was changed from ========== Allow backoff entry serialization if clock is null. ========== to ========== Allow backoff entry serialization if clock is null. BUG=652311 ==========
The CQ bit was checked by olivierrobin@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from johnme@chromium.org, gambard@chromium.org, mmenke@chromium.org Link to the patchset: https://codereview.chromium.org/2390653002/#ps40001 (title: "feedback")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Allow backoff entry serialization if clock is null. BUG=652311 ========== to ========== Allow backoff entry serialization if clock is null. BUG=652311 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Allow backoff entry serialization if clock is null. BUG=652311 ========== to ========== Allow backoff entry serialization if clock is null. BUG=652311 Committed: https://crrev.com/2f5f7eb0bc16edd389575519466539f221737a7f Cr-Commit-Position: refs/heads/master@{#422455} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/2f5f7eb0bc16edd389575519466539f221737a7f Cr-Commit-Position: refs/heads/master@{#422455}
Message was sent while issue was closed.
On 2016/10/03 17:55:16, commit-bot: I haz the power wrote: > Patchset 3 (id:??) landed as > https://crrev.com/2f5f7eb0bc16edd389575519466539f221737a7f > Cr-Commit-Position: refs/heads/master@{#422455} https://appspopo.com/tutuapp/
Message was sent while issue was closed.
On 2016/10/03 11:03:20, Olivier Robin wrote: I have an issue with that. Would you be able to please compose well ordered instructional exercise for me what's more, my gathering mates? My site https://www.essaysupply.com/ |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
