Add unit tests for Signin Header Helpers.

Add unit tests for Signin Header Helpers.

BUG=649389
Committed: https://crrev.com/1c2ec6ccadd007b2066c8ec575ed346e659cd471
Cr-Commit-Position: refs/heads/master@{#420634}

[bzanotti, 2016-09-22 17:27:59]

Description was changed from

==========
Add unit tests for Signin Header Helpers.

BUG=
==========

to

==========
Add unit tests for Signin Header Helpers.

BUG=649389
==========

[bzanotti, 2016-09-22 17:28:56]

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

[bzanotti, 2016-09-22 17:29:20]

bzanotti@chromium.org changed reviewers:
+ rogerta@chromium.org

[bzanotti, 2016-09-22 17:29:20]

Please take a look.

As promised, the unit tests. :)

[commit-bot: I haz the power, 2016-09-22 17:29:47]

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-09-22 17:57:51]

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

[commit-bot: I haz the power, 2016-09-22 17:57:52]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Roger Tawa OOO till Jul 10th, 2016-09-22 19:19:53]

Thanks Benoit :-)  All looks good, see below.  I think there is a missing test
based on our discussion with Dominic.  Do you want to ask him if he wants it? 
I'll give lgtm though since we are in different timezones and maybe my comments
don't apply.

https://codereview.chromium.org/2360733002/diff/40001/components/signin/core/...
File components/signin/core/browser/signin_header_helper_unittest.cc (right):

https://codereview.chromium.org/2360733002/diff/40001/components/signin/core/...
components/signin/core/browser/signin_header_helper_unittest.cc:120:
TEST_F(SigninHeaderHelperTest, TestMirrorRequestDrive) {
Maybe DCHECK that consistency is off at start of test, since enable overrides
disable.

https://codereview.chromium.org/2360733002/diff/40001/components/signin/core/...
components/signin/core/browser/signin_header_helper_unittest.cc:176: }
According to Dominic, if both the original URL and the redirect URL are not
eligible, and the request already contains the header, it should not be
stripped.  Can we add a test for that?

[bzanotti, 2016-09-23 11:44:00]

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

[commit-bot: I haz the power, 2016-09-23 11:44:05]

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-09-23 12:19:47]

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

[commit-bot: I haz the power, 2016-09-23 12:19:48]

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

[bzanotti, 2016-09-23 14:18:10]

https://codereview.chromium.org/2360733002/diff/40001/components/signin/core/...
File components/signin/core/browser/signin_header_helper_unittest.cc (right):

https://codereview.chromium.org/2360733002/diff/40001/components/signin/core/...
components/signin/core/browser/signin_header_helper_unittest.cc:120:
TEST_F(SigninHeaderHelperTest, TestMirrorRequestDrive) {
On 2016/09/22 19:19:52, Roger Tawa wrote:
> Maybe DCHECK that consistency is off at start of test, since enable overrides
> disable.

I think the CommandLine is reset before each tests, but let's make sure of that.
Done. :)

https://codereview.chromium.org/2360733002/diff/40001/components/signin/core/...
components/signin/core/browser/signin_header_helper_unittest.cc:176: }
On 2016/09/22 19:19:52, Roger Tawa wrote:
> According to Dominic, if both the original URL and the redirect URL are not
> eligible, and the request already contains the header, it should not be
> stripped.  Can we add a test for that?

Done.

[bzanotti, 2016-09-23 14:18:18]

The CQ bit was checked by bzanotti@chromium.org

[bzanotti, 2016-09-23 14:18:19]

The patchset sent to the CQ was uploaded after l-g-t-m from rogerta@chromium.org
Link to the patchset: https://codereview.chromium.org/2360733002/#ps60001
(title: "Add DCHECK and new test")

[commit-bot: I haz the power, 2016-09-23 14:18:29]

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-09-23 14:37:28]

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

[commit-bot: I haz the power, 2016-09-23 14:37:29]

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

[bzanotti, 2016-09-23 14:38:13]

The CQ bit was checked by bzanotti@chromium.org

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

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-09-23 14:48:20]

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

[commit-bot: I haz the power, 2016-09-23 14:48:21]

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

[bzanotti, 2016-09-23 15:49:58]

The CQ bit was checked by bzanotti@chromium.org

[commit-bot: I haz the power, 2016-09-23 15:50:16]

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-09-23 16:43:41]

Description was changed from

==========
Add unit tests for Signin Header Helpers.

BUG=649389
==========

to

==========
Add unit tests for Signin Header Helpers.

BUG=649389
==========

[commit-bot: I haz the power, 2016-09-23 16:43:42]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-23 16:45:11]

Description was changed from

==========
Add unit tests for Signin Header Helpers.

BUG=649389
==========

to

==========
Add unit tests for Signin Header Helpers.

BUG=649389

Committed: https://crrev.com/1c2ec6ccadd007b2066c8ec575ed346e659cd471
Cr-Commit-Position: refs/heads/master@{#420634}
==========

[commit-bot: I haz the power, 2016-09-23 16:45:12]

Patchset 4 (id:??) landed as
https://crrev.com/1c2ec6ccadd007b2066c8ec575ed346e659cd471
Cr-Commit-Position: refs/heads/master@{#420634}