New issue
Advanced search Search tips

Issue 875707 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 25
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug



Sign in to add a comment

interactive_ui_tests failing on chromium.mac/Mac10.13 Tests (dbg)

Project Member Reported by sheriff-...@appspot.gserviceaccount.com, Aug 20

Issue description

Filed by sheriff-o-matic@appspot.gserviceaccount.com on behalf of haraken@chromium.org

interactive_ui_tests failing on chromium.mac/Mac10.13 Tests (dbg)

Builders failed on: 
- Mac10.13 Tests (dbg): 
  https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.13%20Tests%20%28dbg%29

BrowserViewTest.TabFullscreenShowTopView has been flakily failing. Let me disable the test for now.

 
Owner: weili@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 20

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

commit 6f360d69ccfdd7fcd2d4c91759cec5cfb50ee03f
Author: Kentaro Hara <haraken@chromium.org>
Date: Mon Aug 20 04:33:53 2018

Disable BrowserViewTest.TabFullscreenShowTopView

This test has been flaky on Mac10.13 Tests

TBR=weili@chromium.org

Bug:  875707 
Change-Id: I71e1f70fb1e7628ba810dccf603d4d58885abb9c
Reviewed-on: https://chromium-review.googlesource.com/1180798
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584359}
[modify] https://crrev.com/6f360d69ccfdd7fcd2d4c91759cec5cfb50ee03f/chrome/browser/ui/views/frame/browser_view_interactive_uitest.cc

 Issue 875719  has been merged into this issue.
Labels: -Sheriff-Chromium
Status: Assigned (was: Available)
According to the flakiness dashboard, most of these tests are failing on Mac only:
https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=interactive_ui_tests&tests=BrowserViewTest

Therefore, will disable the flakies (and reenable tests for Linux/Win if possible) with https://crrev.com/c/1180975.

Example failure for BrowserFullscreenShowTopView and FullscreenShowBookmarkBar:
[56240:775:0820/034142.092112:FATAL:browser_non_client_frame_view.cc(110)] Check failed: browser_view_->IsTabStripVisible().

No culprit found where this could have started - but the flakiness produces a great deal of noise on all Mac bots.

Most recent build with multiple failing tests:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac%20ASan%2064%20Tests%20%281%29/43050
Components: Tests>Flaky UI>Browser
Labels: OS-Mac Type-Bug
Cc: robliao@chromium.org
cc robliao@, were you working on Mac UI tests?
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 20

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

commit 1a150ace90ffa7e5bcc633897b3f95379b15f5df
Author: Friedrich Horschig <fhorschig@chromium.org>
Date: Mon Aug 20 12:08:19 2018

[Flaky][Mac] Disable BrowserViewTests on Mac only

This disables all BrowserViewTests that haven't been disabled yet.
Because these failures have always been Mac-only, reenable one disabled
tests for non-Mac platforms.

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

Bug:  875707 
Change-Id: Ie0a24b706669d07577bf2413a61bd959d70950cc
Reviewed-on: https://chromium-review.googlesource.com/1180975
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584409}
[modify] https://crrev.com/1a150ace90ffa7e5bcc633897b3f95379b15f5df/chrome/browser/ui/views/frame/browser_view_interactive_uitest.cc

Cc: bsep@chromium.org pkasting@chromium.org
The first time interactive_ui_tests started failing was https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.13%20Tests%20%28dbg%29/4760 

Looks like the cause is due to change https://chromium-review.googlesource.com/c/chromium/src/+/1173755 That change wired up calling BrowserNonClientFrameView::HasVisibleBackgroundTabShapes() whose DCHECK() on the very first line could be tripped if the tab_strip_ in BrowserView is not set up yet.

This can not be fixed by simply removing the DCHECK here since the tab_strip_ pointer will be used in later part of code. I am looping in the original CL owner and reviewer for a better way to fix this.

Labels: -Pri-2 Pri-1
Hi!

I'm trying to write a test for a security fix, and I'm hitting

[51486:775:0822/113728.962956:FATAL:browser_non_client_frame_view.cc(110)] Check failed: browser_view_->IsTabStripVisible(). 

on the Mac. Can we get some traction here?
Cc: weili@chromium.org
Owner: pkasting@chromium.org
Assigning to pkasting as he wrote the CL.
This causes non-test DCHECKs when going into fullscreen. Does this revert cleanly?
Pausing the CQ on the revert by request of Peter.

Repro for this DCHECK:
1. Go to youtube.com
2. Click a video
3. Click the fullscreen button

[52799:1295:0822/125842.969688:FATAL:browser_non_client_frame_view.cc(110)] Check failed: browser_view_->IsTabStripVisible(). 

Status: Started (was: Assigned)
Wei and I are investigating.
Cc: -weili@chromium.org
Owner: weili@chromium.org
Project Member

Comment 15 by bugdroid1@chromium.org, Aug 24

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

commit f6cda363d36318e10be7fe4350bf2e9e895182ed
Author: Wei Li <weili@chromium.org>
Date: Fri Aug 24 21:50:22 2018

Avoid paint during fullscreen transition on Mac

During browser fullscreen transition, we have been avoiding doing layout
to prevent jankiness. On Mac, during such fullscreen transition,
CA transaction updates thus full re-paint still happen due to window resize.
This results in transient paint with obsolete layout. For example, if
Chrome is entering content fullscreen mode such as trying to play a video
in fullscreen, BrowserView::IsTabStripVisible() would return false since the
tabstrip should not be shown, but the tab strip is still being painted.

To solve the above problem, and also avoid unnecessary operations, this
CL suppresses CA transaction updates during fullscreen transition. Since
after the fullscreen transition, Chrome will immediately re-layout and
re-paint, the updates during the transition are not needed. This CL also
enables the tests that previously failed due to the state discrepancy
problem.

BUG= 875707 

Change-Id: I89510fcf55afc47e407d78bdf60a0c5a03ad9e57
Reviewed-on: https://chromium-review.googlesource.com/1188035
Commit-Queue: Wei Li <weili@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586003}
[modify] https://crrev.com/f6cda363d36318e10be7fe4350bf2e9e895182ed/chrome/browser/ui/views/frame/browser_view_interactive_uitest.cc
[modify] https://crrev.com/f6cda363d36318e10be7fe4350bf2e9e895182ed/ui/views/cocoa/bridged_native_widget.mm

Status: Fixed (was: Started)
I think this should be fixed.

Sign in to add a comment