Limit size of typed url node.

Limit size of typed url node.

Now, we cap the # visits for each node to 100 items, to avoid hitting the 10K
node size limit on the server. We also ignore all reload visits, since those
are not used by the omnibox suggestion algorithm and tend to be the bulk of
the visits in the large nodes we've encountered.

BUG=89460
TEST=Run unit tests.


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

[Andrew T Wilson (Slow), 2011-08-07 23:00:07]

Please take a look.

[Andrew T Wilson (Slow), 2011-08-09 18:04:59]

Looks like Nicolas is OOO this week - can you take a look at this, Rick?

[Rick Campbell, 2011-08-09 21:25:26]

LGTM with a bunch of nits.

http://codereview.chromium.org/7550062/diff/2001/chrome/browser/sync/glue/typ...
File chrome/browser/sync/glue/typed_url_model_associator.cc (right):

http://codereview.chromium.org/7550062/diff/2001/chrome/browser/sync/glue/typ...
chrome/browser/sync/glue/typed_url_model_associator.cc:570: // Walk the
passed-in visit vector and count the # typed visits.
uber-nit: ... the # of typed visits.

http://codereview.chromium.org/7550062/diff/2001/chrome/browser/sync/glue/typ...
chrome/browser/sync/glue/typed_url_model_associator.cc:578: total++;
nit: ++total

This is definitely deep into nit-picking territory as well, but I generally
believe that prefix should be used nearly exclusively because postfix has
special semantics -- we want the special case to stand out, rather than being
the default.

http://codereview.chromium.org/7550062/diff/2001/chrome/browser/sync/glue/typ...
chrome/browser/sync/glue/typed_url_model_associator.cc:580: typed_count++;
++typed_count

http://codereview.chromium.org/7550062/diff/2001/chrome/browser/sync/glue/typ...
chrome/browser/sync/glue/typed_url_model_associator.cc:608: skip_count--;
--skip_count

http://codereview.chromium.org/7550062/diff/2001/chrome/browser/sync/glue/typ...
chrome/browser/sync/glue/typed_url_model_associator.cc:615:
CHECK_GT(typed_url->visits_size(), 0);
Maybe DCHECK(skip_count == 0) here?

http://codereview.chromium.org/7550062/diff/2001/chrome/browser/sync/glue/typ...
File chrome/browser/sync/glue/typed_url_model_associator_unittest.cc (right):

http://codereview.chromium.org/7550062/diff/2001/chrome/browser/sync/glue/typ...
chrome/browser/sync/glue/typed_url_model_associator_unittest.cc:313: 
Delete blank line.

[Andrew T Wilson (Slow), 2011-08-10 21:11:30]

Thanks. I'm going to run it through the trybots again to make sure that new
DCHECK doesn't cause problems before landing.

http://codereview.chromium.org/7550062/diff/2001/chrome/browser/sync/glue/typ...
File chrome/browser/sync/glue/typed_url_model_associator.cc (right):

http://codereview.chromium.org/7550062/diff/2001/chrome/browser/sync/glue/typ...
chrome/browser/sync/glue/typed_url_model_associator.cc:570: // Walk the
passed-in visit vector and count the # typed visits.
On 2011/08/09 21:25:26, Rick Campbell wrote:
> uber-nit: ... the # of typed visits.

Done.

http://codereview.chromium.org/7550062/diff/2001/chrome/browser/sync/glue/typ...
chrome/browser/sync/glue/typed_url_model_associator.cc:578: total++;
On 2011/08/09 21:25:26, Rick Campbell wrote:
> nit: ++total
> 
> This is definitely deep into nit-picking territory as well, but I generally
> believe that prefix should be used nearly exclusively because postfix has
> special semantics -- we want the special case to stand out, rather than being
> the default.

Done.

http://codereview.chromium.org/7550062/diff/2001/chrome/browser/sync/glue/typ...
chrome/browser/sync/glue/typed_url_model_associator.cc:580: typed_count++;
On 2011/08/09 21:25:26, Rick Campbell wrote:
> ++typed_count

Done.

http://codereview.chromium.org/7550062/diff/2001/chrome/browser/sync/glue/typ...
chrome/browser/sync/glue/typed_url_model_associator.cc:608: skip_count--;
On 2011/08/09 21:25:26, Rick Campbell wrote:
> --skip_count

Done.

http://codereview.chromium.org/7550062/diff/2001/chrome/browser/sync/glue/typ...
chrome/browser/sync/glue/typed_url_model_associator.cc:615:
CHECK_GT(typed_url->visits_size(), 0);
On 2011/08/09 21:25:26, Rick Campbell wrote:
> Maybe DCHECK(skip_count == 0) here?

Done.

http://codereview.chromium.org/7550062/diff/2001/chrome/browser/sync/glue/typ...
File chrome/browser/sync/glue/typed_url_model_associator_unittest.cc (right):

http://codereview.chromium.org/7550062/diff/2001/chrome/browser/sync/glue/typ...
chrome/browser/sync/glue/typed_url_model_associator_unittest.cc:313: 
On 2011/08/09 21:25:26, Rick Campbell wrote:
> Delete blank line.

Done.

[commit-bot: I haz the power, 2011-08-11 01:11:42]

Change committed as 96287