"RBS" as this is P1 blocking MacViews launch.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/26399bca111241fe779fcbcfaf2047faac85deb6

commit 26399bca111241fe779fcbcfaf2047faac85deb6
Author: Peter Boström <pbos@chromium.org>
Date: Thu Jul 19 03:48:54 2018

Match security chip visibility with focus ring

Instantly hides or shows the IconLabelBubbleView separator when focus
rings are available to prevent showing the separator when a focus ring
overlaps it.

Bug:  chromium:865029 
Change-Id: I21a4c296c1da06b81dc91d66101d46e8ad52ad99
Reviewed-on: https://chromium-review.googlesource.com/1142442
Commit-Queue: Peter Boström <pbos@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576358}
[modify] https://crrev.com/26399bca111241fe779fcbcfaf2047faac85deb6/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc
[modify] https://crrev.com/26399bca111241fe779fcbcfaf2047faac85deb6/chrome/browser/ui/views/location_bar/icon_label_bubble_view.h

Fixed?

Yes, the CL description is a bit misleading, the separator goes away instantly on focus but otherwise follows the inkdrop, so inkdrop hover effects are still able to hide it.

[Auto-generated comment by a script] We noticed that this issue is targeted for M-69; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-69 label, otherwise remove Merge-TBD label. Thanks.

Branch hasn't happened yet per abdulsyed@, removing label.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e72294616387f89cb2036f962cc99f2d3a6ea05b

commit e72294616387f89cb2036f962cc99f2d3a6ea05b
Author: Timothy Loh <timloh@chromium.org>
Date: Mon Jul 23 04:55:19 2018

Revert "Match security chip visibility with focus ring"

This reverts commit 26399bca111241fe779fcbcfaf2047faac85deb6.

Reason for revert: OmniboxViewViewsTest.SelectAllOnTabToFocus is Flaky

Original change's description:
> Match security chip visibility with focus ring
> 
> Instantly hides or shows the IconLabelBubbleView separator when focus
> rings are available to prevent showing the separator when a focus ring
> overlaps it.
> 
> Bug:  chromium:865029 
> Change-Id: I21a4c296c1da06b81dc91d66101d46e8ad52ad99
> Reviewed-on: https://chromium-review.googlesource.com/1142442
> Commit-Queue: Peter Boström <pbos@chromium.org>
> Reviewed-by: Bret Sepulveda <bsep@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#576358}

TBR=pbos@chromium.org,bsep@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  865029 ,  866230 
Change-Id: I30372e90c1613b33690b43dd28fbe151abddddef
Reviewed-on: https://chromium-review.googlesource.com/1146421
Reviewed-by: Timothy Loh <timloh@chromium.org>
Commit-Queue: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577110}
[modify] https://crrev.com/e72294616387f89cb2036f962cc99f2d3a6ea05b/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc
[modify] https://crrev.com/e72294616387f89cb2036f962cc99f2d3a6ea05b/chrome/browser/ui/views/location_bar/icon_label_bubble_view.h

Reopening since this got reverted.

tommycli@ has agreed to take this over. Thanks!

PlatformStyle::kPreferFocusRings = true; in platform_style.cc is enough to trigger it if you need to repro it outside of Mac. Then focus it by F6-ing until you get to the location bar, then shift-tab back to the security chip.

I use http://example.com as a non-https site that shows "not secure |" with separator after. I think crrev.com/c/1142442/ is functionally correct but is crashy:

OnFocus: Hide separator instantly
OnBlur: Follow focus ring
otherwise: Follow focus ring

I don't know why setting layer opacity from parent's OnFocus ends up being crashy.

M69 branch #3497, branched at chromium revision #576753. Pls request a merge for revert listed at #8 if needed. Thank you.

Requested in issue 866036.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3c83f323d614760aba156aece59a9ac53864c78b

commit 3c83f323d614760aba156aece59a9ac53864c78b
Author: Timothy Loh <timloh@chromium.org>
Date: Mon Jul 23 18:28:45 2018

Revert "Match security chip visibility with focus ring"

This reverts commit 26399bca111241fe779fcbcfaf2047faac85deb6.

Reason for revert: OmniboxViewViewsTest.SelectAllOnTabToFocus is Flaky

Original change's description:
> Match security chip visibility with focus ring
>
> Instantly hides or shows the IconLabelBubbleView separator when focus
> rings are available to prevent showing the separator when a focus ring
> overlaps it.
>
> Bug:  chromium:865029 
> Change-Id: I21a4c296c1da06b81dc91d66101d46e8ad52ad99
> Reviewed-on: https://chromium-review.googlesource.com/1142442
> Commit-Queue: Peter Boström <pbos@chromium.org>
> Reviewed-by: Bret Sepulveda <bsep@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#576358}

TBR=pbos@chromium.org,bsep@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  865029 ,  866230 
Change-Id: I30372e90c1613b33690b43dd28fbe151abddddef
Reviewed-on: https://chromium-review.googlesource.com/1146421
Reviewed-by: Timothy Loh <timloh@chromium.org>
Commit-Queue: Timothy Loh <timloh@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#577110}(cherry picked from commit e72294616387f89cb2036f962cc99f2d3a6ea05b)
Reviewed-on: https://chromium-review.googlesource.com/1147081
Reviewed-by: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#17}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/3c83f323d614760aba156aece59a9ac53864c78b/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc
[modify] https://crrev.com/3c83f323d614760aba156aece59a9ac53864c78b/chrome/browser/ui/views/location_bar/icon_label_bubble_view.h

Revert is already merged to M69 branch 3497 at #13, is there anything else pending? If not, can this be marked as fixed?

govind: We need to improve the fix, reland, and merge the relanded fix to M69.

Thank you  tommycli@, pls request a merge to M69 when ready.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d0616ceb1713b4b8441efe9679026226de6f2bbc

commit d0616ceb1713b4b8441efe9679026226de6f2bbc
Author: Tommy C. Li <tommycli@chromium.org>
Date: Tue Jul 24 16:58:37 2018

Reland "Match security chip visibility with focus ring"

This is a reland of 26399bca111241fe779fcbcfaf2047faac85deb6

It simplifies the layer creation-and-destruction lifecycle, because
that aspect seemed the most suspicious for causing crashes.

Original change's description:
> Match security chip visibility with focus ring
>
> Instantly hides or shows the IconLabelBubbleView separator when focus
> rings are available to prevent showing the separator when a focus ring
> overlaps it.
>
> Bug:  chromium:865029 
> Change-Id: I21a4c296c1da06b81dc91d66101d46e8ad52ad99
> Reviewed-on: https://chromium-review.googlesource.com/1142442
> Commit-Queue: Peter Boström <pbos@chromium.org>
> Reviewed-by: Bret Sepulveda <bsep@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#576358}

Bug:  chromium:865029 
Change-Id: I7b5b597a6e6ec114faf4888b62da1f8751e64646
Reviewed-on: https://chromium-review.googlesource.com/1147361
Reviewed-by: Peter Boström <pbos@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577587}
[modify] https://crrev.com/d0616ceb1713b4b8441efe9679026226de6f2bbc/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc
[modify] https://crrev.com/d0616ceb1713b4b8441efe9679026226de6f2bbc/chrome/browser/ui/views/location_bar/icon_label_bubble_view.h

Note for govind/TPM: We're not ready to request a merge here. This change needs to bake for at least a couple of days in Canary.

Thank you, pls update bug with canary result on Friday, 07/27. 

Tried checking the issue on version 69.0.3495.0(...expecting this version without fix) and on the latest canary 70.0.3502.0 (with fix) on Mac 10.13.1 by enabling [MacViews] and navigating to a URL having security chip.

Our Observations:
----------------
In 69.0.3495.0 we didn't observe any separator between security chip and rest of the URL
From 69.0.3497.0 version separator is seen and it disappeared on hovering/focused, similar behaviour is seen on the latest version 70.0.3502.0 too.

As we are not very sure about the expected behaviour, could some one help us in verifying the fix and let us know if we have missed anything in the process.

Thanks!

Here are the repro steps:

1. Navigate to google.com. The separator should be visible.

2. Click on the URL in the Omnibox. The separator should still be visible.

3. Press Shift-tab. Now the lock icon should have a blue focus ring.

4. Before the fix, you will see the separator still drawn below the focus ring. After the fix, the separator should be gone.

5. Press Tab again. Now the separator should fade back in.

Note to reproduce with c#22, you need to have Full Keyboard accessibility on: 
https://support.apple.com/en-us/HT204434#fullkeyboard

Thank you Tommy, Verified the fix based on steps provided in comment#22 and 23 on Chrome version 70.0.3502.0.

I was able to verify the fix in Canary as well.

We will wait a bit to see if crash reports before merging, as the previous fix was also valid, but caused crashes.

Thank you  pbommana@ and tommycli@. 

Tommy, pls request a merge to M69 when you feel comfortable with canary data.

The NextAction date has arrived: 2018-07-27

Per discussion with pbos@ we will wait until Monday to make extra sure we have resolved the crashiness.

The NextAction date has arrived: 2018-07-30

How is the change looking in canary? If it looks good and ready for M69 merge, pls request a merge to M69. Thank you.

Last crash in 70.0.3500.0, change went in 70.0.3502.3

https://crash.corp.google.com/browse?q=EXISTS+%28SELECT+1+FROM+UNNEST%28CrashedStackTrace.StackFrame%29+WHERE+FunctionName%3D%27non-virtual+thunk+to+IconLabelBubbleView%3A%3AOnFocus%28%29%27%29#-property-selector,samplereports,productname:1000,+productversion,-magicsignature:50,-magicsignature2:50,-stablesignature:50,-magicsignaturesorted:50

This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Approving merge to M69 branch 3497 based on comment #33. Please merge ASAP. Thank you.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/39bf66d6e8059320d01ff3dd08e2bb226fabe557

commit 39bf66d6e8059320d01ff3dd08e2bb226fabe557
Author: Tommy C. Li <tommycli@chromium.org>
Date: Mon Jul 30 18:53:59 2018

Reland "Match security chip visibility with focus ring"

This is a reland of 26399bca111241fe779fcbcfaf2047faac85deb6

It simplifies the layer creation-and-destruction lifecycle, because
that aspect seemed the most suspicious for causing crashes.

Original change's description:
> Match security chip visibility with focus ring
>
> Instantly hides or shows the IconLabelBubbleView separator when focus
> rings are available to prevent showing the separator when a focus ring
> overlaps it.
>
> Bug:  chromium:865029 
> Change-Id: I21a4c296c1da06b81dc91d66101d46e8ad52ad99
> Reviewed-on: https://chromium-review.googlesource.com/1142442
> Commit-Queue: Peter Boström <pbos@chromium.org>
> Reviewed-by: Bret Sepulveda <bsep@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#576358}

Bug:  chromium:865029 
Change-Id: I7b5b597a6e6ec114faf4888b62da1f8751e64646
Reviewed-on: https://chromium-review.googlesource.com/1147361
Reviewed-by: Peter Boström <pbos@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#577587}(cherry picked from commit d0616ceb1713b4b8441efe9679026226de6f2bbc)
Reviewed-on: https://chromium-review.googlesource.com/1155056
Cr-Commit-Position: refs/branch-heads/3497@{#225}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/39bf66d6e8059320d01ff3dd08e2bb226fabe557/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc
[modify] https://crrev.com/39bf66d6e8059320d01ff3dd08e2bb226fabe557/chrome/browser/ui/views/location_bar/icon_label_bubble_view.h

Can this be marked as fixed if nothing else is pending?

Tried  Verifying the issue on chrome version 69.0.3497.23 using Mac 10.13.1 with the exact steps mentioned in comment#22 & #23 by enabling [MacViews]. After navigating to google.com we didn't observe any separator. Attaching the screencast of the steps performed for reference.
Note: Full Keyboard accessibility is turned ON while checking.

@Tommy C. Li: Could you please have a look at the screen cast and let us know if we have missed anything in the process and requesting you to help us in verifying the fix.

Thanks!

Preconditions to verify this issue :
1. you need to have Full Keyboard accessibility on: 
https://support.apple.com/en-us/HT204434#fullkeyboard
2. Disable chrome://flags/#omnibox-ui-jog-textfield-on-popup(On M69)
3. Enable Use all upcoming UI features.


Cases verified :
1. Navigate to google.com. The separator should be visible.

2. Click on the URL in the Omnibox. The separator should still be visible.

3. Press Shift-tab. Now the lock icon should have a blue focus ring.

4. Before the fix, you will see the separator still drawn below the focus ring. After the fix, the separator should be gone.

5. Press Tab again. Now the separator should fade back in.

+abdulsyed@ fyi, M69 merges taken for Proj-MdRefresh .