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

Issue 810416 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

SadTab::RecordFirstPaint DCHECK on Mac-10.9

Project Member Reported by mcnee@chromium.org, Feb 8 2018

Issue description

OS: Mac-10.9

Many tests that crash/kill a tab are flaking because of this DCHECK on Mac-10.9.

Example:
[ RUN      ] ZoomControllerBrowserTest.CrashedTabsDoNotChangeZoom
[10393:13319:0207/110032.299826:WARNING:notification_platform_bridge_mac.mm(510)] AlertNotificationService: XPC connection invalidated.
2018-02-07 11:00:32.402 browser_tests[10393:143503] *** Owner supplied to -[NSTrackingArea initWithRect:options:owner:userInfo:] referenced a deallocating object. Tracking area behavior is undefined. Break on NSTrackingAreaDeallocatingOwnerError to debug.
[10393:775:0207/110032.673706:WARNING:sad_tab.cc(276)] Tab Killed:
[10393:775:0207/110032.686066:FATAL:sad_tab.cc(198)] Check failed: !recorded_paint_.
0   browser_tests                       0x000000010a50890c base::debug::StackTrace::StackTrace(unsigned long) + 28
1   browser_tests                       0x000000010a52d670 logging::LogMessage::~LogMessage() + 224
2   browser_tests                       0x000000010e33fd48 SadTab::RecordFirstPaint() + 88
3   AppKit                              0x00007fff523398b2 -[_NSViewBackingLayer display] + 407
4   QuartzCore                          0x00007fff5fd45d3b CA::Layer::display_if_needed(CA::Transaction*) + 633
5   QuartzCore                          0x00007fff5fd457f9 CA::Layer::layout_and_display_if_needed(CA::Transaction*) + 35
6   QuartzCore                          0x00007fff5fd44894 CA::Context::commit_transaction(CA::Transaction*) + 326
7   QuartzCore                          0x00007fff5fd4443d CA::Transaction::commit() + 487
8   AppKit                              0x00007fff52af0658 __65+[CATransaction(NSCATransaction) NS_setFlushesWithDisplayRefresh]_block_invoke + 283
9   CoreFoundation                      0x00007fff54c35127 __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 23
10  CoreFoundation                      0x00007fff54c3504f __CFRunLoopDoObservers + 527
11  CoreFoundation                      0x00007fff54c176a8 __CFRunLoopRun + 1240
12  CoreFoundation                      0x00007fff54c16f43 CFRunLoopRunSpecific + 483
13  HIToolbox                           0x00007fff53f2ee26 RunCurrentEventLoopInMode + 286
14  HIToolbox                           0x00007fff53f2eb96 ReceiveNextEventCommon + 613
15  HIToolbox                           0x00007fff53f2e914 _BlockUntilNextEventMatchingListInModeWithFilter + 64
16  AppKit                              0x00007fff521f9f5f _DPSNextEvent + 2085
17  AppKit                              0x00007fff5298fb4c -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 3044
18  browser_tests                       0x000000010a6cff60 __71-[BrowserCrApplication nextEventMatchingMask:untilDate:inMode:dequeue:]_block_invoke + 64
19  browser_tests                       0x000000010a53006a base::mac::CallWithEHFrame(void () block_pointer) + 10
20  browser_tests                       0x000000010a6cfea4 -[BrowserCrApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 164
21  AppKit                              0x00007fff521eed6d -[NSApplication run] + 764
22  browser_tests                       0x000000010a54d47c base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate*) + 364
23  browser_tests                       0x000000010a54b6de base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*) + 110
24  browser_tests                       0x000000010a548119 base::MessageLoop::Run(bool) + 169
25  browser_tests                       0x000000010a585ef9 base::RunLoop::Run() + 249
26  browser_tests                       0x000000010ae172e9 content::MessageLoopRunner::Run() + 153
27  browser_tests                       0x000000010adb7ea3 content::RenderProcessHostWatcher::Wait() + 99
28  browser_tests                       0x0000000106d9d157 ZoomControllerBrowserTest_CrashedTabsDoNotChangeZoom_Test::RunTestOnMainThread() + 231
29  browser_tests                       0x000000010adb6384 content::BrowserTestBase::ProxyRunTestOnMainThreadLoop() + 404
30  browser_tests                       0x000000010a6d6200 ChromeBrowserMainParts::PreMainMessageLoopRunImpl() + 4784
31  browser_tests                       0x000000010a6d4e4e ChromeBrowserMainParts::PreMainMessageLoopRun() + 62
32  browser_tests                       0x00000001086c86d3 content::BrowserMainLoop::PreMainMessageLoopRun() + 67
33  browser_tests                       0x0000000108c160b5 content::StartupTaskRunner::RunAllTasksNow() + 117
34  browser_tests                       0x00000001086c6cfd content::BrowserMainLoop::CreateStartupTasks() + 765
35  browser_tests                       0x00000001086cb373 content::BrowserMainRunnerImpl::Initialize(content::MainFunctionParams const&) + 99
36  browser_tests                       0x00000001086c4c22 content::BrowserMain(content::MainFunctionParams const&) + 210
37  browser_tests                       0x000000010a406445 content::ContentMainRunnerImpl::Run() + 565
38  browser_tests                       0x000000010ce8cdc2 service_manager::Main(service_manager::MainParams const&) + 2482
39  browser_tests                       0x000000010a4057d4 content::ContentMain(content::ContentMainParams const&) + 68
40  browser_tests                       0x000000010adb5fc9 content::BrowserTestBase::SetUp() + 3193
41  browser_tests                       0x000000010a62a9f9 InProcessBrowserTest::SetUp() + 505
42  browser_tests                       0x000000010790afb1 testing::Test::Run() + 97
43  browser_tests                       0x000000010790bc50 testing::TestInfo::Run() + 288
44  browser_tests                       0x000000010790c277 testing::TestCase::Run() + 263
45  browser_tests                       0x0000000107914877 testing::internal::UnitTestImpl::RunAllTests() + 903
46  browser_tests                       0x00000001079144c3 testing::UnitTest::Run() + 163
47  browser_tests                       0x000000010a64d1e7 base::TestSuite::Run() + 167
48  browser_tests                       0x000000010a4eee45 ChromeTestSuiteRunner::RunTestSuite(int, char**) + 37
49  browser_tests                       0x000000010ae1120e content::LaunchTests(content::TestLauncherDelegate*, unsigned long, int, char**) + 478
50  browser_tests                       0x000000010a4ef35c LaunchChromeTests(unsigned long, content::TestLauncherDelegate*, int, char**) + 348
51  browser_tests                       0x000000010a4eed9e main + 94
52  libdyld.dylib                       0x00007fff7c52a115 start + 1
53  ???                                 0x000000000000000a 0x0 + 10
 

Comment 1 by mcnee@chromium.org, Feb 8 2018

 Issue 810244  has been merged into this issue.

Comment 2 by mcnee@chromium.org, Feb 8 2018

Cc: mcnee@chromium.org wjmaclean@chromium.org wfh@chromium.org
 Issue 810243  has been merged into this issue.

Comment 3 by mcnee@chromium.org, Feb 8 2018

 Issue 810208  has been merged into this issue.

Comment 4 by mcnee@chromium.org, Feb 8 2018

 Issue 810150  has been merged into this issue.

Comment 5 by mcnee@chromium.org, Feb 8 2018

I have a CL to disable the check on mac:
https://chromium-review.googlesource.com/c/chromium/src/+/909129

Comment 6 by mcnee@chromium.org, Feb 8 2018

Owner: sdy@chromium.org
Project Member

Comment 7 by bugdroid1@chromium.org, Feb 8 2018

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

commit ecd6be27b79aefc23b0a1e0801b9c67afcaa3131
Author: Kevin McNee <mcnee@chromium.org>
Date: Thu Feb 08 18:23:52 2018

Disable SadTab::RecordFirstPaint DCHECK on Mac

Many tests that crash/kill a tab are flaking because of this DCHECK
on Mac-10.9.

TBR=sky@chromium.org

Bug:  810416 
Change-Id: Id0ffdd7372562e627d7476f2369b0241f3ab5da4
Reviewed-on: https://chromium-review.googlesource.com/909129
Reviewed-by: Kevin McNee <mcnee@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535446}
[modify] https://crrev.com/ecd6be27b79aefc23b0a1e0801b9c67afcaa3131/chrome/browser/ui/sad_tab.cc

Comment 8 by mcnee@chromium.org, Feb 8 2018

 Issue 810405  has been merged into this issue.

Comment 9 by mcnee@chromium.org, Feb 8 2018

 Issue 810406  has been merged into this issue.
Cc: imch...@chromium.org
 Issue 810526  has been merged into this issue.
 Issue 810536  has been merged into this issue.
 Issue 810537  has been merged into this issue.
Cc: dmazz...@chromium.org
Status: Assigned (was: Untriaged)
[mac triage]
this came up in https://chromium-review.googlesource.com/c/chromium/src/+/887736/15/chrome/browser/ui/views/sad_tab_view.h#58 recently..

+dmazzoni fyi
Cc: sky@chromium.org
sky@ recommended against disabling the DCHECK, so I'll revert the previous CL once I've disabled the affected tests.
Isn't the root cause of this bug that [SadTabView updateLayer] can be called more than once now? There's a comment:

- (void)updateLayer {
  // Currently, updateLayer is only called once. If that changes, a DCHECK in
  // SadTab::RecordFirstPaint will pipe up and we should add a guard here.
  sadTab_->RecordFirstPaint();
}

If so, the fix is a guard in that function.
Cc: sdy@chromium.org
Owner: ellyjo...@chromium.org
Status: Started (was: Assigned)
Fix: <https://chromium-review.googlesource.com/c/chromium/src/+/911708>

Comment 17 by sky@chromium.org, Feb 9 2018

Thanks Elly!
Project Member

Comment 18 by bugdroid1@chromium.org, Feb 9 2018

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

commit 0cd0ec8d5bffdfabb2453cba847e7fdcfa393d8a
Author: Elly Fong-Jones <ellyjones@google.com>
Date: Fri Feb 09 17:46:19 2018

cocoa: guard SadTabView's first paint recording against multiple calls

AppKit can call [SadTabView updateLayer] whenever it wants, and we should
not assume it is only called once.

TBR=pkasting@chromium.org

Bug:  810416 
Change-Id: I1dd937cb74ebaf821599b904991156c3ab0eedef
Reviewed-on: https://chromium-review.googlesource.com/911708
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535745}
[modify] https://crrev.com/0cd0ec8d5bffdfabb2453cba847e7fdcfa393d8a/chrome/browser/ui/cocoa/tab_contents/sad_tab_view_cocoa.mm
[modify] https://crrev.com/0cd0ec8d5bffdfabb2453cba847e7fdcfa393d8a/chrome/browser/ui/sad_tab.cc

Labels: -Sheriff-Chromium
Removing Sheriff-Chromium now that this is fixed and the DCHECK is reenabled on Mac.
Status: Fixed (was: Started)

Sign in to add a comment