Domain Reliability: Expire queued beacons after 1 hour.

Domain Reliability: Expire queued beacons after 1 hour.

This isn't perfect -- it doesn't reschedule uploads when beacons expire,
and it assumes beacon start times are roughly in increasing order -- but
it should get the job done.

(The imperfections may result in it uploading the beacon *after* an
expired one slightly early, or uploading an expired beacon because it
started before but ended after an unexpired beacon that therefore ended
up *before* it in the queue.)

(A more thorough version of this would keep an explicit "queued time"
field, but I'm hoping to merge this, so I want the simplest working
solution.)

BUG=661632
Committed: https://crrev.com/619e356507bb7c8f8e39981f662117af591f6cfa
Cr-Commit-Position: refs/heads/master@{#430060}

[Julia Tuttle, 2016-11-02 16:48:27]

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

[commit-bot: I haz the power, 2016-11-02 16:48:58]

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

[Julia Tuttle, 2016-11-02 17:01:06]

Description was changed from

==========
Domain Reliability: Expire queued beacons after 1 hour

BUG=661632
==========

to

==========
Domain Reliability: Expire queued beacons after 1 hour

This isn't perfect -- it can end up uploading unexpired beacons slightly
early, since it prunes the expired beacons right before starting an 
upload -- but it is simple and should get the job done.

BUG=661632
==========

[commit-bot: I haz the power, 2016-11-02 18:40:37]

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

[commit-bot: I haz the power, 2016-11-02 18:40:38]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Julia Tuttle, 2016-11-03 20:08:58]

Description was changed from

==========
Domain Reliability: Expire queued beacons after 1 hour

This isn't perfect -- it can end up uploading unexpired beacons slightly
early, since it prunes the expired beacons right before starting an 
upload -- but it is simple and should get the job done.

BUG=661632
==========

to

==========
Domain Reliability: Expire queued beacons after 1 hour.

This isn't perfect -- it can end up uploading unexpired beacons slightly
early, since it prunes the expired beacons right before starting an 
upload -- but it is simple and should get the job done.

BUG=661632
==========

[Julia Tuttle, 2016-11-03 20:09:08]

juliatuttle@chromium.org changed reviewers:
+ rdsmith@chromium.org

[Julia Tuttle, 2016-11-03 20:09:09]

PTAL, rdsmith.

[Julia Tuttle, 2016-11-03 22:47:20]

Description was changed from

==========
Domain Reliability: Expire queued beacons after 1 hour.

This isn't perfect -- it can end up uploading unexpired beacons slightly
early, since it prunes the expired beacons right before starting an 
upload -- but it is simple and should get the job done.

BUG=661632
==========

to

==========
Domain Reliability: Expire queued beacons after 1 hour.

This isn't perfect -- it doesn't reschedule uploads when beacons expire,and it
assumes beacon start times are roughly in increasing order -- but
it should get the job done.

(A more thorough version of this would keep an explicit "queued time"
field, but I'm hoping to merge this, so I want the simplest working
solution.)

BUG=661632
==========

[Julia Tuttle, 2016-11-03 22:47:44]

Description was changed from

==========
Domain Reliability: Expire queued beacons after 1 hour.

This isn't perfect -- it doesn't reschedule uploads when beacons expire,and it
assumes beacon start times are roughly in increasing order -- but
it should get the job done.

(A more thorough version of this would keep an explicit "queued time"
field, but I'm hoping to merge this, so I want the simplest working
solution.)

BUG=661632
==========

to

==========
Domain Reliability: Expire queued beacons after 1 hour.

This isn't perfect -- it doesn't reschedule uploads when beacons expire,
and it assumes beacon start times are roughly in increasing order -- but
it should get the job done.

(A more thorough version of this would keep an explicit "queued time"
field, but I'm hoping to merge this, so I want the simplest working
solution.)

BUG=661632
==========

[Julia Tuttle, 2016-11-03 22:47:59]

On 2016/11/03 20:09:09, Julia Tuttle wrote:
> PTAL, rdsmith.

Clarified description, as requested in person.

[Randy Smith (Not in Mondays), 2016-11-04 16:06:29]

"it doesn't reschedule uploads when beacons expire": I'd feel a bit happier if
this was followed by "resulting in the strange behavior of ...", but that's
suggestion level.

LGTM with request for constant below.

https://codereview.chromium.org/2464163003/diff/1/components/domain_reliabili...
File components/domain_reliability/context.cc (right):

https://codereview.chromium.org/2464163003/diff/1/components/domain_reliabili...
components/domain_reliability/context.cc:275: const base::TimeDelta kMaxAge =
base::TimeDelta::FromHours(1);
Defined constant somewhere?

[Julia Tuttle, 2016-11-04 19:28:55]

Description was changed from

==========
Domain Reliability: Expire queued beacons after 1 hour.

This isn't perfect -- it doesn't reschedule uploads when beacons expire,
and it assumes beacon start times are roughly in increasing order -- but
it should get the job done.

(A more thorough version of this would keep an explicit "queued time"
field, but I'm hoping to merge this, so I want the simplest working
solution.)

BUG=661632
==========

to

==========
Domain Reliability: Expire queued beacons after 1 hour.

This isn't perfect -- it doesn't reschedule uploads when beacons expire,
and it assumes beacon start times are roughly in increasing order -- but
it should get the job done.

(The imperfections may result in it uploading the beacon *after* an 
expired one slightly early, or uploading an expired beacon because it 
started before but ended after an unexpired beacon that therefore ended
up *before* it in the queue.)

(A more thorough version of this would keep an explicit "queued time"
field, but I'm hoping to merge this, so I want the simplest working
solution.)

BUG=661632
==========

[Julia Tuttle, 2016-11-04 19:29:55]

On 2016/11/04 16:06:29, Randy Smith - Not in Mondays wrote:
> "it doesn't reschedule uploads when beacons expire": I'd feel a bit happier if
> this was followed by "resulting in the strange behavior of ...", but that's
> suggestion level.

Done.

> LGTM with request for constant below.
> 
>
https://codereview.chromium.org/2464163003/diff/1/components/domain_reliabili...
> File components/domain_reliability/context.cc (right):
> 
>
https://codereview.chromium.org/2464163003/diff/1/components/domain_reliabili...
> components/domain_reliability/context.cc:275: const base::TimeDelta kMaxAge =
> base::TimeDelta::FromHours(1);
> Defined constant somewhere?

Line 275 is somewhere, and that is in fact a constant. :) Do you want me to move
it further up in the file?

[Randy Smith (Not in Mondays), 2016-11-04 19:37:27]

On 2016/11/04 19:29:55, Julia Tuttle wrote:
> On 2016/11/04 16:06:29, Randy Smith - Not in Mondays wrote:
> > "it doesn't reschedule uploads when beacons expire": I'd feel a bit happier
if
> > this was followed by "resulting in the strange behavior of ...", but that's
> > suggestion level.
> 
> Done.
> 
> > LGTM with request for constant below.
> > 
> >
>
https://codereview.chromium.org/2464163003/diff/1/components/domain_reliabili...
> > File components/domain_reliability/context.cc (right):
> > 
> >
>
https://codereview.chromium.org/2464163003/diff/1/components/domain_reliabili...
> > components/domain_reliability/context.cc:275: const base::TimeDelta kMaxAge
=
> > base::TimeDelta::FromHours(1);
> > Defined constant somewhere?
> 
> Line 275 is somewhere, and that is in fact a constant. :) Do you want me to
move
> it further up in the file?

Huh.  Good point.  Never mind.

[Julia Tuttle, 2016-11-04 20:32:16]

The CQ bit was checked by juliatuttle@chromium.org

[commit-bot: I haz the power, 2016-11-04 20:32:48]

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-11-04 23:11:14]

Description was changed from

==========
Domain Reliability: Expire queued beacons after 1 hour.

This isn't perfect -- it doesn't reschedule uploads when beacons expire,
and it assumes beacon start times are roughly in increasing order -- but
it should get the job done.

(The imperfections may result in it uploading the beacon *after* an 
expired one slightly early, or uploading an expired beacon because it 
started before but ended after an unexpired beacon that therefore ended
up *before* it in the queue.)

(A more thorough version of this would keep an explicit "queued time"
field, but I'm hoping to merge this, so I want the simplest working
solution.)

BUG=661632
==========

to

==========
Domain Reliability: Expire queued beacons after 1 hour.

This isn't perfect -- it doesn't reschedule uploads when beacons expire,
and it assumes beacon start times are roughly in increasing order -- but
it should get the job done.

(The imperfections may result in it uploading the beacon *after* an 
expired one slightly early, or uploading an expired beacon because it 
started before but ended after an unexpired beacon that therefore ended
up *before* it in the queue.)

(A more thorough version of this would keep an explicit "queued time"
field, but I'm hoping to merge this, so I want the simplest working
solution.)

BUG=661632
==========

[commit-bot: I haz the power, 2016-11-04 23:11:14]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-11-04 23:16:28]

Description was changed from

==========
Domain Reliability: Expire queued beacons after 1 hour.

This isn't perfect -- it doesn't reschedule uploads when beacons expire,
and it assumes beacon start times are roughly in increasing order -- but
it should get the job done.

(The imperfections may result in it uploading the beacon *after* an 
expired one slightly early, or uploading an expired beacon because it 
started before but ended after an unexpired beacon that therefore ended
up *before* it in the queue.)

(A more thorough version of this would keep an explicit "queued time"
field, but I'm hoping to merge this, so I want the simplest working
solution.)

BUG=661632
==========

to

==========
Domain Reliability: Expire queued beacons after 1 hour.

This isn't perfect -- it doesn't reschedule uploads when beacons expire,
and it assumes beacon start times are roughly in increasing order -- but
it should get the job done.

(The imperfections may result in it uploading the beacon *after* an
expired one slightly early, or uploading an expired beacon because it
started before but ended after an unexpired beacon that therefore ended
up *before* it in the queue.)

(A more thorough version of this would keep an explicit "queued time"
field, but I'm hoping to merge this, so I want the simplest working
solution.)

BUG=661632

Committed: https://crrev.com/619e356507bb7c8f8e39981f662117af591f6cfa
Cr-Commit-Position: refs/heads/master@{#430060}
==========

[commit-bot: I haz the power, 2016-11-04 23:16:29]

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