predictors: Remove unused field from the resource_prefetch_predictor database.

predictors: Remove unused field from the resource_prefetch_predictor database.

BUG=631966
Committed: https://crrev.com/df268d9d43870d8168aa6550837c7bd5d32d399b
Cr-Commit-Position: refs/heads/master@{#415250}

[Benoit L, 2016-08-26 12:47:40]

lizeb@chromium.org changed reviewers:
+ pasko@chromium.org

[Benoit L, 2016-08-26 12:47:42]

Hi,

This is actually a simplified (and reduced in scope) version of
https://codereview.chromium.org/2268283005/, after the discussion with you.

PTAL, thanks.

[pasko, 2016-08-26 13:40:30]

removing unused field definitely lgtm, a question and an optional nit below

https://codereview.chromium.org/2287473002/diff/1/chrome/browser/predictors/r...
File chrome/browser/predictors/resource_prefetch_predictor_tables.h (right):

https://codereview.chromium.org/2287473002/diff/1/chrome/browser/predictors/r...
chrome/browser/predictors/resource_prefetch_predictor_tables.h:63: size_t
number_of_hits;
Do we want to still plumb these one by one in FromProto/ToProto? Is removing
them a plan for the next cleanup?

https://codereview.chromium.org/2287473002/diff/1/chrome/browser/predictors/r...
chrome/browser/predictors/resource_prefetch_predictor_tables.h:123: static const
size_t kMaxStringLength = 1024;
I would recommend constexpr. The idea of including this constant in all the
various .cc files and then resolving the duplicates in the linker is not
uncommon in C++ land, but somehow feels less optimal, given a strictly better
alternative.

[Benoit L, 2016-08-29 13:11:40]

Thanks!

https://codereview.chromium.org/2287473002/diff/1/chrome/browser/predictors/r...
File chrome/browser/predictors/resource_prefetch_predictor_tables.h (right):

https://codereview.chromium.org/2287473002/diff/1/chrome/browser/predictors/r...
chrome/browser/predictors/resource_prefetch_predictor_tables.h:63: size_t
number_of_hits;
On 2016/08/26 13:40:30, pasko wrote:
> Do we want to still plumb these one by one in FromProto/ToProto? Is removing
> them a plan for the next cleanup?

If the proto doesn't reflect everything and needs methods added, no.

https://codereview.chromium.org/2287473002/diff/1/chrome/browser/predictors/r...
chrome/browser/predictors/resource_prefetch_predictor_tables.h:123: static const
size_t kMaxStringLength = 1024;
On 2016/08/26 13:40:30, pasko wrote:
> I would recommend constexpr. The idea of including this constant in all the
> various .cc files and then resolving the duplicates in the linker is not
> uncommon in C++ land, but somehow feels less optimal, given a strictly better
> alternative. 

Done.

[Benoit L, 2016-08-29 13:11:45]

The CQ bit was checked by lizeb@chromium.org

[Benoit L, 2016-08-29 13:11:45]

The patchset sent to the CQ was uploaded after l-g-t-m from pasko@chromium.org
Link to the patchset: https://codereview.chromium.org/2287473002/#ps20001
(title: "Rebase.")

[commit-bot: I haz the power, 2016-08-29 13:11:56]

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-29 13:46:28]

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

[commit-bot: I haz the power, 2016-08-29 13:46:29]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Benoit L, 2016-08-29 14:00:33]

The CQ bit was checked by lizeb@chromium.org

[commit-bot: I haz the power, 2016-08-29 14:00:56]

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-29 14:30:58]

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

[commit-bot: I haz the power, 2016-08-29 14:30:59]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Benoit L, 2016-08-29 14:33:31]

The CQ bit was checked by lizeb@chromium.org

[commit-bot: I haz the power, 2016-08-29 14:34:22]

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-29 16:05:44]

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

[commit-bot: I haz the power, 2016-08-29 16:05:46]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Benoit L, 2016-08-30 08:25:12]

The CQ bit was checked by lizeb@chromium.org

[commit-bot: I haz the power, 2016-08-30 08:25:28]

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 09:00:53]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-30 09:03:16]

Description was changed from

==========
predictors: Remove unused field from the resource_prefetch_predictor database.

BUG=631966
==========

to

==========
predictors: Remove unused field from the resource_prefetch_predictor database.

BUG=631966

Committed: https://crrev.com/df268d9d43870d8168aa6550837c7bd5d32d399b
Cr-Commit-Position: refs/heads/master@{#415250}
==========

[commit-bot: I haz the power, 2016-08-30 09:03:17]

Patchset 2 (id:??) landed as
https://crrev.com/df268d9d43870d8168aa6550837c7bd5d32d399b
Cr-Commit-Position: refs/heads/master@{#415250}