Remove stl_util use from domain_reliability.

Remove stl_util use from domain_reliability.

BUG=555865
Committed: https://crrev.com/12ce31d0d42e72ef3f7064ba811fc27b0e109418
Cr-Commit-Position: refs/heads/master@{#415318}

[Avi (use Gerrit), 2016-08-29 19:06:50]

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

[Avi (use Gerrit), 2016-08-29 19:54:08]

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

[commit-bot: I haz the power, 2016-08-29 19:54:51]

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

[Avi (use Gerrit), 2016-08-29 20:54:18]

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

[commit-bot: I haz the power, 2016-08-29 21:27:14]

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

[Avi (use Gerrit), 2016-08-29 21:37:31]

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

[commit-bot: I haz the power, 2016-08-29 22:30:27]

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

[Avi (use Gerrit), 2016-08-29 23:01:59]

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

[Avi (use Gerrit), 2016-08-30 00:33:25]

avi@chromium.org changed reviewers:
+ davidben@chromium.org, rdsmith@chromium.org

[Avi (use Gerrit), 2016-08-30 00:33:26]

PTAL

[davidben, 2016-08-30 00:52:54]

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

[davidben, 2016-08-30 00:52:54]

Looks good but for uneasiness about the O(N) thing. Adding Julia who might be
able to weigh in more usefully there.

https://codereview.chromium.org/2289763003/diff/80001/components/domain_relia...
File components/domain_reliability/dispatcher.cc (right):

https://codereview.chromium.org/2289763003/diff/80001/components/domain_relia...
components/domain_reliability/dispatcher.cc:120: });
This switches from O(lg N) to O(N) which is a bit unfortunate. (I did see the
thread. Crappy situation. :-/)

+juliatuttle, do you know if making this O(N) would an issue here?

I do wonder if we should add a base::OwnedSet<T> instead. It seems pretty common
to have a parent object maintain some owned set of jobs or tasks or other
sub-objects which need to get cleared out when the parent is destroyed.

https://codereview.chromium.org/2289763003/diff/80001/components/domain_relia...
File components/domain_reliability/monitor.cc (right):

https://codereview.chromium.org/2289763003/diff/80001/components/domain_relia...
components/domain_reliability/monitor.cc:11: #include "base/memory/ptr_util.h"
I think this is no longer needed?

[commit-bot: I haz the power, 2016-08-30 02:10:59]

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-08-30 03:42:22]

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

[commit-bot: I haz the power, 2016-08-30 03:42:23]

Dry run: This issue passed the CQ dry run.

[Julia Tuttle, 2016-08-30 13:46:37]

https://codereview.chromium.org/2289763003/diff/80001/components/domain_relia...
File components/domain_reliability/dispatcher.cc (right):

https://codereview.chromium.org/2289763003/diff/80001/components/domain_relia...
components/domain_reliability/dispatcher.cc:120: });
On 2016/08/30 00:52:54, davidben wrote:
> This switches from O(lg N) to O(N) which is a bit unfortunate. (I did see the
> thread. Crappy situation. :-/)
> 
> +juliatuttle, do you know if making this O(N) would an issue here?
> 
> I do wonder if we should add a base::OwnedSet<T> instead. It seems pretty
common
> to have a parent object maintain some owned set of jobs or tasks or other
> sub-objects which need to get cleared out when the parent is destroyed.

Shouldn't be an issue. At absolute worst it will be O(400) or so, since that's
how many domains we have in the system.

[davidben, 2016-08-30 13:49:16]

https://codereview.chromium.org/2289763003/diff/80001/components/domain_relia...
File components/domain_reliability/dispatcher.cc (right):

https://codereview.chromium.org/2289763003/diff/80001/components/domain_relia...
components/domain_reliability/dispatcher.cc:120: });
On 2016/08/30 13:46:37, Julia Tuttle wrote:
> On 2016/08/30 00:52:54, davidben wrote:
> > This switches from O(lg N) to O(N) which is a bit unfortunate. (I did see
the
> > thread. Crappy situation. :-/)
> > 
> > +juliatuttle, do you know if making this O(N) would an issue here?
> > 
> > I do wonder if we should add a base::OwnedSet<T> instead. It seems pretty
> common
> > to have a parent object maintain some owned set of jobs or tasks or other
> > sub-objects which need to get cleared out when the parent is destroyed.
> 
> Shouldn't be an issue. At absolute worst it will be O(400) or so, since that's
> how many domains we have in the system.

Hrm. Wouldn't that bound increase once we have the standards-based header
version rather than a hard-coded domain list?

[Julia Tuttle, 2016-08-30 13:59:53]

https://codereview.chromium.org/2289763003/diff/80001/components/domain_relia...
File components/domain_reliability/dispatcher.cc (right):

https://codereview.chromium.org/2289763003/diff/80001/components/domain_relia...
components/domain_reliability/dispatcher.cc:120: });
On 2016/08/30 13:49:16, davidben wrote:
> On 2016/08/30 13:46:37, Julia Tuttle wrote:
> > On 2016/08/30 00:52:54, davidben wrote:
> > > This switches from O(lg N) to O(N) which is a bit unfortunate. (I did see
> the
> > > thread. Crappy situation. :-/)
> > > 
> > > +juliatuttle, do you know if making this O(N) would an issue here?
> > > 
> > > I do wonder if we should add a base::OwnedSet<T> instead. It seems pretty
> > common
> > > to have a parent object maintain some owned set of jobs or tasks or other
> > > sub-objects which need to get cleared out when the parent is destroyed.
> > 
> > Shouldn't be an issue. At absolute worst it will be O(400) or so, since
that's
> > how many domains we have in the system.
> 
> Hrm. Wouldn't that bound increase once we have the standards-based header
> version rather than a hard-coded domain list?

Nah, I'm hoping that ends up going through the Reporting API.

[Randy Smith (Not in Mondays), 2016-08-30 14:58:30]

rdsmith@chromium.org changed reviewers:
- rdsmith@chromium.org

[Randy Smith (Not in Mondays), 2016-08-30 14:58:31]

Julia's the right reviewer for domain reliability stuff.  Feel free to re-add me
if there's something specific you want me for.

[Avi (use Gerrit), 2016-08-30 16:11:26]

So, what do you think?

https://codereview.chromium.org/2289763003/diff/80001/components/domain_relia...
File components/domain_reliability/monitor.cc (right):

https://codereview.chromium.org/2289763003/diff/80001/components/domain_relia...
components/domain_reliability/monitor.cc:11: #include "base/memory/ptr_util.h"
On 2016/08/30 00:52:54, davidben wrote:
> I think this is no longer needed?

There's still a base::WrapUnique in the file on line 259...

https://codereview.chromium.org/2289763003/diff/80001/components/domain_relia...
components/domain_reliability/monitor.cc:259: return base::WrapUnique(new
DomainReliabilityContext(
...here.

[davidben, 2016-08-30 16:32:35]

Sounds like Julia's happy with the find_if, so lgtm.

[Avi (use Gerrit), 2016-08-30 16:34:02]

The CQ bit was checked by avi@chromium.org

[commit-bot: I haz the power, 2016-08-30 16:34:21]

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-08-30 16:38:21]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-08-30 16:42:04]

Description was changed from

==========
Remove stl_util use from domain_reliability.

BUG=555865
==========

to

==========
Remove stl_util use from domain_reliability.

BUG=555865

Committed: https://crrev.com/12ce31d0d42e72ef3f7064ba811fc27b0e109418
Cr-Commit-Position: refs/heads/master@{#415318}
==========

[commit-bot: I haz the power, 2016-08-30 16:42:05]

Patchset 5 (id:??) landed as
https://crrev.com/12ce31d0d42e72ef3f7064ba811fc27b0e109418
Cr-Commit-Position: refs/heads/master@{#415318}