Simplify origin logic in login_handler

Simplify origin logic in login_handler

This code had logic for continuing execution after a NOTREACHED(),
which is against the style guide. With this removed the code is much
simpler.

Committed: https://crrev.com/e406acaf0d5e19c1f13161f0e2c7d552329dfebe
Cr-Commit-Position: refs/heads/master@{#424840}

[Charlie Harrison, 2016-10-05 04:51:43]

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

[commit-bot: I haz the power, 2016-10-05 04:51:55]

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-10-05 05:59:42]

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

[commit-bot: I haz the power, 2016-10-05 05:59:43]

Dry run: This issue passed the CQ dry run.

[Charlie Harrison, 2016-10-05 11:58:21]

csharrison@chromium.org changed reviewers:
+ asanka@chromium.org

[Charlie Harrison, 2016-10-05 11:58:22]

asanka, is this reasonable? Should we CHECK this condition? I tried tracing the
code to see if this will ever be false and didn't see anything obvious.

[Charlie Harrison, 2016-10-05 12:31:56]

Description was changed from

==========
Simplify origin logic in login_handler

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

to

==========
Simplify origin logic in login_handler

This code had logic for continuing execution after a NOTREACHED(),
which is against the style guide. With this removed the code is much
simpler.
==========

[asanka, 2016-10-05 13:35:58]

On 2016/10/05 at 11:58:22, csharrison wrote:
> asanka, is this reasonable? Should we CHECK this condition? I tried tracing
the code to see if this will ever be false and didn't see anything obvious.

Yeah, a DCHECK is fine to leave in. Looks like the logic was left over from an
old bug hunt.

LGTM

[Charlie Harrison, 2016-10-05 13:55:07]

csharrison@chromium.org changed reviewers:
+ vabr@chromium.org

[Charlie Harrison, 2016-10-05 13:55:07]

Thanks, yeah that was my take as well. I was just nervous that there might be
security implications because this is login code.

+vabr for OWNERS and a double-check.

[vabr (Chromium), 2016-10-06 09:25:30]

On 2016/10/05 13:55:07, Charlie Harrison wrote:
> Thanks, yeah that was my take as well. I was just nervous that there might be
> security implications because this is login code.
> 
> +vabr for OWNERS and a double-check.

Sorry for the delayed response, I had some unexpected long meetings yesterday.

LGTM! The DCHECK is equivalent to the NOTREACHED before, and the lack of
handling it will only surface if there is actually a bug. In that case,
surfacing that bug is a good thing.

Cheers,
Vaclav

[Charlie Harrison, 2016-10-06 11:55:50]

The CQ bit was checked by csharrison@chromium.org

[Charlie Harrison, 2016-10-06 11:55:57]

The CQ bit was unchecked by csharrison@chromium.org

[Charlie Harrison, 2016-10-12 19:43:10]

The CQ bit was checked by csharrison@chromium.org

[commit-bot: I haz the power, 2016-10-12 19:43:38]

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-10-12 20:38:06]

Description was changed from

==========
Simplify origin logic in login_handler

This code had logic for continuing execution after a NOTREACHED(),
which is against the style guide. With this removed the code is much
simpler.
==========

to

==========
Simplify origin logic in login_handler

This code had logic for continuing execution after a NOTREACHED(),
which is against the style guide. With this removed the code is much
simpler.
==========

[commit-bot: I haz the power, 2016-10-12 20:38:08]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-10-12 20:40:15]

Description was changed from

==========
Simplify origin logic in login_handler

This code had logic for continuing execution after a NOTREACHED(),
which is against the style guide. With this removed the code is much
simpler.
==========

to

==========
Simplify origin logic in login_handler

This code had logic for continuing execution after a NOTREACHED(),
which is against the style guide. With this removed the code is much
simpler.

Committed: https://crrev.com/e406acaf0d5e19c1f13161f0e2c7d552329dfebe
Cr-Commit-Position: refs/heads/master@{#424840}
==========

[commit-bot: I haz the power, 2016-10-12 20:40:16]

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