New issue
Advanced search Search tips

Issue 702554 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

ExtensionMessageBubbleViewBrowserTest.TestClickingLearnMoreButton flaky on Mac

Project Member Reported by vasi...@chromium.org, Mar 17 2017

Issue description

See https://build.chromium.org/p/chromium.mac/builders/Mac10.11%20Tests/builds/9691

[ RUN      ] ExtensionMessageBubbleViewBrowserTest.TestClickingLearnMoreButton
2017-03-17 02:26:58.904 browser_tests[72630:1036553] NSWindow warning: adding an unknown subview: <FullSizeContentView: 0x7f94cba34f70>. Break on NSLog to debug.
2017-03-17 02:26:58.905 browser_tests[72630:1036553] Call stack:
(
    "+callStackSymbols disabled for performance reasons"
)
[72630:775:0317/022659.858287:ERROR:native_widget_mac.mm(281)] Not implemented reached in virtual void views::NativeWidgetMac::SetWindowIcons(const gfx::ImageSkia &, const gfx::ImageSkia &)
[72630:775:0317/022659.905525:ERROR:native_widget_mac.mm(329)] Not implemented reached in virtual void views::NativeWidgetMac::StackAbove(gfx::NativeView)
BrowserTestBase received signal: Segmentation fault: 11. Backtrace:
0   browser_tests                       0x000000010e25225c base::debug::StackTrace::StackTrace(unsigned long) + 28
1   browser_tests                       0x000000010e9838fa content::(anonymous namespace)::DumpStackTraceSignalHandler(int) + 202
2   libsystem_platform.dylib            0x00007fff94a2e52a _sigtramp + 26
3   libobjc.A.dylib                     0x00007fff7cea1700 (anonymous namespace)::SideTableBuf + 6272
4   browser_tests                       0x000000010f1fff80 views::DialogDelegateView::GetAccessibleNodeData(ui::AXNodeData*) + 32
5   browser_tests                       0x000000010f18847a views::BubbleDialogDelegateView::HandleVisibilityChanged(views::Widget*, bool) + 138
6   browser_tests                       0x000000010f1f8e04 views::Widget::OnNativeWidgetVisibilityChanged(bool) + 244
7   browser_tests                       0x000000010f19189f views::BridgedNativeWidget::OnVisibilityChanged() + 479
8   browser_tests                       0x000000010f194e10 -[NativeWidgetMacNSWindow orderWindow:relativeTo:] + 80
9   CoreFoundation                      0x00007fff9b702b1c __CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__ + 12
10  CoreFoundation                      0x00007fff9b702aaf ___CFXRegistrationPost_block_invoke + 63
11  CoreFoundation                      0x00007fff9b702a27 _CFXRegistrationPost + 407
12  CoreFoundation                      0x00007fff9b702792 ___CFXNotificationPost_block_invoke + 50
13  CoreFoundation                      0x00007fff9b6bf542 -[_CFXNotificationRegistrar find:object:observer:enumerator:] + 1922
14  CoreFoundation                      0x00007fff9b6be795 _CFXNotificationPost + 693
15  Foundation                          0x00007fff98dc617a -[NSNotificationCenter postNotificationName:object:userInfo:] + 66
16  CoreGraphics                        0x00007fff92beb941 (anonymous namespace)::notify_datagram_handler(unsigned int, CGSDatagramType, void*, unsigned long, void*) + 885
17  CoreGraphics                        0x00007fff92be87fe CGSDatagramReadStream::dispatch_datagrams() + 440
18  CoreGraphics                        0x00007fff92c26f9e ___ZN21CGSDatagramReadStream24dispatch_datagrams_asyncEP16dispatch_queue_sPS__block_invoke + 18
19  libdispatch.dylib                   0x00007fff9dcfb93d _dispatch_call_block_and_release + 12
20  libdispatch.dylib                   0x00007fff9dcf040b _dispatch_client_callout + 8
21  libdispatch.dylib                   0x00007fff9dd03c1c _dispatch_main_queue_callback_4CF + 1685
22  CoreFoundation                      0x00007fff9b72c949 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 9
23  CoreFoundation                      0x00007fff9b6eb83d __CFRunLoopRun + 1949
24  CoreFoundation                      0x00007fff9b6eae38 CFRunLoopRunSpecific + 296
25  HIToolbox                           0x00007fff8f3d0935 RunCurrentEventLoopInMode + 235
26  HIToolbox                           0x00007fff8f3d076f ReceiveNextEventCommon + 432
27  HIToolbox                           0x00007fff8f3d05af _BlockUntilNextEventMatchingListInModeWithFilter + 71
28  AppKit                              0x00007fff9955fdf6 _DPSNextEvent + 1067
29  AppKit                              0x00007fff9955f226 -[NSApplication _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 454
30  browser_tests                       0x000000010e378870 __71-[BrowserCrApplication nextEventMatchingMask:untilDate:inMode:dequeue:]_block_invoke + 64
31  browser_tests                       0x000000010e26cf0a base::mac::CallWithEHFrame(void () block_pointer) + 10
32  browser_tests                       0x000000010e3787a9 -[BrowserCrApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 169
33  AppKit                              0x00007fff99553d80 -[NSApplication run] + 682
34  browser_tests                       0x000000010e27d15e base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate*) + 334
35  browser_tests                       0x000000010e27c74c base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*) + 92
36  browser_tests                       0x000000010e278a4e base::MessageLoop::RunHandler() + 94
37  browser_tests                       0x000000010e29cb53 base::RunLoop::Run() + 115
38  browser_tests                       0x000000010c7867b1 ExtensionMessageBubbleBrowserTest::TestClickingLearnMoreButton() + 273
39  browser_tests                       0x000000010e312ad7 InProcessBrowserTest::RunTestOnMainThreadLoop() + 215
40  browser_tests                       0x000000010e983656 content::BrowserTestBase::ProxyRunTestOnMainThreadLoop() + 310
41  browser_tests                       0x000000010e37d9a0 ChromeBrowserMainParts::PreMainMessageLoopRunImpl() + 4176
42  browser_tests                       0x000000010e37c84e ChromeBrowserMainParts::PreMainMessageLoopRun() + 62
43  browser_tests                       0x000000010d11d293 content::BrowserMainLoop::PreMainMessageLoopRun() + 67
44  browser_tests                       0x000000010d455026 content::StartupTaskRunner::RunAllTasksNow() + 38
45  browser_tests                       0x000000010d11b572 content::BrowserMainLoop::CreateStartupTasks() + 658
46  browser_tests                       0x000000010d120164 content::BrowserMainRunnerImpl::Initialize(content::MainFunctionParams const&) + 756
47  browser_tests                       0x000000010d118f64 content::BrowserMain(content::MainFunctionParams const&) + 100
48  browser_tests                       0x000000010e238210 content::ContentMainRunnerImpl::Run() + 368
49  browser_tests                       0x000000010e2373f9 content::ContentMain(content::ContentMainParams const&) + 153
50  browser_tests                       0x000000010e983342 content::BrowserTestBase::SetUp() + 1906
51  browser_tests                       0x000000010e311db7 InProcessBrowserTest::SetUp() + 391
52  browser_tests                       0x000000010f0b1bd1 testing::Test::Run() + 97
53  browser_tests                       0x000000010f0b26f0 testing::TestInfo::Run() + 288
54  browser_tests                       0x000000010f0b2c87 testing::TestCase::Run() + 263
55  browser_tests                       0x000000010f0b8cb7 testing::internal::UnitTestImpl::RunAllTests() + 871
56  browser_tests                       0x000000010f0b8923 testing::UnitTest::Run() + 163
57  browser_tests                       0x000000010e327a73 base::TestSuite::Run() + 163
58  browser_tests                       0x000000010e24452f ChromeTestSuiteRunner::RunTestSuite(int, char**) + 31
59  browser_tests                       0x000000010e9c242e content::LaunchTests(content::TestLauncherDelegate*, int, int, char**) + 302
60  browser_tests                       0x000000010e2444ba main + 90
61  libdyld.dylib                       0x00007fff96cff5ad start + 1
 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 17 2017

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

commit a3808ac0ef6f16ef618330393bf2d5e95b8c9564
Author: Vasilii Sukhanov <vasilii@chromium.org>
Date: Fri Mar 17 10:24:40 2017

Disable ExtensionMessageBubbleViewBrowserTest.TestClickingLearnMoreButton on Mac

BUG= 702554 
TBR=tapted@chromium.org

Review-Url: https://codereview.chromium.org/2753263002 .
Cr-Commit-Position: refs/heads/master@{#457730}

[modify] https://crrev.com/a3808ac0ef6f16ef618330393bf2d5e95b8c9564/chrome/browser/ui/views/extensions/extension_message_bubble_view_browsertest.cc

 https://codereview.chromium.org/2630473003 can be relevant here.

Comment 3 by tapted@chromium.org, Mar 17 2017

Labels: -Pri-3 M-59 Pri-1
Status: Assigned (was: Untriaged)
Thanks! I'll take a look next week.
 Issue 702723  has been merged into this issue.

Comment 5 by tapted@chromium.org, Mar 20 2017

Status: Started (was: Assigned)
Have repro'd in debug to get the all-important "non-virtual thunk" that's optimised out and reveals the actual problem:

5   ???                                 0x0100000123bda531 0x0 + 72057598932526385
6   browser_tests                       0x00000001137397e0 non-virtual thunk to ToolbarActionsBarBubbleViews::GetWindowTitle() const + 32
7   libviews.dylib                      0x000000012a37a75f views::DialogDelegateView::GetAccessibleNodeData(ui::AXNodeData*) + 63
8   libviews.dylib                      0x000000012a1b8ce7 views::BubbleDialogDelegateView::HandleVisibilityChanged(views::Widget*, bool) + 327
9   libviews.dylib                      0x000000012a1b8b8c views::BubbleDialogDelegateView::OnWidgetVisibilityChanged(views::Widget*, bool) + 44
10  libviews.dylib                      0x000000012a3623f3 views::Widget::OnNativeWidgetVisibilityChanged(bool) + 227
11  libviews.dylib                      0x000000012a1d8662 views::BridgedNativeWidget::OnVisibilityChanged() + 610
12  libviews.dylib                      0x000000012a1effe8 -[ViewsNSWindowDelegate onWindowOrderChanged:] + 40

base::string16 ToolbarActionsBarBubbleViews::GetWindowTitle() const {
  return delegate_->GetHeadingText();
}

delegate_ can be null, so it needs a check. See, e.g.,

bool ToolbarActionsBarBubbleViews::Close() {
  if (delegate_) {
    delegate_->OnBubbleClosed(
        ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS_DEACTIVATION);
  }
  return true;
}

Comment 7 by tapted@chromium.org, Mar 21 2017

Suggsted fix for this issue -> https://codereview.chromium.org/2762893002/

Updated Issue 624560 for autofill::SaveCardBubbleViews::GetWindowTitle which still appears to be crashy.
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 24 2017

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

commit b9fc45d0bb0654521d098aae19e73925e79ac270
Author: tapted <tapted@chromium.org>
Date: Fri Mar 24 10:39:42 2017

Fix a lifetime issue in ToolbarActionsBarBubbleViews.

Also re-enable
ExtensionMessageBubbleViewBrowserTest.TestClickingLearnMoreButton on
Mac, which tickled it, and add more coverage.

The test was disabled on Mac in r457730, but the crash reporter has
matching stacks recorded on Windows - see  http://crbug.com/702554#c6 .

The problem is that clicking a button would trigger an asynchronous
Widget::Close() after setting the view's delegate to null. But OS events
can still arrive while waiting for the asynchronous Close() to complete,
and cause a segfault.

To fix, don't clear the view's delegate: just set state to ensure the
correct close reason is forwarded to it when the dialog is closed via
the usual codepaths.

BUG= 702554 

Review-Url: https://codereview.chromium.org/2762893002
Cr-Commit-Position: refs/heads/master@{#459382}

[modify] https://crrev.com/b9fc45d0bb0654521d098aae19e73925e79ac270/chrome/browser/ui/views/extensions/extension_message_bubble_view_browsertest.cc
[modify] https://crrev.com/b9fc45d0bb0654521d098aae19e73925e79ac270/chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views.cc
[modify] https://crrev.com/b9fc45d0bb0654521d098aae19e73925e79ac270/chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views.h

Project Member

Comment 9 by bugdroid1@chromium.org, Mar 24 2017

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

commit b9fc45d0bb0654521d098aae19e73925e79ac270
Author: tapted <tapted@chromium.org>
Date: Fri Mar 24 10:39:42 2017

Fix a lifetime issue in ToolbarActionsBarBubbleViews.

Also re-enable
ExtensionMessageBubbleViewBrowserTest.TestClickingLearnMoreButton on
Mac, which tickled it, and add more coverage.

The test was disabled on Mac in r457730, but the crash reporter has
matching stacks recorded on Windows - see  http://crbug.com/702554#c6 .

The problem is that clicking a button would trigger an asynchronous
Widget::Close() after setting the view's delegate to null. But OS events
can still arrive while waiting for the asynchronous Close() to complete,
and cause a segfault.

To fix, don't clear the view's delegate: just set state to ensure the
correct close reason is forwarded to it when the dialog is closed via
the usual codepaths.

BUG= 702554 

Review-Url: https://codereview.chromium.org/2762893002
Cr-Commit-Position: refs/heads/master@{#459382}

[modify] https://crrev.com/b9fc45d0bb0654521d098aae19e73925e79ac270/chrome/browser/ui/views/extensions/extension_message_bubble_view_browsertest.cc
[modify] https://crrev.com/b9fc45d0bb0654521d098aae19e73925e79ac270/chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views.cc
[modify] https://crrev.com/b9fc45d0bb0654521d098aae19e73925e79ac270/chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views.h

Status: Fixed (was: Started)
That landed in 59.0.3051.0. 59.0.3043.0 is the last recorded crash (1, on Windows). Calling this one fixed.

Sign in to add a comment