Redirect handling in the resource_prefetch_predictor.

chrome/browser/predictors/resource_prefetch_predictor.proto

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Protocol buffers used for storing in SQLite.
 // CAUTION: When any change is done here, bump kDatabaseVersion in
 //          resource_prefetch_predictor.h.

[Benoit L, 2016/09/26 11:18:36]

Oops, can you correct my mistake here as well?

This is resource_prefetch_predictor_tables.h...

[alexilin, 2016/09/26 12:14:54]

Done.

 
 syntax = "proto2";
 
 package chrome_browser_predictors;
 
 // Required in Chrome.
 option optimize_for = LITE_RUNTIME;
 
 message Metadata {
   optional string main_page_url = 1;
@@ skipping 36 matching lines @@
   optional string resource_url = 1;
   optional ResourceType resource_type = 2;
   optional uint32 number_of_hits = 3;
   optional uint32 number_of_misses = 4;
   optional uint32 consecutive_misses = 5;
   optional double average_position = 6;
   optional Priority priority = 7;
   optional bool has_validators = 8;
   optional bool always_revalidate = 9;
 }
+
+message RedirectData {
+  // Represents single redirect chain endpoint

[Benoit L, 2016/09/26 11:18:36]

nit: Represents a single redirect chain endpoint.

[pasko, 2016/09/26 12:28:15]

.. except that it is a mapping of origin to all possible endpoints, right?

[alexilin, 2016/09/26 15:38:28]

This comment refers to RedirectStat message, not RedirectData.

[pasko, 2016/09/26 15:58:22]

Ah oh, agreed. I was confused because of lack of comment for the RedirectData.
Also, an empty line above this comment would help identifying where it
attributes to.

[alexilin, 2016/09/27 14:52:45]

Added comment for the RedirectData. 'git cl format' doesn't allow to add an
empty line above this comment.
Done.

+  message RedirectStat {
+    optional string url = 1;
+    optional uint32 number_of_hits = 2;
+    optional uint32 number_of_misses = 3;
+    optional uint32 consecutive_misses = 4;
+  }
+
+  optional string primary_key = 1;
+  optional uint64 last_visit_time = 2;
+  repeated RedirectStat redirects = 3;

[pasko, 2016/09/22 14:27:09]

This proto would benefit from comments and probably renaming of this field. Is
|redirects| a redirect chain or a collection of endpoints in all redirect chains
for primary_key?

[alexilin, 2016/09/22 16:48:19]

Collection of endpoints.
Ok, let's agree on the names before I'll make painful renaming.

redirects -> endpoints ?

Also, it could be reasonable to rename RedirectStat::url field because it could
be a host as well (when we use host-based prefetching). Rename it to 'key'?

[pasko, 2016/09/26 12:28:15]

I would suggest redirect_endpoints
I'd prefer |primary_key| for consistency, but I agree it is weird to have
primary keys in a proto repeated field. We can also invent a new name for
url-or-host, ideas:
* url_or_host
* hrl
* target

.. 'host' is probably also a bad name because we care about scheme+host+port,
which is effectively an 'origin', but this renaming is certainly *not* for this
change.

WDYT?

[alexilin, 2016/09/27 14:52:45]

Renamed redirects -> redirect_endpoints.
Leave further renaming for next reviews.
Done.

+}