New issue
Advanced search Search tips

Issue 840657 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

OmniboxPopupContentsViewTest.ThemeIntegration tests failing mash browser tests on linux-chromeos-rel CQ bot

Project Member Reported by noel@chromium.org, May 8 2018

Issue description

Let me revert [1] and see if it helps.
Project Member

Comment 2 by bugdroid1@chromium.org, May 8 2018

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

commit a6598e5db8a6aada00c8ffc6d8000ef8638e12b3
Author: Kunihiko Sakamoto <ksakamoto@chromium.org>
Date: Tue May 08 05:35:37 2018

Revert "Use GG700 / Chrome icon color for toolbar"

This reverts commit b5d6fee994cc84be26d06b03ce6a38bb4e665587.

Reason for revert: Speculative revert for mash_browser_tests failure on linux-chromeos-rel
https://ci.chromium.org/buildbot/chromium.chromiumos/linux-chromeos-rel/7672

Original change's description:
> Use GG700 / Chrome icon color for toolbar
> 
> Replaces GG800 with GG700 as icon color in a couple of places:
> 
> * Omnibox default chip (HTTP (i) icon for instance).
> * BubbleIconView inside omnibar (these used to be GG700 but regressed
>   to GG800 in Touchable / Refresh recently).
> * Toolbar button icons.
> 
> Bug: chromium:821996,  chromium:834510 
> Change-Id: I9e953c9da1e0eee1da132e3e0e71474ff7877aef
> Reviewed-on: https://chromium-review.googlesource.com/1040875
> Commit-Queue: Peter Boström <pbos@chromium.org>
> Reviewed-by: Peter Kasting <pkasting@chromium.org>
> Reviewed-by: Evan Stade <estade@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#556646}

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

Change-Id: I1d56fd212cbe1ed50b620b61b9287cfa8abc5b48
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:821996,  chromium:834510 ,  chromium:840657 
Reviewed-on: https://chromium-review.googlesource.com/1049445
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556700}
[modify] https://crrev.com/a6598e5db8a6aada00c8ffc6d8000ef8638e12b3/chrome/browser/themes/theme_properties.cc
[modify] https://crrev.com/a6598e5db8a6aada00c8ffc6d8000ef8638e12b3/chrome/browser/ui/omnibox/omnibox_theme.cc
[modify] https://crrev.com/a6598e5db8a6aada00c8ffc6d8000ef8638e12b3/chrome/browser/ui/views/location_bar/bubble_icon_view.cc
[modify] https://crrev.com/a6598e5db8a6aada00c8ffc6d8000ef8638e12b3/chrome/browser/ui/views/location_bar/bubble_icon_view.h
[modify] https://crrev.com/a6598e5db8a6aada00c8ffc6d8000ef8638e12b3/chrome/browser/ui/views/location_bar/find_bar_icon.cc
[modify] https://crrev.com/a6598e5db8a6aada00c8ffc6d8000ef8638e12b3/chrome/browser/ui/views/location_bar/find_bar_icon.h
[modify] https://crrev.com/a6598e5db8a6aada00c8ffc6d8000ef8638e12b3/chrome/browser/ui/views/location_bar/intent_picker_view.cc
[modify] https://crrev.com/a6598e5db8a6aada00c8ffc6d8000ef8638e12b3/chrome/browser/ui/views/location_bar/intent_picker_view.h
[modify] https://crrev.com/a6598e5db8a6aada00c8ffc6d8000ef8638e12b3/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/a6598e5db8a6aada00c8ffc6d8000ef8638e12b3/chrome/browser/ui/views/location_bar/location_bar_view.h
[modify] https://crrev.com/a6598e5db8a6aada00c8ffc6d8000ef8638e12b3/chrome/browser/ui/views/location_bar/star_view.cc
[modify] https://crrev.com/a6598e5db8a6aada00c8ffc6d8000ef8638e12b3/chrome/browser/ui/views/location_bar/star_view.h
[modify] https://crrev.com/a6598e5db8a6aada00c8ffc6d8000ef8638e12b3/chrome/browser/ui/views/location_bar/zoom_view.cc
[modify] https://crrev.com/a6598e5db8a6aada00c8ffc6d8000ef8638e12b3/chrome/browser/ui/views/location_bar/zoom_view.h
[modify] https://crrev.com/a6598e5db8a6aada00c8ffc6d8000ef8638e12b3/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc
[modify] https://crrev.com/a6598e5db8a6aada00c8ffc6d8000ef8638e12b3/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.h
[modify] https://crrev.com/a6598e5db8a6aada00c8ffc6d8000ef8638e12b3/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view_browsertest.cc

Comment 3 by noel@chromium.org, May 8 2018

Cc: -pbos@chromium.org ksakamoto@chromium.org
Owner: pbos@chromium.org
Status: Assigned (was: Untriaged)
Reproduced locally.

 ./out/Debug/browser_tests \
     --gtest_filter="OmniboxPopupContentsViewTest.ThemeIntegration*" \
     --enable-features=Mash

 > Cr-Commit-Position: refs/heads/master@{#556454} good
 > Cr-Commit-Position: refs/heads/master@{#556646} bad

So yeap, crrev.com/556646 was the cause.




Thanks for checking!

Comment 5 by noel@chromium.org, May 8 2018

NO worries, saved a log (attached).
840657.log.txt
28.8 KB View Download
Project Member

Comment 6 by bugdroid1@chromium.org, May 9 2018

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

commit acb195073e0f1d65cd2df44f8e129104262ce53e
Author: Peter Boström <pbos@chromium.org>
Date: Wed May 09 01:57:39 2018

Reland "Use GG700 / Chrome icon color for toolbar"

This reverts commit a6598e5db8a6aada00c8ffc6d8000ef8638e12b3.

Reason for revert: Partial reland that doesn't remove caching of tint_.

Original change's description:
> Revert "Use GG700 / Chrome icon color for toolbar"
>
> This reverts commit b5d6fee994cc84be26d06b03ce6a38bb4e665587.
>
> Reason for revert: Speculative revert for mash_browser_tests failure on linux-chromeos-rel
> https://ci.chromium.org/buildbot/chromium.chromiumos/linux-chromeos-rel/7672
>
> Original change's description:
> > Use GG700 / Chrome icon color for toolbar
> >
> > Replaces GG800 with GG700 as icon color in a couple of places:
> >
> > * Omnibox default chip (HTTP (i) icon for instance).
> > * BubbleIconView inside omnibar (these used to be GG700 but regressed
> >   to GG800 in Touchable / Refresh recently).
> > * Toolbar button icons.
> >
> > Bug: chromium:821996,  chromium:834510 
> > Change-Id: I9e953c9da1e0eee1da132e3e0e71474ff7877aef
> > Reviewed-on: https://chromium-review.googlesource.com/1040875
> > Commit-Queue: Peter Boström <pbos@chromium.org>
> > Reviewed-by: Peter Kasting <pkasting@chromium.org>
> > Reviewed-by: Evan Stade <estade@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#556646}
>
> TBR=pkasting@chromium.org,estade@chromium.org,pbos@chromium.org,bsep@chromium.org
>
> Change-Id: I1d56fd212cbe1ed50b620b61b9287cfa8abc5b48
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: chromium:821996,  chromium:834510 ,  chromium:840657 
> Reviewed-on: https://chromium-review.googlesource.com/1049445
> Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
> Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#556700}

TBR=pkasting@chromium.org,estade@chromium.org,bsep@chromium.org

Change-Id: I280c016c9cf772ce5919c29ebee4dba8535e44e3
Bug: chromium:821996,  chromium:834510 ,  chromium:840657 
Reviewed-on: https://chromium-review.googlesource.com/1050486
Commit-Queue: Peter Boström <pbos@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557060}
[modify] https://crrev.com/acb195073e0f1d65cd2df44f8e129104262ce53e/chrome/browser/themes/theme_properties.cc
[modify] https://crrev.com/acb195073e0f1d65cd2df44f8e129104262ce53e/chrome/browser/ui/omnibox/omnibox_theme.cc
[modify] https://crrev.com/acb195073e0f1d65cd2df44f8e129104262ce53e/chrome/browser/ui/views/location_bar/bubble_icon_view.cc
[modify] https://crrev.com/acb195073e0f1d65cd2df44f8e129104262ce53e/chrome/browser/ui/views/location_bar/bubble_icon_view.h
[modify] https://crrev.com/acb195073e0f1d65cd2df44f8e129104262ce53e/chrome/browser/ui/views/location_bar/find_bar_icon.cc
[modify] https://crrev.com/acb195073e0f1d65cd2df44f8e129104262ce53e/chrome/browser/ui/views/location_bar/find_bar_icon.h
[modify] https://crrev.com/acb195073e0f1d65cd2df44f8e129104262ce53e/chrome/browser/ui/views/location_bar/intent_picker_view.cc
[modify] https://crrev.com/acb195073e0f1d65cd2df44f8e129104262ce53e/chrome/browser/ui/views/location_bar/intent_picker_view.h
[modify] https://crrev.com/acb195073e0f1d65cd2df44f8e129104262ce53e/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/acb195073e0f1d65cd2df44f8e129104262ce53e/chrome/browser/ui/views/location_bar/location_bar_view.h
[modify] https://crrev.com/acb195073e0f1d65cd2df44f8e129104262ce53e/chrome/browser/ui/views/location_bar/star_view.cc
[modify] https://crrev.com/acb195073e0f1d65cd2df44f8e129104262ce53e/chrome/browser/ui/views/location_bar/star_view.h
[modify] https://crrev.com/acb195073e0f1d65cd2df44f8e129104262ce53e/chrome/browser/ui/views/location_bar/zoom_view.cc
[modify] https://crrev.com/acb195073e0f1d65cd2df44f8e129104262ce53e/chrome/browser/ui/views/location_bar/zoom_view.h

Comment 7 by noel@chromium.org, May 9 2018

Status: Fixed (was: Assigned)
Results looking good in browser_test & browser_test (with patch) on the bots. 

Sign in to add a comment