|
|
DescriptionDon't record re-auths, unlocks, or secondary account additions in Signin.SigninStartedAccessPoint
BUG=632052
Committed: https://crrev.com/bbea865be4f600c35292aff11dc6e6f34232578e
Cr-Commit-Position: refs/heads/master@{#422969}
Patch Set 1 #
Total comments: 1
Messages
Total messages: 17 (10 generated)
Description was changed from ========== Don't record re-auths, unlocks, or secondary account additions in Signin.SigninStartedAccessPoint BUG= ========== to ========== Don't record re-auths, unlocks, or secondary account additions in Signin.SigninStartedAccessPoint BUG=632052 ==========
gogerald@chromium.org changed reviewers: + rogerta@chromium.org
Hi, PTAL,
The CQ bit was checked by gogerald@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by gogerald@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Don't record re-auths, unlocks, or secondary account additions in Signin.SigninStartedAccessPoint BUG=632052 ========== to ========== Don't record re-auths, unlocks, or secondary account additions in Signin.SigninStartedAccessPoint BUG=632052 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Don't record re-auths, unlocks, or secondary account additions in Signin.SigninStartedAccessPoint BUG=632052 ========== to ========== Don't record re-auths, unlocks, or secondary account additions in Signin.SigninStartedAccessPoint BUG=632052 Committed: https://crrev.com/bbea865be4f600c35292aff11dc6e6f34232578e Cr-Commit-Position: refs/heads/master@{#422969} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/bbea865be4f600c35292aff11dc6e6f34232578e Cr-Commit-Position: refs/heads/master@{#422969}
Message was sent while issue was closed.
brucedawson@chromium.org changed reviewers: + brucedawson@chromium.org
Message was sent while issue was closed.
https://codereview.chromium.org/2384233002/diff/1/chrome/browser/ui/webui/sig... File chrome/browser/ui/webui/signin/inline_login_handler.cc (right): https://codereview.chromium.org/2384233002/diff/1/chrome/browser/ui/webui/sig... chrome/browser/ui/webui/signin/inline_login_handler.cc:203: reason != signin_metrics::Reason::REASON_ADD_SECONDARY_ACCOUNT) { Tautology error detected. |reason| will always be not equal to *one* of those values. I assume you meant "==" instead of "!=" or "&&" instead of "||". This was discovered by the experimental VC++ /analyze static analyze bot. Warning was: src\chrome\browser\ui\webui\signin\inline_login_handler.cc(201) : warning C6289: Incorrect operator: mutual exclusion over || is always a non-zero constant. Did you intend to use && instead? Does this also point out a gap in test coverage? Tracking bug for /analyze found errors is 427616.
Message was sent while issue was closed.
On 2016/10/11 20:22:01, brucedawson wrote: > https://codereview.chromium.org/2384233002/diff/1/chrome/browser/ui/webui/sig... > File chrome/browser/ui/webui/signin/inline_login_handler.cc (right): > > https://codereview.chromium.org/2384233002/diff/1/chrome/browser/ui/webui/sig... > chrome/browser/ui/webui/signin/inline_login_handler.cc:203: reason != > signin_metrics::Reason::REASON_ADD_SECONDARY_ACCOUNT) { > Tautology error detected. |reason| will always be not equal to *one* of those > values. I assume you meant "==" instead of "!=" or "&&" instead of "||". > > This was discovered by the experimental VC++ /analyze static analyze bot. > Warning was: > > src\chrome\browser\ui\webui\signin\inline_login_handler.cc(201) : warning C6289: > Incorrect operator: mutual exclusion over || is always a non-zero constant. > Did you intend to use && instead? > > Does this also point out a gap in test coverage? > > Tracking bug for /analyze found errors is 427616. Found this problem, fixing in this CL https://codereview.chromium.org/2411883002/ |