Additional validation code for PrefixSet.

Additional validation code for PrefixSet.

The PrefixSet-vs-BloomFilter histograms showed a minor discrepency,
with a very small number of PREFIX_SET_EVENT_BLOOM_MISS_PREFIX_HIT
reports.  This CL adds code to regenerate the prefix list and
manually double-check.

Additionally, reduce memory use by requiring the input prefix vector
to be pre-sorted.

BUG=71832
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=76853

[Scott Hess - ex-Googler, 2011-03-03 21:57:41]

Ping!  Would like to get this in before weekend...

[lzheng1, 2011-03-03 22:15:11]

Working on it now.

On Thu, Mar 3, 2011 at 1:57 PM, <shess@chromium.org> wrote:

> Ping!  Would like to get this in before weekend...
>
>
>
>
> http://codereview.chromium.org/6591087/
>

[lzheng, 2011-03-03 22:30:06]

LGTM.

I thought I already done with this.

http://codereview.chromium.org/6591087/diff/1/chrome/browser/safe_browsing/pr...
File chrome/browser/safe_browsing/prefix_set.cc (right):

http://codereview.chromium.org/6591087/diff/1/chrome/browser/safe_browsing/pr...
chrome/browser/safe_browsing/prefix_set.cc:25: PrefixSet::PrefixSet(const
std::vector<SBPrefix>& prefixes) {
Do you want to call "prefixes" "sorted_prefixes" to make it clear a sorted list
is expected?

http://codereview.chromium.org/6591087/diff/1/chrome/browser/safe_browsing/pr...
File chrome/browser/safe_browsing/prefix_set.h (right):

http://codereview.chromium.org/6591087/diff/1/chrome/browser/safe_browsing/pr...
chrome/browser/safe_browsing/prefix_set.h:62: // |prefixes| must be sorted.
How about rename the  "prefixes" to "sorted_prefixes"?

[Scott Hess - ex-Googler, 2011-03-03 22:39:19]

seems reasonable.  Thanks!

On Thu, Mar 3, 2011 at 2:30 PM,  <lzheng@chromium.org> wrote:
> LGTM.
>
> I thought I already done with this.
>
>
>
http://codereview.chromium.org/6591087/diff/1/chrome/browser/safe_browsing/pr...
> File chrome/browser/safe_browsing/prefix_set.cc (right):
>
>
http://codereview.chromium.org/6591087/diff/1/chrome/browser/safe_browsing/pr...
> chrome/browser/safe_browsing/prefix_set.cc:25:
> PrefixSet::PrefixSet(const std::vector<SBPrefix>& prefixes) {
> Do you want to call "prefixes" "sorted_prefixes" to make it clear a
> sorted list is expected?
>
>
http://codereview.chromium.org/6591087/diff/1/chrome/browser/safe_browsing/pr...
> File chrome/browser/safe_browsing/prefix_set.h (right):
>
>
http://codereview.chromium.org/6591087/diff/1/chrome/browser/safe_browsing/pr...
> chrome/browser/safe_browsing/prefix_set.h:62: // |prefixes| must be
> sorted.
> How about rename the  "prefixes" to "sorted_prefixes"?
>
> http://codereview.chromium.org/6591087/
>

[lzheng1, 2011-03-03 23:09:12]

LG.

On Thu, Mar 3, 2011 at 2:39 PM, Scott Hess <shess@chromium.org> wrote:

> seems reasonable.  Thanks!
>
> On Thu, Mar 3, 2011 at 2:30 PM,  <lzheng@chromium.org> wrote:
> > LGTM.
> >
> > I thought I already done with this.
> >
> >
> >
>
http://codereview.chromium.org/6591087/diff/1/chrome/browser/safe_browsing/pr...
> > File chrome/browser/safe_browsing/prefix_set.cc (right):
> >
> >
>
http://codereview.chromium.org/6591087/diff/1/chrome/browser/safe_browsing/pr...
> > chrome/browser/safe_browsing/prefix_set.cc:25:
> > PrefixSet::PrefixSet(const std::vector<SBPrefix>& prefixes) {
> > Do you want to call "prefixes" "sorted_prefixes" to make it clear a
> > sorted list is expected?
> >
> >
>
http://codereview.chromium.org/6591087/diff/1/chrome/browser/safe_browsing/pr...
> > File chrome/browser/safe_browsing/prefix_set.h (right):
> >
> >
>
http://codereview.chromium.org/6591087/diff/1/chrome/browser/safe_browsing/pr...
> > chrome/browser/safe_browsing/prefix_set.h:62: // |prefixes| must be
> > sorted.
> > How about rename the  "prefixes" to "sorted_prefixes"?
> >
> > http://codereview.chromium.org/6591087/
> >
>

[Scott Hess - ex-Googler, 2011-03-04 00:07:57]

oops.  I noticed that the sense of the new histogram was wrong.  fixed it rather
than being confusing.

AND, I noticed that only one of the GetPrefixes() checks was setting the
"GetPrefixes() broken" histogram.  So fixed that, too.

[lzheng, 2011-03-04 00:29:06]

Don't forget to get rid of the change in protocol_manager.cc

http://codereview.chromium.org/6591087/diff/2002/chrome/browser/safe_browsing...
File chrome/browser/safe_browsing/protocol_manager.cc (right):

http://codereview.chromium.org/6591087/diff/2002/chrome/browser/safe_browsing...
chrome/browser/safe_browsing/protocol_manager.cc:120: next_update_sec_ = 30;
This should not be in the change?

http://codereview.chromium.org/6591087/diff/2002/chrome/browser/safe_browsing...
File chrome/browser/safe_browsing/safe_browsing_database.cc (right):

http://codereview.chromium.org/6591087/diff/2002/chrome/browser/safe_browsing...
chrome/browser/safe_browsing/safe_browsing_database.cc:191:
PREFIX_SET_EVENT_BLOOM_MISS_PREFIX_HIT_INVALID,
Yeah, much better.

[Scott Hess - ex-Googler, 2011-03-04 00:40:07]

http://codereview.chromium.org/6591087/diff/2002/chrome/browser/safe_browsing...
File chrome/browser/safe_browsing/protocol_manager.cc (right):

http://codereview.chromium.org/6591087/diff/2002/chrome/browser/safe_browsing...
chrome/browser/safe_browsing/protocol_manager.cc:120: next_update_sec_ = 30;
On 2011/03/04 00:29:06, lzheng wrote:
> This should not be in the change?

Doh!  Testing code.  thx.