Actually I can only reproduce on Mac, I think.

spqchang: I don't have a Mac build set up and was wondering if this is something you might be able to take a look at. It looks to me like LocationBarViewMac::UpdateSecurityState() might not be getting called when you start typing in the credit card field, but difficult to debug without a local build.

(Also note: a prerequisite for the reproduction steps is running Chrome with the following flags:

--mark-non-secure-as=show-non-secure-passwords-cc-ui --security-chip=show-nonsecure-only

or flipping those flags in chrome://flags.)

Also, sorry I misspelled your username, spqchan :(

No worries :)

I don't have a linux machine on me. Can you show me a screencast of the expected output?

Sure, I attached screencasts of expected and actual behavior from Mac Canary 56.0.2896.0

Deleted: actual.mp4 570 KB

	actual.mp4 570 KB View Download

Deleted: expected.mp4 793 KB

	expected.mp4 793 KB View Download

Thanks! I'll look into this

Huh, I just reproduced this with passwords too, on http://http-password.badssl.com/.

Aha, I think this is a more general bug with verbose states. Here's another reproduction case that doesn't rely on passwords or credit cards:

1. Open Chrome, visit https://expird.badssl.com and click Advanced > Proceed to expired.badssl.com.
2. Close Chrome, reopen, visit https://badssl.com.
3. Open DevTools and run the following JS:
var i = document.createElement("iframe");
i.src = "https://expired.badssl.com";
document.body.appendChild(i)
4. Observe that "Not secure" does not show up in the omnibox until you click to focus the omnibox.

Looks like the omnibox isn't called to update when that happens.
Thanks for looking into this!

I dug into this and it looks like the culprit might be:
https://cs.chromium.org/chromium/src/components/security_state/security_state_model.cc?rcl=1477495967&l=28

I noticed that this is returning false on mac, which results in the security level to be set to NONE. Perhaps something is not set for mac on finch?

Note: Once I modified that function so that it'll always set the level to HTTP_SHOW_WARNING, the state appears fine on mac.

Hmm. Can you reproduce with the steps I gave in comment 9? Those steps shouldn't go through that code path at all. (There's a typo in step 1, it should be expired.badssl.com)

Strange...#9 seems to work for me on Dev and Canary

I debugged this a little bit with a build from the trybots (https://codereview.chromium.org/2452253002/). Here's what I think is happening:

1. When the password field appears and the security level drops to HTTP_SHOW_WARNING, LocationBarViewMac::Update() is called.
2. LBVMac::Update() calls UpdateSecurityState(). At this point, we have never changed |is_width_available_for_security_verbose_| from its default value of false, so UpdateSecurityState() does not call AnimateIn() or ShowWithoutAnimation() on the security chip.
3. Update() then calls OnChanged() which calls UpdateSecurityState() again (which still does nothing) followed by UpdateLocationIcon(), which calls Layout().
4. Layout() sees that ShouldShowSecurityState() is true and sets |is_width_available_for_security_verbose_| to true and calls security_state_bubble_decoration_->SetVisible(true)... but doesn't actually show or animate in the security chip. The chip only appears the next time UpdateSecurityState() is called (e.g. when you focus the omnibox, switch tabs, etc.)

Does that make any sense? I don't understand this code very well so I might be completely off... and I'm definitely not sure what the right fix is if that is the bug.

That shouldn't be it, |is_width_available_for_security_verbose_| means that if the width of the omnibox is narrow, the security verbose shouldn't appear (since there's not enough space).

What size of the browser are you testing it in?

I was testing in a full-width browser on my Macbook Air. The issue is that when UpdateSecurityState() runs, |is_width_available_for_security_verbose_| still has its default value of false; it doesn't get set to true until Layout() is called later.

(I'm not sure if the SetVisible(true) call in Layout() is supposed to make the chip actually appear, but it seems to have no effect unless it's followed by a AnimateIn() or ShowWithoutAnimation() call.)

I wonder if setting the default value to true would help? Doing that might be a bandaid to an underlying problem though. It makes sense that | is_width_available_for_security_verbose_| doesn't get set until Layout() gets called though. I'm still unable to reproduce what you're seeing at #9.

Quick question: when you set the flag to "Show all" and visit  https://badssl.com, does "Secure" shows up? Perhaps we should meet up today.

Yeah, "Secure" shows up; I think the issue occurs with verbose states that we enter dynamically after the page loads. When a state (like Secure) is applied at page load, UpdateSecurityState() gets called various times after Layout(), so the chip gets animated in. But when we enter the "not secure" state dynamically (because a password fields appears on the page), UpdateSecurityState() gets called once, then Layout() gets called, but UpdateSecurityState() doesn't run again to animate in the chip.

Friendly ping, spqchan would you be able to put together a fix for this sometime this week? (I think we talked about calling UpdateSecurityState() after Layout()?)

Sorry, I submitted a CL and it's currently blocked by the reviewer:

https://codereview.chromium.org/2456993002/

Oh, I didn't realize you had a CL out already - thanks!

No prob :)

[Mac] Fix security chip page load issue

Make sure that |is_width_available_for_security_verbose_| is always
updated when the omnibox is relayout.

BUG= 657650 

Review-Url: https://codereview.chromium.org/2456993002
Cr-Commit-Position: refs/heads/master@{#428771}

Security>UX component is deprecated in favor of the Team-Security-UX label