New issue
Advanced search Search tips

Issue 714334 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Apr 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

When you click outside of a TrayBubbleView, OnWidgetClosing is called twice

Project Member Reported by est...@chromium.org, Apr 22 2017

Issue description

from these two stack traces, you can see that OnWidgetClosing is called twice for a single Widget::Close() because wm::FocusManager calls Widget::Close(), which winds up unfocusing that widget, and BubbleDialogDelegates that use close_on_deactivate will then try to close the same widget again.

#0 0x7f1d6dc5d6f7 base::debug::StackTrace::StackTrace()
#1 0x7f1d6964e535 views::TrayBubbleView::OnWidgetClosing()
#2 0x7f1d69640a43 views::Widget::Close()
#3 0x7f1d68e67bcf ash::TrayBubbleWrapper::~TrayBubbleWrapper()
#4 0x7f1d68e2a031 ash::ImeMenuTray::HideImeMenuBubble()
#5 0x7f1d68e6a91d ash::TrayEventFilter::ProcessPressedEvent()
#6 0x7f1d68da49ba ash::PointerWatcherAdapter::NotifyWatchers()
#7 0x7f1d68da4696 ash::PointerWatcherAdapter::OnMouseEvent()
#8 0x7f1d6a043956 ui::EventDispatcher::DispatchEventToEventHandlers()
#9 0x7f1d6a0435e2 ui::EventDispatcher::ProcessEvent()
#10 0x7f1d6a043414 ui::EventDispatcherDelegate::DispatchEvent()
#11 0x7f1d6a04413d ui::EventProcessor::OnEventFromSource()
#12 0x7f1d6a044716 ui::EventSource::SendEventToSink()
#13 0x7f1d69f1fbe9 aura::WindowTreeHostX11::DispatchXI2Event()
#14 0x7f1d69f1f5b3 aura::WindowTreeHostX11::DispatchEvent()
#15 0x7f1d6e0246e6 ui::PlatformEventSource::DispatchEvent()
#16 0x7f1d68ab88ac ui::X11EventSource::ExtractCookieDataDispatchEvent()
#17 0x7f1d68ab885d ui::X11EventSource::DispatchXEvents()
#18 0x7f1d68abbf49 ui::(anonymous namespace)::XSourceDispatch()
#19 0x7f1d64929ce5 g_main_context_dispatch
#20 0x7f1d6492a048 <unknown>
#21 0x7f1d6492a0ec g_main_context_iteration
#22 0x7f1d6dc8f166 base::MessagePumpGlib::Run()
#23 0x7f1d6dc8c7da base::MessageLoop::RunHandler()
#24 0x7f1d6dcbf70f base::RunLoop::Run()
#25 0x56045d9cf92e ChromeBrowserMainParts::MainMessageLoopRun()
#26 0x7f1d6b4847d2 content::BrowserMainLoop::RunMainMessageLoopParts()
#27 0x7f1d6b4876fe content::BrowserMainRunnerImpl::Run()
#28 0x7f1d6b47fcf8 content::BrowserMain()
#29 0x7f1d6bc20f03 content::ContentMainRunnerImpl::Run()
#30 0x7f1d61397e7f service_manager::Main()
#31 0x7f1d6bc1fb12 content::ContentMain()
#32 0x56045d0678a7 ChromeMain
#33 0x7f1d61cccf45 __libc_start_main
#34 0x56045d0676cd <unknown>
#0 0x7f1d6dc5d6f7 base::debug::StackTrace::StackTrace()
#1 0x7f1d6964e535 views::TrayBubbleView::OnWidgetClosing()
#2 0x7f1d69640a43 views::Widget::Close()
#3 0x7f1d69641996 views::Widget::OnNativeWidgetActivationChanged()
#4 0x7f1d6965cfae views::NativeWidgetAura::OnWindowActivated()
#5 0x7f1d6913bb47 wm::FocusController::SetActiveWindow()
#6 0x7f1d6913b1d0 wm::FocusController::WindowLostFocusFromDispositionChange()
#7 0x7f1d69f175c6 aura::Window::Notifcs.yWindowVisibilityChangedAtReceiver()
#8 0x7f1d69f17280 aura::Window::NotifyWindowVisibilityChangedDown()
#9 0x7f1d69f145fb aura::Window::SetVisible()
#10 0x7f1d6965b99c views::NativeWidgetAura::Close()
#11 0x7f1d69640a92 views::Widget::Close()
#12 0x7f1d68e67bcf ash::TrayBubbleWrapper::~TrayBubbleWrapper()
#13 0x7f1d68e2a031 ash::ImeMenuTray::HideImeMenuBubble()
#14 0x7f1d68e6a91d ash::TrayEventFilter::ProcessPressedEvent()
#15 0x7f1d68da49ba ash::PointerWatcherAdapter::NotifyWatchers()
#16 0x7f1d68da4696 ash::PointerWatcherAdapter::OnMouseEvent()
#17 0x7f1d6a043956 ui::EventDispatcher::DispatchEventToEventHandlers()
#18 0x7f1d6a0435e2 ui::EventDispatcher::ProcessEvent()
#19 0x7f1d6a043414 ui::EventDispatcherDelegate::DispatchEvent()
#20 0x7f1d6a04413d ui::EventProcessor::OnEventFromSource()
#21 0x7f1d6a044716 ui::EventSource::SendEventToSink()
#22 0x7f1d69f1fbe9 aura::WindowTreeHostX11::DispatchXI2Event()
#23 0x7f1d69f1f5b3 aura::WindowTreeHostX11::DispatchEvent()
#24 0x7f1d6e0246e6 ui::PlatformEventSource::DispatchEvent()
#25 0x7f1d68ab88ac ui::X11EventSource::ExtractCookieDataDispatchEvent()
#26 0x7f1d68ab885d ui::X11EventSource::DispatchXEvents()
#27 0x7f1d68abbf49 ui::(anonymous namespace)::XSourceDispatch()
#28 0x7f1d64929ce5 g_main_context_dispatch
#29 0x7f1d6492a048 <unknown>
#30 0x7f1d6492a0ec g_main_context_iteration
#31 0x7f1d6dc8f166 base::MessagePumpGlib::Run()
#32 0x7f1d6dc8c7da base::MessageLoop::RunHandler()
#33 0x7f1d6dcbf70f base::RunLoop::Run()
#34 0x56045d9cf92e ChromeBrowserMainParts::MainMessageLoopRun()
#35 0x7f1d6b4847d2 content::BrowserMainLoop::RunMainMessageLoopParts()
#36 0x7f1d6b4876fe content::BrowserMainRunnerImpl::Run()
#37 0x7f1d6b47fcf8 content::BrowserMain()
#38 0x7f1d6bc20f03 content::ContentMainRunnerImpl::Run()
#39 0x7f1d61397e7f service_manager::Main()
#40 0x7f1d6bc1fb12 content::ContentMain()
#41 0x56045d0678a7 ChromeMain
#42 0x7f1d61cccf45 __libc_start_main
#43 0x56045d0676cd <unknown>
 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 26 2017

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

commit bf25b17bd99b5dbeff92e62f8dddd289c335dc0a
Author: estade <estade@chromium.org>
Date: Wed Apr 26 00:09:29 2017

Don't allow a widget to send close notifications more than once.

Add a test to make sure this works. Also fix a couple WidgetTests that
were disabled.

BUG= 714334 

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

[modify] https://crrev.com/bf25b17bd99b5dbeff92e62f8dddd289c335dc0a/ui/views/widget/widget.cc
[modify] https://crrev.com/bf25b17bd99b5dbeff92e62f8dddd289c335dc0a/ui/views/widget/widget_unittest.cc

Comment 2 by est...@chromium.org, Apr 26 2017

Components: UI
Status: Fixed (was: Started)
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 27 2017

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

commit efd8268f95f1c7a212e5b6edd521761dd8a00e31
Author: alph <alph@chromium.org>
Date: Thu Apr 27 00:31:32 2017

Revert of Don't allow a widget to send close notifications more than once. (patchset #3 id:40001 of https://codereview.chromium.org/2834943002/ )

Reason for revert:
The enabled WidgetObserverTest.ActivationChange test seems to be flaky

https://build.chromium.org/p/chromium.mac/builders/Mac10.10%20Tests/builds/17210

Original issue's description:
> Don't allow a widget to send close notifications more than once.
>
> Add a test to make sure this works. Also fix a couple WidgetTests that
> were disabled.
>
> BUG= 714334 
>
> Review-Url: https://codereview.chromium.org/2834943002
> Cr-Commit-Position: refs/heads/master@{#467171}
> Committed: https://chromium.googlesource.com/chromium/src/+/bf25b17bd99b5dbeff92e62f8dddd289c335dc0a

TBR=sky@chromium.org,estade@chromium.org
NOTRY=true
BUG= 714334 

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

[modify] https://crrev.com/efd8268f95f1c7a212e5b6edd521761dd8a00e31/ui/views/widget/widget.cc
[modify] https://crrev.com/efd8268f95f1c7a212e5b6edd521761dd8a00e31/ui/views/widget/widget_unittest.cc

Comment 4 by est...@chromium.org, Apr 27 2017

did you see any flakes on non-mac bots?
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 27 2017

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

commit 3a303320be089d790c88890ffc28617415238c5c
Author: estade <estade@chromium.org>
Date: Thu Apr 27 01:50:08 2017

Reland of Don't allow a widget to send close notifications more than once. (patchset #1 id:1 of https://codereview.chromium.org/2844933002/ )

Reason for revert:
We will disable the failing test on Mac.

Original issue's description:
> Revert of Don't allow a widget to send close notifications more than once. (patchset #3 id:40001 of https://codereview.chromium.org/2834943002/ )
>
> Reason for revert:
> The enabled WidgetObserverTest.ActivationChange test seems to be flaky
>
> https://build.chromium.org/p/chromium.mac/builders/Mac10.10%20Tests/builds/17210
>
> Original issue's description:
> > Don't allow a widget to send close notifications more than once.
> >
> > Add a test to make sure this works. Also fix a couple WidgetTests that
> > were disabled.
> >
> > BUG= 714334 
> >
> > Review-Url: https://codereview.chromium.org/2834943002
> > Cr-Commit-Position: refs/heads/master@{#467171}
> > Committed: https://chromium.googlesource.com/chromium/src/+/bf25b17bd99b5dbeff92e62f8dddd289c335dc0a
>
> TBR=sky@chromium.org,estade@chromium.org
> NOTRY=true
> BUG= 714334 
>
> Review-Url: https://codereview.chromium.org/2844933002
> Cr-Commit-Position: refs/heads/master@{#467528}
> Committed: https://chromium.googlesource.com/chromium/src/+/efd8268f95f1c7a212e5b6edd521761dd8a00e31

TBR=sky@chromium.org,alph@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 714334 

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

[modify] https://crrev.com/3a303320be089d790c88890ffc28617415238c5c/ui/views/widget/widget.cc
[modify] https://crrev.com/3a303320be089d790c88890ffc28617415238c5c/ui/views/widget/widget_unittest.cc

Status: Verified (was: Fixed)

Sign in to add a comment