Unit tests for importing prefs from the Java side implemented

Unit tests for importing prefs from the Java side implemented

BUG=551029
Committed: https://crrev.com/e6f94ff0cf7d4385b92585baf41e3ab879326221
Cr-Commit-Position: refs/heads/master@{#360941}

[Alexander Agulenko, 2015-11-20 12:15:54]

Description was changed from

==========
t tests for importing prefs from the Java side implemented
Unit tests for importing prefs from the Java side implemented

BUG=551029
==========

to

==========
Unit tests for importing prefs from the Java side implemented

BUG=551029
==========

[Alexander Agulenko, 2015-11-20 12:15:54]

agulenko@google.com changed reviewers:
+ asvitkine@chromium.org, holte@chromium.org

[Alexander Agulenko, 2015-11-20 12:20:38]

Implemented unit tests for importing Java prefs from the C++ side.

A lot of changes in .gn files were caused by gn format --in-place, not sure that
they are actually needed. Do we need to revert them?

[Alexei Svitkine (slow), 2015-11-20 16:28:28]

https://codereview.chromium.org/1462953004/diff/20001/components/BUILD.gn
File components/BUILD.gn (right):

https://codereview.chromium.org/1462953004/diff/20001/components/BUILD.gn#new...
components/BUILD.gn:50: "//components/infobars/core",
These diffs don't look right.

https://codereview.chromium.org/1462953004/diff/20001/components/variations/a...
File
components/variations/android/java/src/org/chromium/components/variations/firstrun/VariationsSeedBridgeTest.java
(right):

https://codereview.chromium.org/1462953004/diff/20001/components/variations/a...
components/variations/android/java/src/org/chromium/components/variations/firstrun/VariationsSeedBridgeTest.java:18:
"seed_signature_test", "seed_country_code_test", "seed_response_date_test",
true);
Instead having a whole new Java class for all of this, can you just invoke this
function via JNI?

Then, I think you don't need all the Java and gn/gypi code for this.

https://codereview.chromium.org/1462953004/diff/20001/components/variations/v...
File components/variations/variations_seed_store_unittest.cc (right):

https://codereview.chromium.org/1462953004/diff/20001/components/variations/v...
components/variations/variations_seed_store_unittest.cc:396:
EXPECT_EQ(seed_data, "raw_seed_data_test");
Nit: Expected value should be on the left

[Alexander Agulenko, 2015-11-20 17:33:32]

https://codereview.chromium.org/1462953004/diff/20001/components/BUILD.gn
File components/BUILD.gn (right):

https://codereview.chromium.org/1462953004/diff/20001/components/BUILD.gn#new...
components/BUILD.gn:50: "//components/infobars/core",
On 2015/11/20 16:28:28, Alexei Svitkine (slow) wrote:
> These diffs don't look right.

gn format --in-place just sorted everything alphabetically. It is really
strange, and I agree to revert this file to initial state. What do yoy think?

https://codereview.chromium.org/1462953004/diff/20001/components/variations/a...
File
components/variations/android/java/src/org/chromium/components/variations/firstrun/VariationsSeedBridgeTest.java
(right):

https://codereview.chromium.org/1462953004/diff/20001/components/variations/a...
components/variations/android/java/src/org/chromium/components/variations/firstrun/VariationsSeedBridgeTest.java:18:
"seed_signature_test", "seed_country_code_test", "seed_response_date_test",
true);
On 2015/11/20 16:28:28, Alexei Svitkine (slow) wrote:
> Instead having a whole new Java class for all of this, can you just invoke
this
> function via JNI?
> 
> Then, I think you don't need all the Java and gn/gypi code for this.

Acknowledged.

https://codereview.chromium.org/1462953004/diff/20001/components/variations/v...
File components/variations/variations_seed_store_unittest.cc (right):

https://codereview.chromium.org/1462953004/diff/20001/components/variations/v...
components/variations/variations_seed_store_unittest.cc:396:
EXPECT_EQ(seed_data, "raw_seed_data_test");
On 2015/11/20 16:28:28, Alexei Svitkine (slow) wrote:
> Nit: Expected value should be on the left

Done.

[Alexander Agulenko, 2015-11-20 19:02:08]

https://codereview.chromium.org/1462953004/diff/20001/components/variations/a...
File
components/variations/android/java/src/org/chromium/components/variations/firstrun/VariationsSeedBridgeTest.java
(right):

https://codereview.chromium.org/1462953004/diff/20001/components/variations/a...
components/variations/android/java/src/org/chromium/components/variations/firstrun/VariationsSeedBridgeTest.java:18:
"seed_signature_test", "seed_country_code_test", "seed_response_date_test",
true);
On 2015/11/20 16:28:28, Alexei Svitkine (slow) wrote:
> Instead having a whole new Java class for all of this, can you just invoke
this
> function via JNI?
> 
> Then, I think you don't need all the Java and gn/gypi code for this.

Done.

[Alexei Svitkine (slow), 2015-11-20 19:10:34]

https://codereview.chromium.org/1462953004/diff/60001/components/components_t...
File components/components_tests.gyp (left):

https://codereview.chromium.org/1462953004/diff/60001/components/components_t...
components/components_tests.gyp:1: # Copyright 2014 The Chromium Authors. All
rights reserved.
Looks like you're not changing the gyp/BUILD files anymore. Can you revert your
diffs for them?

https://codereview.chromium.org/1462953004/diff/60001/components/variations/a...
File components/variations/android/variations_seed_bridge.h (right):

https://codereview.chromium.org/1462953004/diff/60001/components/variations/a...
components/variations/android/variations_seed_bridge.h:34: void
SetTestData(std::string seed_data,
Nit: SetJavaFirstRunPrefsForTesting()

https://codereview.chromium.org/1462953004/diff/60001/components/variations/a...
components/variations/android/variations_seed_bridge.h:37: std::string
response_date,
Make params "const std::string&"

[Alexander Agulenko, 2015-11-20 19:23:06]

https://codereview.chromium.org/1462953004/diff/60001/components/components_t...
File components/components_tests.gyp (left):

https://codereview.chromium.org/1462953004/diff/60001/components/components_t...
components/components_tests.gyp:1: # Copyright 2014 The Chromium Authors. All
rights reserved.
On 2015/11/20 19:10:33, Alexei Svitkine (slow) wrote:
> Looks like you're not changing the gyp/BUILD files anymore. Can you revert
your
> diffs for them?

Reverted BUILD.gn file, in this file I removed extra white space only.

[Alexander Agulenko, 2015-11-20 19:23:06]

https://codereview.chromium.org/1462953004/diff/60001/components/components_t...
File components/components_tests.gyp (left):

https://codereview.chromium.org/1462953004/diff/60001/components/components_t...
components/components_tests.gyp:1: # Copyright 2014 The Chromium Authors. All
rights reserved.
On 2015/11/20 19:10:33, Alexei Svitkine (slow) wrote:
> Looks like you're not changing the gyp/BUILD files anymore. Can you revert
your
> diffs for them?

Reverted BUILD.gn file, in this file I removed extra white space only.

[Alexander Agulenko, 2015-11-20 19:24:04]

https://codereview.chromium.org/1462953004/diff/60001/components/variations/a...
File components/variations/android/variations_seed_bridge.h (right):

https://codereview.chromium.org/1462953004/diff/60001/components/variations/a...
components/variations/android/variations_seed_bridge.h:34: void
SetTestData(std::string seed_data,
On 2015/11/20 19:10:33, Alexei Svitkine (slow) wrote:
> Nit: SetJavaFirstRunPrefsForTesting()

Done.

https://codereview.chromium.org/1462953004/diff/60001/components/variations/a...
components/variations/android/variations_seed_bridge.h:37: std::string
response_date,
On 2015/11/20 19:10:33, Alexei Svitkine (slow) wrote:
> Make params "const std::string&"

Done (shame-shame-shame).

[Alexei Svitkine (slow), 2015-11-20 19:26:34]

lgtm

[Alexander Agulenko, 2015-11-20 19:27:33]

The CQ bit was checked by agulenko@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-11-20 19:29:02]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1462953004/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1462953004/80001

[commit-bot: I haz the power, 2015-11-20 19:46:33]

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

[commit-bot: I haz the power, 2015-11-20 19:46:34]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_compile...)
  linux_android_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_android_r...)

[Alexander Agulenko, 2015-11-20 19:51:45]

The CQ bit was checked by agulenko@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-11-20 19:53:07]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1462953004/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1462953004/100001

[commit-bot: I haz the power, 2015-11-20 21:08:43]

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

[commit-bot: I haz the power, 2015-11-20 21:08:44]

Dry run: This issue passed the CQ dry run.

[Steven Holte, 2015-11-20 22:22:59]

lgtm

[Alexander Agulenko, 2015-11-20 23:02:17]

The CQ bit was checked by agulenko@google.com

[Alexander Agulenko, 2015-11-20 23:02:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/1462953004/#ps100001
(title: "Compile error fix")

[commit-bot: I haz the power, 2015-11-20 23:05:25]

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

[commit-bot: I haz the power, 2015-11-20 23:17:03]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-11-20 23:18:16]

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