predictors: Store protobufs in the database.

predictors: Store protobufs in the database.

This is the first step in the conversion of the database to a protocol
buffer based storage format. This introduces a protocol buffer based
format for storage. A forthcoming CL will remove ResourceRow.

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

[Benoit L, 2016-08-22 14:42:14]

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

[Benoit L, 2016-08-22 14:43:01]

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

[commit-bot: I haz the power, 2016-08-22 14:49:38]

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

[Benoit L, 2016-08-22 15:00:13]

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

[commit-bot: I haz the power, 2016-08-22 15:00:29]

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-22 15:49:48]

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

[commit-bot: I haz the power, 2016-08-22 15:49:49]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[pasko, 2016-08-22 15:51:28]

overall looks good, one question about the tool below

https://codereview.chromium.org/2263243002/diff/20001/chrome/browser/BUILD.gn
File chrome/browser/BUILD.gn (right):

https://codereview.chromium.org/2263243002/diff/20001/chrome/browser/BUILD.gn...
chrome/browser/BUILD.gn:992: if (!is_ios) {
just curious: is it because we cannot have protos on ios at all or something
more subtle?

https://codereview.chromium.org/2263243002/diff/20001/tools/resource_prefetch...
File tools/resource_prefetch_predictor/prefetch_predictor_tool.py (left):

https://codereview.chromium.org/2263243002/diff/20001/tools/resource_prefetch...
tools/resource_prefetch_predictor/prefetch_predictor_tool.py:93: print
Entry.HEADER
it was in CSV format, so I guess there was value of importing it as a CSV, but
now it is going to be something that:
a) only lists a part of data
b) contains newlines and does not contain a CSV header

I am probably OK with (b), even though I do not see reasoning behind it, but it
seems like we still want to be able to see all the data in the database, so (a)
is a problem.

Am I missing something?

[Benoit L, 2016-08-23 09:13:52]

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

[commit-bot: I haz the power, 2016-08-23 09:14:20]

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

[Benoit L, 2016-08-23 09:46:05]

Thanks!

https://codereview.chromium.org/2263243002/diff/20001/chrome/browser/BUILD.gn
File chrome/browser/BUILD.gn (right):

https://codereview.chromium.org/2263243002/diff/20001/chrome/browser/BUILD.gn...
chrome/browser/BUILD.gn:992: if (!is_ios) {
On 2016/08/22 15:51:28, pasko wrote:
> just curious: is it because we cannot have protos on ios at all or something
> more subtle?

The code is not used on iOS, and that's the way the other proto library
declarations are done in this file.

https://codereview.chromium.org/2263243002/diff/20001/tools/resource_prefetch...
File tools/resource_prefetch_predictor/prefetch_predictor_tool.py (left):

https://codereview.chromium.org/2263243002/diff/20001/tools/resource_prefetch...
tools/resource_prefetch_predictor/prefetch_predictor_tool.py:93: print
Entry.HEADER
On 2016/08/22 15:51:28, pasko wrote:
> it was in CSV format, so I guess there was value of importing it as a CSV, but
> now it is going to be something that:
> a) only lists a part of data
> b) contains newlines and does not contain a CSV header
> 
> I am probably OK with (b), even though I do not see reasoning behind it, but
it
> seems like we still want to be able to see all the data in the database, so
(a)
> is a problem.
> 
> Am I missing something?

The previous format was just there because it was simple. The new format is
actually a lot better, as the enum values are clearly shown.

Example:

score: 122.0
main_page_url: www.hardware.fr
confidence: 1.000000"
primary_key: ""
resource_url:
"http://www.hardware.fr/js/SocialSharePrivacy/stylesheets/modules/twitter.css"
resource_type: RESOURCE_TYPE_STYLESHEET
number_of_hits: 2
number_of_misses: 0
consecutive_misses: 0
average_position: 78.0
priority: REQUEST_PRIORITY_IDLE
has_validators: true
always_revalidate: false
score: 122.0
main_page_url: www.hardware.fr
confidence: 1.000000"
primary_key: ""
resource_url:
"http://www.hardware.fr/js/SocialSharePrivacy/stylesheets/modules/twitter.css"
resource_type: RESOURCE_TYPE_STYLESHEET
number_of_hits: 2
number_of_misses: 0
consecutive_misses: 0
average_position: 78.0
priority: REQUEST_PRIORITY_IDLE
has_validators: true
always_revalidate: false

[commit-bot: I haz the power, 2016-08-23 11:29:22]

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

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

Dry run: This issue passed the CQ dry run.

[pasko, 2016-08-23 11:41:20]

lgtm, agreed that the output of the tool makes sense

[Benoit L, 2016-08-23 11:45:07]

+jochen@ for OWNERS review of the BUILD.gn change.
PTAL, thanks!

[Benoit L, 2016-08-23 11:46:03]

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

[Benoit L, 2016-08-23 11:46:03]

+jochen@ for OWNERS review of the BUILD.gn change.
PTAL, thanks!

[jochen (gone - plz use gerrit), 2016-08-23 12:00:23]

lgtm

[Benoit L, 2016-08-23 12:00:44]

The CQ bit was checked by lizeb@chromium.org

[commit-bot: I haz the power, 2016-08-23 12:01:03]

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-23 12:04:42]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-23 12:06:21]

Description was changed from

==========
predictors: Store protobufs in the database.

This is the first step in the conversion of the database to a protocol
buffer based storage format. This introduces a protocol buffer based
format for storage. A forthcoming CL will remove ResourceRow.

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

to

==========
predictors: Store protobufs in the database.

This is the first step in the conversion of the database to a protocol
buffer based storage format. This introduces a protocol buffer based
format for storage. A forthcoming CL will remove ResourceRow.

BUG=631966

Committed: https://crrev.com/60cd6e859b9f557d2312f5bf532f6aec5f284980
Cr-Commit-Position: refs/heads/master@{#413716}
==========

[commit-bot: I haz the power, 2016-08-23 12:06:22]

Patchset 3 (id:??) landed as
https://crrev.com/60cd6e859b9f557d2312f5bf532f6aec5f284980
Cr-Commit-Position: refs/heads/master@{#413716}