New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 772832 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression : Separator line is not seen in omnibox after reloading a page.

Reported by avsha...@etouch.net, Oct 9 2017

Issue description

Chrome version : 63.0.3236.0 (Official Build) 2fa96eead8c5eea003b5b7fb4f9262b3d136d76b-refs/heads/master@{#507286} 32/64 bit
OS : Windows (7,8,10), Linux(14.04 LTS)

Test URL : www.google.co.in

What steps will reproduce the problem?
1. Launch chrome and navigate to above test URL.
2. Hit 'F6' key to move focus in omnibox and press 'Shift + tab' to shift focus on 'Secure' chip.
3. Now hit F5 key to reload the page and observe the separator line between 'Secure' chip and URL address in omnibox.

Actual Result : Separator line between 'Secure' chrome chip and URL address is not seen after reloading a page.

Expected Result : Separator line between 'Secure' chrome chip and URL address should stay visible even after reloading a page.

This is a regression issue broken in ‘M-63’, below is the manual bisect range and will soon update other info.
Good build : 63.0.3232.0
Bad build : 63.0.3233.0
 
separator_line.png
28.2 KB View Download
Actual_Result.mp4
1.8 MB View Download
Expected_Result.mp4
1.4 MB View Download
Components: UI>Browser>Omnibox>SecurityIndicators>VerboseChip
Labels: hasbisect-per-revision
Owner: spqc...@chromium.org
Status: Assigned (was: Unconfirmed)
Using the per-revision bisect providing the bisect results,
Good build : 63.0.3232.0 (Revision : 506257)
Bad build : 63.0.3233.0 (Revision : 506599)

You are probably looking for a change made after 506308 (known good), but no later than 506309 (first known bad).

CHANGELOG URL: 
https://chromium.googlesource.com/chromium/src/+log/ad84e4b20f836797b8e4a842b8d5e70f7531ba7c..8d15c77dedd4725f6ac4ec2af51826b419d8bfe2

Suspect : https://chromium.googlesource.com/chromium/src/+/8d15c77dedd4725f6ac4ec2af51826b419d8bfe2

@spqchan : Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner.

Note : 
1.Above issue is not observed on Mac(10.12.6) OS.
2. This issue can be reproduced on any page that has chrome chip in omnibox.

Thank you!
Components: -UI>Browser>Omnibox>SecurityIndicators>VerboseChip
Labels: ReleaseBlock-Stable
Tagging with blocker label, please undo if not the case.
Description: Show this description
Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 17 2017

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

commit cbee442cad90ae15b0f61418a87918addf2821c8
Author: Sarah Chan <spqchan@chromium.org>
Date: Tue Oct 17 02:29:33 2017

[Views] Fix omnibox separator issue

Currently, the separator in an omnibox bubble view
disappears when the ink drop animates in and then
reappears when it animates out. However, it's
possible for the ink drop to be removed without
animation. This causes the separator to go missing
since its opacity has not be updated.

This CL fix this issue by updating the separator's
opacity when the ink drop layer is removed.

Bug:  772832 
Change-Id: I812d8dcacd79851bf23b0dce2d7fd00330887278
Reviewed-on: https://chromium-review.googlesource.com/713398
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509218}
[modify] https://crrev.com/cbee442cad90ae15b0f61418a87918addf2821c8/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc
[modify] https://crrev.com/cbee442cad90ae15b0f61418a87918addf2821c8/chrome/browser/ui/views/location_bar/icon_label_bubble_view.h
[modify] https://crrev.com/cbee442cad90ae15b0f61418a87918addf2821c8/chrome/browser/ui/views/location_bar/icon_label_bubble_view_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 17 2017

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

commit e228732b4a18721df9398bcd0900347b331e4420
Author: Martin Šrámek <msramek@chromium.org>
Date: Tue Oct 17 10:25:47 2017

Revert "[Views] Fix omnibox separator issue"

This reverts commit cbee442cad90ae15b0f61418a87918addf2821c8.

Reason for revert: MemorySanitizer found use-of-uninitialized-value in IconLabelBubbleView::SeparatorView::UpdateOpacity(). See https://build.chromium.org/p/chromium.memory/builders/Linux%20ChromiumOS%20MSan%20Tests/builds/3587 for more details.

Original change's description:
> [Views] Fix omnibox separator issue
> 
> Currently, the separator in an omnibox bubble view
> disappears when the ink drop animates in and then
> reappears when it animates out. However, it's
> possible for the ink drop to be removed without
> animation. This causes the separator to go missing
> since its opacity has not be updated.
> 
> This CL fix this issue by updating the separator's
> opacity when the ink drop layer is removed.
> 
> Bug:  772832 
> Change-Id: I812d8dcacd79851bf23b0dce2d7fd00330887278
> Reviewed-on: https://chromium-review.googlesource.com/713398
> Reviewed-by: Scott Violet <sky@chromium.org>
> Commit-Queue: Sarah Chan <spqchan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#509218}

TBR=sky@chromium.org,spqchan@chromium.org

Change-Id: Ide2a32109cb0e3c2f7f34e2c519725a497596b33
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  772832 
Reviewed-on: https://chromium-review.googlesource.com/721444
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Commit-Queue: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509345}
[modify] https://crrev.com/e228732b4a18721df9398bcd0900347b331e4420/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc
[modify] https://crrev.com/e228732b4a18721df9398bcd0900347b331e4420/chrome/browser/ui/views/location_bar/icon_label_bubble_view.h
[modify] https://crrev.com/e228732b4a18721df9398bcd0900347b331e4420/chrome/browser/ui/views/location_bar/icon_label_bubble_view_unittest.cc

This patch broke Linux ChromiumOS MSan Tests.

See https://build.chromium.org/p/chromium.memory/builders/Linux%20ChromiumOS%20MSan%20Tests/builds/3587 - plenty of browsertests are failing with use-of-uninitialized-value.


Excerpt:

[ RUN      ] TabManagerTest.AutoDiscardable
[24959:24959:1016/210906.954059:WARNING:chrome_browser_main_chromeos.cc(620)] Running as stub user with profile dir: test-user
[24959:24959:1016/210907.879973:WARNING:install_attributes.cc(117)] Install attributes missing, first sign in
[24959:24959:1016/210908.562055:ERROR:input_method_manager_impl.cc(1031)] IMEEngine for "fgoepimhcoialccpbmpnnblemnepkkao" is not registered
[24959:25087:1016/210908.603186:WARNING:freezer_cgroup_process_manager.cc(62)] Cgroup freezer does not exist or is not writable. Unable to freeze renderer processes.
==24959==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x1cf3b33d in IconLabelBubbleView::SeparatorView::UpdateOpacity() chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc:84:7
    #1 0x1379d147 in views::InkDrop::NotifyInkDropAnimationStarted() ui/views/animation/ink_drop.cc:27:14
    #2 0x17865c7c in Run base/callback.h:92:12
    #3 0x17865c7c in CheckAllSequencesStarted ui/compositor/callback_layer_animation_observer.cc:107:0
    #4 0x17865c7c in ui::CallbackLayerAnimationObserver::SetActive() ui/compositor/callback_layer_animation_observer.cc:52:0
    #5 0x137a2195 in views::InkDropHighlight::AnimateFade(views::InkDropHighlight::AnimationType, base::TimeDelta const&, gfx::SizeF const&, gfx::SizeF const&) ui/views/animation/ink_drop_highlight.cc:152:23
    #6 0x137aa5e4 in SetHighlight ui/views/animation/ink_drop_impl.cc:789:17
    #7 0x137aa5e4 in views::InkDropImpl::NoAutoHighlightVisibleState::Enter() ui/views/animation/ink_drop_impl.cc:214:0
    #8 0x135ff4dc in views::Button::OnFocus() ui/views/controls/button/button.cc:441:20
    #9 0x136c78ba in views::FocusManager::SetFocusedViewWithReason(views::View*, views::FocusManager::FocusChangeReason) ui/views/focus/focus_manager.cc:343:20
    #10 0x136ca783 in AdvanceFocus ui/views/focus/focus_manager.cc:128:7
    #11 0x136ca783 in views::FocusManager::AdvanceFocusIfNecessary() ui/views/focus/focus_manager.cc:368:0
    #12 0x1d0d07ba in views::WebView::SetWebContents(content::WebContents*) ui/views/controls/webview/webview.cc:60:3
    #13 0x1c8e8a85 in BrowserView::OnActiveTabChanged(content::WebContents*, content::WebContents*, int, int) chrome/browser/ui/views/frame/browser_view.cc:756:25
    #14 0x1c1b908f in Browser::ActiveTabChanged(content::WebContents*, content::WebContents*, int, int) chrome/browser/ui/browser.cc:1069:12
    #15 0x1c2a46e9 in TabStripModel::NotifyIfActiveTabChanged(content::WebContents*, TabStripModel::NotifyTypes) chrome/browser/ui/tabs/tab_strip_model.cc:1291:16
    #16 0x1c2b9525 in TabStripModel::NotifyIfActiveOrSelectionChanged(content::WebContents*, TabStripModel::NotifyTypes, ui::ListSelectionModel const&) chrome/browser/ui/tabs/tab_strip_model.cc:1302:3
    #17 0x1c2a2907 in TabStripModel::SetSelection(ui::ListSelectionModel const&, TabStripModel::NotifyTypes) chrome/browser/ui/tabs/tab_strip_model.cc:1319:3
    #18 0x1c29e8e6 in TabStripModel::InsertWebContentsAt(int, content::WebContents*, int) chrome/browser/ui/tabs/tab_strip_model.cc:332:5
    #19 0x1c2af072 in TabStripModel::AddWebContents(content::WebContents*, int, ui::PageTransition, int) chrome/browser/ui/tabs/tab_strip_model.cc:785:3
    #20 0x1c2020d4 in chrome::Navigate(chrome::NavigateParams*) chrome/browser/ui/browser_navigator.cc:590:41
    #21 0x1c1c03d4 in Browser::OpenURLFromTab(content::WebContents*, content::OpenURLParams const&) chrome/browser/ui/browser.cc:1475:3
    #22 0x21c36d0 in resource_coordinator::TabManagerTest_AutoDiscardable_Test::RunTestOnMainThread() chrome/browser/resource_coordinator/tab_manager_browsertest.cc:494:14
    #23 0x1121244d in content::BrowserTestBase::ProxyRunTestOnMainThreadLoop() content/public/test/browser_test_base.cc:316:5
    #24 0xfc2068c in Run base/callback.h:92:12
    #25 0xfc2068c in ChromeBrowserMainParts::PreMainMessageLoopRunImpl() chrome/browser/chrome_browser_main.cc:1860:0
Just for completeness, the same happens on Linux MSan tests too.
https://build.chromium.org/p/chromium.memory/builders/Linux%20MSan%20Tests/builds/5091
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 18 2017

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

commit 4f62159f3a61c875e142df92019a79fae6d359db
Author: Sarah Chan <spqchan@chromium.org>
Date: Wed Oct 18 19:07:41 2017

[Views] Fix omnibox separator issue

Relanding
https://chromium-review.googlesource.com/713398

Currently, the separator in an omnibox bubble view
disappears when the ink drop animates in and then
reappears when it animates out. However, it's
possible for the ink drop to be removed without
animation. This causes the separator to go missing
since its opacity has not be updated.

This CL fix this issue by updating the separator's
opacity when the ink drop layer is removed.

Bug:  772832 
Change-Id: Ieda2a6f95328033be9509e5af644b74a974d1f90
Reviewed-on: https://chromium-review.googlesource.com/723762
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509826}
[modify] https://crrev.com/4f62159f3a61c875e142df92019a79fae6d359db/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc
[modify] https://crrev.com/4f62159f3a61c875e142df92019a79fae6d359db/chrome/browser/ui/views/location_bar/icon_label_bubble_view.h
[modify] https://crrev.com/4f62159f3a61c875e142df92019a79fae6d359db/chrome/browser/ui/views/location_bar/icon_label_bubble_view_unittest.cc

Labels: Merge-Request-63
Project Member

Comment 11 by sheriffbot@chromium.org, Oct 19 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
This change had been tested (an automated test is also added) and is low risk
Labels: -Merge-Review-63 Merge-Approved-63
Approving merge to M63 branch 3239 based on comment #12. Please merge ASAP so we can take it in for next week dev release. Thank you.
Project Member

Comment 14 by bugdroid1@chromium.org, Oct 19 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2e120493cc6824c658fa8f56852b48b0466a688a

commit 2e120493cc6824c658fa8f56852b48b0466a688a
Author: spqchan <spqchan@chromium.org>
Date: Thu Oct 19 22:47:22 2017

[Views] Fix omnibox separator issue

Relanding
https://chromium-review.googlesource.com/713398

Currently, the separator in an omnibox bubble view
disappears when the ink drop animates in and then
reappears when it animates out. However, it's
possible for the ink drop to be removed without
animation. This causes the separator to go missing
since its opacity has not be updated.

This CL fix this issue by updating the separator's
opacity when the ink drop layer is removed.

(cherry picked from commit 4f62159f3a61c875e142df92019a79fae6d359db)

Bug:  772832 
Change-Id: Ieda2a6f95328033be9509e5af644b74a974d1f90
Reviewed-on: https://chromium-review.googlesource.com/723762
Commit-Queue: Sarah Chan <spqchan@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#509826}
Reviewed-on: https://chromium-review.googlesource.com/729522
Reviewed-by: Sarah Chan <spqchan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#90}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/2e120493cc6824c658fa8f56852b48b0466a688a/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc
[modify] https://crrev.com/2e120493cc6824c658fa8f56852b48b0466a688a/chrome/browser/ui/views/location_bar/icon_label_bubble_view.h
[modify] https://crrev.com/2e120493cc6824c658fa8f56852b48b0466a688a/chrome/browser/ui/views/location_bar/icon_label_bubble_view_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment