New issue
Advanced search Search tips

Issue 859718 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 3
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

"TabTest.HitTestTopPixel" is flaky

Project Member Reported by chromium...@appspot.gserviceaccount.com, Jul 3

Issue description

"TabTest.HitTestTopPixel" is flaky.

This issue was created automatically by the chromium-try-flakes app. Please find the right owner to fix the respective test/step and assign this issue to them. If the step/test is infrastructure-related, please add Infra-Troopers label and change issue status to Untriaged. When done, please remove the issue from Sheriff Bug Queue by removing the Sheriff-Chromium label.

We have detected 4 recent flakes. List of all flakes can be found at https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyIgsSBUZsYWtlIhdUYWJUZXN0LkhpdFRlc3RUb3BQaXhlbAw.

Flaky tests should be disabled within 30 minutes unless culprit CL is found and reverted. Please see more details here: https://sites.google.com/a/chromium.org/dev/developers/tree-sheriffs/sheriffing-bug-queues#triaging-auto-filed-flakiness-bugs
 
Cc: thestig@chromium.org pkasting@chromium.org
It's actually failing a few tests together:
TabTest.LayeredThrobber
TabTest.TooltipProvidedByTab
TabTest.HitTestTopPixel
TabTest.LayoutAndVisibilityOfElements
TabTest.CloseButtonLayout
TabTest.TitleHiddenWhenSmall
TabTest.CloseButtonFocus

It's crashing on a DCHECK:
---
[ RUN      ] TabTest.HitTestTopPixel
[4616:4616:0702/135504.300284:1112718500:FATAL:chrome_layout_provider.cc(34)] Check failed: g_chrome_layout_provider == views::LayoutProvider::Get() ((nil) vs. 0x282f5e225470)
#0 0x0000070b869c base::debug::StackTrace::StackTrace()
#1 0x000007021ecb logging::LogMessage::~LogMessage()
#2 0x0000096d6ec8 ChromeLayoutProvider::Get()
#3 0x0000097c5959 Tab::GetContentsInsets()
#4 0x0000097c5658 Tab::Tab()
#5 0x000003c51ba2 TabTest_HitTestTopPixel_Test::TestBody()
#6 0x000003e880e2 testing::Test::Run()
#7 0x000003e88ca0 testing::TestInfo::Run()
#8 0x000003e891b7 testing::TestCase::Run()
#9 0x000003e94a97 testing::internal::UnitTestImpl::RunAllTests()
#10 0x000003e9460d testing::UnitTest::Run()
#11 0x0000066e2b71 base::TestSuite::Run()
#12 0x0000066e527d base::(anonymous namespace)::LaunchUnitTestsInternal()
#13 0x0000066e50d1 base::LaunchUnitTests()
#14 0x0000066d876a main
#15 0x7fd7af08df45 __libc_start_main
#16 0x00000234f02a _start

[8479/8514] TabTest.HitTestTopPixel (CRASHED)
---

The issue seems to have started ~3h ago which correlates with this CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1121787
----


On further checking, these are all tests from TabTest since that CL changed the base class it's very likely the culprit.

Proceeding to revert it.
That CL was meant to prevent this precise problem, which would otherwise be caused by other Tab changes.  The failure is saying a (non-Chrome) Views LayoutProvider has already been set up by the time someone is asking for the Chrome one.

Reverting is likely to turn this confusing flakiness into a guaranteed failure on all bots.
Cc: bsep@chromium.org
r572070 just landed with the revert...
right, I'll watch next few builds on dashboard and re-land if it gets worst.

https://ci.chromium.org/p/chromium/g/chromium/console
pkasting@ noted that all reports from try-flakes are actually from his attempt on his CL which didn't have the proper dependency on other CLs.

So, it's better to reland the original CL, proceeding to that.
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 3

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

commit ef166a2cea68934ecfb9c66e15b77c1fce0d6279
Author: Luciano Pacheco <lucmult@chromium.org>
Date: Tue Jul 03 01:58:06 2018

Reland "Convert chrome/ tests using ViewsTestBase to ChromeViewsTestBase."

This reverts commit cf25f1427cb867f8b5de93ea54341abacc59ecdf.

Reason for revert:  crbug.com/859718  was a false alarm, failures was
before merging the CL originally reverted.

Original change's description:
> Revert "Convert chrome/ tests using ViewsTestBase to ChromeViewsTestBase."
> 
> This reverts commit ed47f31beff24c6610486dbc958948cee7a68fd3.
> 
> Reason for revert: All tests from TabTest reported flaky  crbug.com/859718 
> 
> Original change's description:
> > Convert chrome/ tests using ViewsTestBase to ChromeViewsTestBase.
> > 
> > This ensures these tests are testing the actual harmony/refresh behavior, and
> > makes sure as people add more calls to ChromeLayoutProvider to the code they
> > don't result in unexpected test crashes.
> > 
> > Bug: none
> > Change-Id: If21b7eec387dc47be448e2dbb2730ddacb7b51fe
> > Reviewed-on: https://chromium-review.googlesource.com/1121787
> > Commit-Queue: Peter Kasting <pkasting@chromium.org>
> > Reviewed-by: Lei Zhang <thestig@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#572008}
> 
> TBR=pkasting@chromium.org,thestig@chromium.org
> 
> Change-Id: Ib0d68d4aaff4a8617b95f4456eef0dad9f66e57b
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: none
> Reviewed-on: https://chromium-review.googlesource.com/1123839
> Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
> Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#572070}

TBR=pkasting@chromium.org,thestig@chromium.org,lucmult@chromium.org

Change-Id: I7c738232c08a506baa0ebf43d1a4df87fa2ca2f9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  859718 
Reviewed-on: https://chromium-review.googlesource.com/1123859
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572082}
[modify] https://crrev.com/ef166a2cea68934ecfb9c66e15b77c1fce0d6279/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge_unittest.cc
[modify] https://crrev.com/ef166a2cea68934ecfb9c66e15b77c1fce0d6279/chrome/browser/media/webrtc/native_desktop_media_list_unittest.cc
[modify] https://crrev.com/ef166a2cea68934ecfb9c66e15b77c1fce0d6279/chrome/browser/ui/views/autofill/autofill_popup_view_native_views_unittest.cc
[modify] https://crrev.com/ef166a2cea68934ecfb9c66e15b77c1fce0d6279/chrome/browser/ui/views/download/download_item_view_unittest.cc
[modify] https://crrev.com/ef166a2cea68934ecfb9c66e15b77c1fce0d6279/chrome/browser/ui/views/frame/desktop_linux_browser_frame_view_layout_unittest.cc
[modify] https://crrev.com/ef166a2cea68934ecfb9c66e15b77c1fce0d6279/chrome/browser/ui/views/frame/opaque_browser_frame_view_layout_unittest.cc
[modify] https://crrev.com/ef166a2cea68934ecfb9c66e15b77c1fce0d6279/chrome/browser/ui/views/location_bar/icon_label_bubble_view_unittest.cc
[modify] https://crrev.com/ef166a2cea68934ecfb9c66e15b77c1fce0d6279/chrome/browser/ui/views/omnibox/omnibox_result_view_unittest.cc
[modify] https://crrev.com/ef166a2cea68934ecfb9c66e15b77c1fce0d6279/chrome/browser/ui/views/payments/validating_textfield_unittest.cc
[modify] https://crrev.com/ef166a2cea68934ecfb9c66e15b77c1fce0d6279/chrome/browser/ui/views/payments/view_stack_unittest.cc
[modify] https://crrev.com/ef166a2cea68934ecfb9c66e15b77c1fce0d6279/chrome/browser/ui/views/sync/bubble_sync_promo_view_unittest.cc
[modify] https://crrev.com/ef166a2cea68934ecfb9c66e15b77c1fce0d6279/chrome/browser/ui/views/tabs/tab_unittest.cc
[modify] https://crrev.com/ef166a2cea68934ecfb9c66e15b77c1fce0d6279/chrome/test/views/accessibility_checker_unittest.cc

Labels: -Sheriff-Chromium
Owner: lucmult@chromium.org
Status: Fixed (was: Untriaged)
Reverted the revert:
https://chromium-review.googlesource.com/c/chromium/src/+/1123859

Gerrit was smart enough to detect it as a "reland" of original. :-)

Marking this as fixed and I'll report this situation to Try-Flakes team, since this seems a false positive that we can automatically detect.
Reported Try-Flakes issue: crbug.com/859763
 Issue 859739  has been merged into this issue.

Sign in to add a comment