Support delta-compressed variations server responses.

Support delta-compressed variations server responses.

Makes the client send the "A-IM: x-bm" header to the server to
specify that it supports deltas in x-bm format and then makes it
process such deltas when received from the server (when the
response has the "IM: x-bm" header).

If a delta encoded response fails to apply, tracks the failure mode
in the existing store seed histogram and triggers a re-fetch of the
full response.

Also, changes the "If-Match" header sent that specifies the seed
SN to instead be "If-None-Match" to correctly match the semantics.
(The server now supports both.)

BUG=514699
Committed: https://crrev.com/b24f45595e1f29fd3ff04ea0c53ef14785ce9289
Cr-Commit-Position: refs/heads/master@{#342273}

[Alexei Svitkine (slow), 2015-07-21 17:59:55]

Patchset #5 (id:80001) has been deleted

[Alexei Svitkine (slow), 2015-07-21 18:00:17]

Patchset #4 (id:60001) has been deleted

[Alexei Svitkine (slow), 2015-07-28 15:30:56]

Patchset #3 (id:40001) has been deleted

[Alexei Svitkine (slow), 2015-07-28 15:31:01]

Patchset #2 (id:20001) has been deleted

[Alexei Svitkine (slow), 2015-07-28 15:31:08]

Patchset #5 (id:160001) has been deleted

[Alexei Svitkine (slow), 2015-07-28 15:31:13]

Patchset #3 (id:120001) has been deleted

[Alexei Svitkine (slow), 2015-07-28 15:31:19]

Patchset #1 (id:1) has been deleted

[Alexei Svitkine (slow), 2015-07-28 15:31:25]

Patchset #2 (id:140001) has been deleted

[Alexei Svitkine (slow), 2015-07-28 17:17:32]

Patchset #3 (id:200001) has been deleted

[Alexei Svitkine (slow), 2015-07-28 17:17:37]

Patchset #2 (id:180001) has been deleted

[Alexei Svitkine (slow), 2015-07-28 17:17:42]

Patchset #1 (id:100001) has been deleted

[Alexei Svitkine (slow), 2015-07-28 17:17:50]

Patchset #2 (id:240001) has been deleted

[Alexei Svitkine (slow), 2015-07-28 17:17:56]

Patchset #1 (id:220001) has been deleted

[Alexei Svitkine (slow), 2015-07-28 17:21:40]

Patchset #1 (id:260001) has been deleted

[Alexei Svitkine (slow), 2015-07-30 16:22:58]

Patchset #1 (id:280001) has been deleted

[Alexei Svitkine (slow), 2015-08-03 20:48:30]

Patchset #2 (id:320001) has been deleted

[Alexei Svitkine (slow), 2015-08-03 20:48:35]

Patchset #3 (id:360001) has been deleted

[Alexei Svitkine (slow), 2015-08-03 20:48:48]

Patchset #2 (id:340001) has been deleted

[Alexei Svitkine (slow), 2015-08-03 20:48:53]

Patchset #2 (id:380001) has been deleted

[Alexei Svitkine (slow), 2015-08-03 20:49:03]

Patchset #2 (id:400001) has been deleted

[Alexei Svitkine (slow), 2015-08-03 20:56:24]

Patchset #2 (id:420001) has been deleted

[Alexei Svitkine (slow), 2015-08-03 21:41:22]

Patchset #1 (id:300001) has been deleted

[Alexei Svitkine (slow), 2015-08-03 21:42:21]

asvitkine@chromium.org changed reviewers:
+ rkaplow@chromium.org

[Alexei Svitkine (slow), 2015-08-03 21:42:21]

Rob, PTAL. Feel free to take your time on this, since it's not yet urgent. I
still need to do some end to end testing which is currently blocked on some
server-side changes. But otherwise, I think this is just about ready.

[Alexei Svitkine (slow), 2015-08-04 21:50:19]

Added an extra test and a couple more histograms for us to measure the benefits
of this.

Also, finished testing it end to end with a local server. (And it works.)

[rkaplow, 2015-08-04 22:37:05]

https://codereview.chromium.org/1200233005/diff/460001/chrome/browser/metrics...
File chrome/browser/metrics/variations/variations_seed_store.cc (right):

https://codereview.chromium.org/1200233005/diff/460001/chrome/browser/metrics...
chrome/browser/metrics/variations/variations_seed_store.cc:87:
VARIATIONS_SEED_STORE_DELTA_COUNT,
DELTA_COUNT is a bit unlike the rest (not really a result). Seems fine to keep
it in this histogram but maybe comment on it here to explain what it is about.

https://codereview.chromium.org/1200233005/diff/460001/chrome/browser/metrics...
chrome/browser/metrics/variations/variations_seed_store.cc:230: //
TODO(asvitkine): Move this method down to match declaration order in a
did you leave it as is as to make the diff more readable? You are truly a
gentlemen.

https://codereview.chromium.org/1200233005/diff/460001/chrome/browser/metrics...
chrome/browser/metrics/variations/variations_seed_store.cc:415: if (value == 0)
{
hm, maybe I'm not familiar enough with data diffs, but I didn't know about this
pattern of a 0, followed by a offset,length to determine which parts to get from
original data.
Might be worth a little bit of english so it's obvious for the reader. If you
could also link to where this is defined that would be great.

https://codereview.chromium.org/1200233005/diff/460001/chrome/browser/metrics...
File chrome/browser/metrics/variations/variations_seed_store.h (right):

https://codereview.chromium.org/1200233005/diff/460001/chrome/browser/metrics...
chrome/browser/metrics/variations/variations_seed_store.h:35: // Stores the
given seed data (serialized protobuf) to local state, along
nit, would change to |data| too then

https://codereview.chromium.org/1200233005/diff/460001/chrome/browser/metrics...
chrome/browser/metrics/variations/variations_seed_store.h:38: // compressed and
attempts to decode it first using the store seed data.
did you mean stored btw?

https://codereview.chromium.org/1200233005/diff/460001/chrome/browser/metrics...
chrome/browser/metrics/variations/variations_seed_store.h:45: const std::string&
country_code,
might be worth to add a line in the comment on how country_code interacts.

[Alexei Svitkine (slow), 2015-08-05 15:46:15]

Thanks, comments addressed.

https://codereview.chromium.org/1200233005/diff/460001/chrome/browser/metrics...
File chrome/browser/metrics/variations/variations_seed_store.cc (right):

https://codereview.chromium.org/1200233005/diff/460001/chrome/browser/metrics...
chrome/browser/metrics/variations/variations_seed_store.cc:87:
VARIATIONS_SEED_STORE_DELTA_COUNT,
On 2015/08/04 22:37:05, rkaplow wrote:
> DELTA_COUNT is a bit unlike the rest (not really a result). Seems fine to keep
> it in this histogram but maybe comment on it here to explain what it is about.

Done.

https://codereview.chromium.org/1200233005/diff/460001/chrome/browser/metrics...
chrome/browser/metrics/variations/variations_seed_store.cc:230: //
TODO(asvitkine): Move this method down to match declaration order in a
On 2015/08/04 22:37:05, rkaplow wrote:
> did you leave it as is as to make the diff more readable? You are truly a
> gentlemen.

Yep. :)

https://codereview.chromium.org/1200233005/diff/460001/chrome/browser/metrics...
chrome/browser/metrics/variations/variations_seed_store.cc:415: if (value == 0)
{
On 2015/08/04 22:37:05, rkaplow wrote:
> hm, maybe I'm not familiar enough with data diffs, but I didn't know about
this
> pattern of a 0, followed by a offset,length to determine which parts to get
from
> original data.
> Might be worth a little bit of english so it's obvious for the reader. If you
> could also link to where this is defined that would be great.

It's actually just the format of the diff produced by the "bm" library we use.
This is its re-implementation in C++.

I've reversed the if and added comments to both branches to explain the format
more.

https://codereview.chromium.org/1200233005/diff/460001/chrome/browser/metrics...
File chrome/browser/metrics/variations/variations_seed_store.h (right):

https://codereview.chromium.org/1200233005/diff/460001/chrome/browser/metrics...
chrome/browser/metrics/variations/variations_seed_store.h:35: // Stores the
given seed data (serialized protobuf) to local state, along
On 2015/08/04 22:37:05, rkaplow wrote:
> nit, would change to |data| too then

Done.

https://codereview.chromium.org/1200233005/diff/460001/chrome/browser/metrics...
chrome/browser/metrics/variations/variations_seed_store.h:38: // compressed and
attempts to decode it first using the store seed data.
On 2015/08/04 22:37:05, rkaplow wrote:
> did you mean stored btw?

No meant, store's - i.e. refererring to this class which is a
VariationsSeedStore. Added an apostrophe - let me know if it's better now or
whether you prefer that I rephrase it more.

https://codereview.chromium.org/1200233005/diff/460001/chrome/browser/metrics...
chrome/browser/metrics/variations/variations_seed_store.h:45: const std::string&
country_code,
On 2015/08/04 22:37:05, rkaplow wrote:
> might be worth to add a line in the comment on how country_code interacts.

Done.

[rkaplow, 2015-08-05 18:01:56]

lgtm

https://codereview.chromium.org/1200233005/diff/460001/chrome/browser/metrics...
File chrome/browser/metrics/variations/variations_seed_store.h (right):

https://codereview.chromium.org/1200233005/diff/460001/chrome/browser/metrics...
chrome/browser/metrics/variations/variations_seed_store.h:38: // compressed and
attempts to decode it first using the store seed data.
On 2015/08/05 15:46:15, Alexei Svitkine wrote:
> On 2015/08/04 22:37:05, rkaplow wrote:
> > did you mean stored btw?
> 
> No meant, store's - i.e. refererring to this class which is a
> VariationsSeedStore. Added an apostrophe - let me know if it's better now or
> whether you prefer that I rephrase it more.

it's fine - wasn't sure which way you meant it.

[Alexei Svitkine (slow), 2015-08-06 14:49:13]

The CQ bit was checked by asvitkine@chromium.org

[commit-bot: I haz the power, 2015-08-06 14:49:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1200233005/500001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1200233005/500001

[commit-bot: I haz the power, 2015-08-06 15:02:38]

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

[commit-bot: I haz the power, 2015-08-06 15:02:39]

Try jobs failed on following builders:
  mac_chromium_10.10_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_10.1...)

[Alexei Svitkine (slow), 2015-08-06 17:40:33]

Patchset #5 (id:520001) has been deleted

[Alexei Svitkine (slow), 2015-08-06 19:39:40]

Uploaded a new patchset with a few fixes:
  - Fixed compile following rebase and also expanded the test that this rebased
on to test more cases.
  - Fixed a missing return statement after I added a histogram in patch set 3.

Will cq - up to you if you want to look again.

[Alexei Svitkine (slow), 2015-08-06 19:40:00]

The CQ bit was checked by asvitkine@chromium.org

[Alexei Svitkine (slow), 2015-08-06 19:40:01]

The patchset sent to the CQ was uploaded after l-g-t-m from rkaplow@chromium.org
Link to the patchset: https://codereview.chromium.org/1200233005/#ps560001
(title: "Fixing missing return in !is_compressed case.")

[commit-bot: I haz the power, 2015-08-06 19:40:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1200233005/560001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1200233005/560001

[commit-bot: I haz the power, 2015-08-06 20:27:40]

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

[commit-bot: I haz the power, 2015-08-06 20:27:41]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Alexei Svitkine (slow), 2015-08-06 20:56:39]

The CQ bit was checked by asvitkine@chromium.org

[commit-bot: I haz the power, 2015-08-06 20:57:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1200233005/560001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1200233005/560001

[commit-bot: I haz the power, 2015-08-06 22:01:55]

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

[commit-bot: I haz the power, 2015-08-06 22:01:56]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Alexei Svitkine (slow), 2015-08-07 02:16:39]

The CQ bit was checked by asvitkine@chromium.org

[commit-bot: I haz the power, 2015-08-07 02:17:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1200233005/560001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1200233005/560001

[commit-bot: I haz the power, 2015-08-07 02:57:37]

Committed patchset #6 (id:560001)

[commit-bot: I haz the power, 2015-08-07 02:58:22]

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