New issue
Advanced search Search tips

Issue 868416 link

Starred by 1 user
Status: Fixed
Owner:
weili@chromium.org
Closed: Jul 27
Cc:
sdy@chromium.org
a...@chromium.org
lgrey@chromium.org
ellyjo...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

Popup from user fullscreen DCHECKs

Project Member Reported by a...@chromium.org, Jul 27 
To repro:

0. Download the PoC from  bug 858929 .
1. Load the "testcase.html" file.
2. Go into user fullscreen by clicking the green button.
3. Click the "click-2" button.

What happens:

A popup appears in small size, then a crash (attached below).

If you bypass the DCHECK, what happens is that the popup ends up gaining user fullscreen too. What should happen is that there is no crash, and the popup should stay non-fullscreened.

[40595:1295:0727/122743.874591:FATAL:bridged_native_widget.mm(670)] Check failed: window_visible_. 
0   libbase.dylib                       0x000000012198781e base::debug::StackTrace::StackTrace(unsigned long) + 174
1   libbase.dylib                       0x00000001219878dd base::debug::StackTrace::StackTrace(unsigned long) + 29
2   libbase.dylib                       0x00000001215628dc base::debug::StackTrace::StackTrace() + 28
3   libbase.dylib                       0x00000001215ddf4c logging::LogMessage::~LogMessage() + 460
4   libbase.dylib                       0x00000001215dbcb5 logging::LogMessage::~LogMessage() + 21
5   libviews.dylib                      0x000000014268c5e3 views::BridgedNativeWidget::OnFullscreenTransitionStart(bool) + 243
6   libviews.dylib                      0x00000001426a6a8d -[ViewsNSWindowDelegate windowWillEnterFullScreen:] + 45
7   CoreFoundation                      0x00007fff895fcb0c __CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__ + 12
8   CoreFoundation                      0x00007fff895fca9f ___CFXRegistrationPost_block_invoke + 63
9   CoreFoundation                      0x00007fff895fca17 _CFXRegistrationPost + 407
10  CoreFoundation                      0x00007fff895fc782 ___CFXNotificationPost_block_invoke + 50
11  CoreFoundation                      0x00007fff895b9592 -[_CFXNotificationRegistrar find:object:observer:enumerator:] + 1922
12  CoreFoundation                      0x00007fff895b87e5 _CFXNotificationPost + 693
13  Foundation                          0x00007fff89f38f5a -[NSNotificationCenter postNotificationName:object:userInfo:] + 66
14  AppKit                              0x00007fff87c8e834 __84-[_NSFullScreenTransition enterFullScreenTransitionWithPresentationOptions:options:]_block_invoke + 74
15  AppKit                              0x00007fff87c8e7c4 -[_NSFullScreenTransition enterFullScreenTransitionWithPresentationOptions:options:] + 1409
16  AppKit                              0x00007fff87c919a5 -[_NSWindowFullScreenTransition enterFullScreenTransitionForWindow:options:] + 461
17  AppKit                              0x00007fff87b4ea10 -[NSWindow _enterFullScreenMode:options:] + 298
18  AppKit                              0x00007fff87b4e8d7 -[NSWindow _enterFullScreenMode:options:implicitlyTile:] + 150
19  AppKit                              0x00007fff876a2348 -[NSWindow enterFullScreenMode:] + 167
20  AppKit                              0x00007fff874c2c91 -[NSWindow _reallyDoOrderWindow:relativeTo:findKey:forCounter:force:isModal:] + 3104
21  AppKit                              0x00007fff874c0f5f -[NSWindow _doOrderWindow:relativeTo:findKey:forCounter:force:isModal:] + 894
22  AppKit                              0x00007fff874c0b83 -[NSWindow orderWindow:relativeTo:] + 159
23  libviews.dylib                      0x00000001426a46b1 -[NativeWidgetMacNSWindow orderWindow:relativeTo:] + 81
24  AppKit                              0x00007fff8757ffc6 -[NSWindow makeKeyAndOrderFront:] + 106
25  libviews.dylib                      0x000000014268a58f views::BridgedNativeWidget::SetVisibilityState(views::BridgedNativeWidget::WindowVisibilityState) + 1103
26  libviews.dylib                      0x000000014281eb76 views::NativeWidgetMac::ShowWithWindowState(ui::WindowShowState) + 790
27  libviews.dylib                      0x00000001428318f2 views::Widget::Show() + 786
28  libchrome_dll.dylib                 0x00000001112814fb BrowserView::Show() + 139
29  libchrome_dll.dylib                 0x0000000110a03be3 (anonymous namespace)::ScopedBrowserShower::~ScopedBrowserShower() + 131
30  libchrome_dll.dylib                 0x0000000110a02655 (anonymous namespace)::ScopedBrowserShower::~ScopedBrowserShower() + 21
31  libchrome_dll.dylib                 0x00000001109ff542 Navigate(NavigateParams*) + 7810
32  libchrome_dll.dylib                 0x0000000110a093ef chrome::AddWebContents(Browser*, content::WebContents*, std::__1::unique_ptr<content::WebContents, std::__1::default_delete<content::WebContents> >, WindowOpenDisposition, gfx::Rect const&) + 1135
33  libchrome_dll.dylib                 0x00000001109cc0c2 Browser::AddNewContents(content::WebContents*, std::__1::unique_ptr<content::WebContents, std::__1::default_delete<content::WebContents> >, WindowOpenDisposition, gfx::Rect const&, bool, bool*) + 594
34  libchrome_dll.dylib                 0x00000001109cc250 non-virtual thunk to Browser::AddNewContents(content::WebContents*, std::__1::unique_ptr<content::WebContents, std::__1::default_delete<content::WebContents> >, WindowOpenDisposition, gfx::Rect const&, bool, bool*) + 96
35  libcontent.dylib                    0x000000012a7810c3 content::WebContentsImpl::ShowCreatedWindow(int, int, WindowOpenDisposition, gfx::Rect const&, bool) + 931
36  libcontent.dylib                    0x00000001298b3084 content::RenderFrameHostImpl::OnShowCreatedWindow(int, WindowOpenDisposition, gfx::Rect const&, bool) + 116
37  libcontent.dylib                    0x0000000129900215 void base::DispatchToMethodImpl<content::RenderFrameHostImpl*, void (content::RenderFrameHostImpl::*)(int, WindowOpenDisposition, gfx::Rect const&, bool), std::__1::tuple<int, WindowOpenDisposition, gfx::Rect, bool>, 0ul, 1ul, 2ul, 3ul>(content::RenderFrameHostImpl* const&, void (content::RenderFrameHostImpl::*)(int, WindowOpenDisposition, gfx::Rect const&, bool), std::__1::tuple<int, WindowOpenDisposition, gfx::Rect, bool>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul>) + 325
38  libcontent.dylib                    0x00000001299000c8 void base::DispatchToMethod<content::RenderFrameHostImpl*, void (content::RenderFrameHostImpl::*)(int, WindowOpenDisposition, gfx::Rect const&, bool), std::__1::tuple<int, WindowOpenDisposition, gfx::Rect, bool> >(content::RenderFrameHostImpl* const&, void (content::RenderFrameHostImpl::*)(int, WindowOpenDisposition, gfx::Rect const&, bool), std::__1::tuple<int, WindowOpenDisposition, gfx::Rect, bool>&&) + 104
39  libcontent.dylib                    0x0000000129900032 void IPC::DispatchToMethod<content::RenderFrameHostImpl, void (content::RenderFrameHostImpl::*)(int, WindowOpenDisposition, gfx::Rect const&, bool), void, std::__1::tuple<int, WindowOpenDisposition, gfx::Rect, bool> >(content::RenderFrameHostImpl*, void (content::RenderFrameHostImpl::*)(int, WindowOpenDisposition, gfx::Rect const&, bool), void*, std::__1::tuple<int, WindowOpenDisposition, gfx::Rect, bool>&&) + 114
40  libcontent.dylib                    0x00000001298b2f97 bool IPC::MessageT<FrameHostMsg_ShowCreatedWindow_Meta, std::__1::tuple<int, WindowOpenDisposition, gfx::Rect, bool>, void>::Dispatch<content::RenderFrameHostImpl, content::RenderFrameHostImpl, void, void (content::RenderFrameHostImpl::*)(int, WindowOpenDisposition, gfx::Rect const&, bool)>(IPC::Message const*, content::RenderFrameHostImpl*, content::RenderFrameHostImpl*, void*, void (content::RenderFrameHostImpl::*)(int, WindowOpenDisposition, gfx::Rect const&, bool)) + 631
41  libcontent.dylib                    0x00000001298a32e1 content::RenderFrameHostImpl::OnMessageReceived(IPC::Message const&) + 6321
42  libcontent.dylib                    0x000000012a1c9c03 content::RenderProcessHostImpl::OnMessageReceived(IPC::Message const&) + 835
43  libipc.dylib                        0x0000000126e22875 IPC::ChannelProxy::Context::OnDispatchMessage(IPC::Message const&) + 149

Comment 1 by a...@chromium.org, Jul 27

Description: Show this description

Comment 2 by a...@chromium.org, Jul 27 

The question that I have is: how does this popup get into fullscreen? We don't seem to be putting it into fullscreen (and DCHECK in surprise when this window that isn't even supposed to be visible shows up in the OnFullscreenTransitionStart callback).

The answer seems to be Cocoa magic. We get the popup window and try to show it (views::Widget::Show(), stack frame 27). views::BridgedNativeWidget::SetVisibilityState (stack frame 25) calls -[NSWindow makeKeyAndOrderFront:nil] to show the popup window in front of everything. And surprise! Cocoa sees that the current "frontmost" window is in fullscreen, so it automagically fullscreens the popup too!

Comment 3 Deleted

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 27 

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

commit c552cd7b8a0862f6b3c8c6a07f98bda3721101eb
Author: Avi Drissman <avi@chromium.org>
Date: Fri Jul 27 19:44:37 2018

Mac: turn popups into new tabs while in fullscreen.

It's platform convention to show popups as new tabs while in
non-HTML5 fullscreen. (Popups cause tabs to lose HTML5 fullscreen.)

This was implemented for Cocoa in a BrowserWindow override, but
it makes sense to just stick it into Browser and remove a ton
of override code put in just to support this.

BUG= 858929 ,  868416 
TEST=as in bugs

Change-Id: I43471f242813ec1159d9c690bab73dab3e610b7d
Reviewed-on: https://chromium-review.googlesource.com/1153455
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578755}
[modify] https://crrev.com/c552cd7b8a0862f6b3c8c6a07f98bda3721101eb/chrome/browser/ui/browser.cc
[modify] https://crrev.com/c552cd7b8a0862f6b3c8c6a07f98bda3721101eb/chrome/browser/ui/browser_navigator.cc
[modify] https://crrev.com/c552cd7b8a0862f6b3c8c6a07f98bda3721101eb/chrome/browser/ui/browser_window.h
[modify] https://crrev.com/c552cd7b8a0862f6b3c8c6a07f98bda3721101eb/chrome/browser/ui/cocoa/browser_window_cocoa.h
[modify] https://crrev.com/c552cd7b8a0862f6b3c8c6a07f98bda3721101eb/chrome/browser/ui/cocoa/browser_window_cocoa.mm
[modify] https://crrev.com/c552cd7b8a0862f6b3c8c6a07f98bda3721101eb/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/c552cd7b8a0862f6b3c8c6a07f98bda3721101eb/chrome/browser/ui/views/frame/browser_view.h
[modify] https://crrev.com/c552cd7b8a0862f6b3c8c6a07f98bda3721101eb/chrome/test/base/test_browser_window.cc
[modify] https://crrev.com/c552cd7b8a0862f6b3c8c6a07f98bda3721101eb/chrome/test/base/test_browser_window.h

Comment 5 by a...@chromium.org, Jul 27

Status: Fixed (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 29

Labels: merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c18c88413f1a3210e77fd73ef1242637481d9fc8

commit c18c88413f1a3210e77fd73ef1242637481d9fc8
Author: Avi Drissman <avi@chromium.org>
Date: Sun Jul 29 22:22:26 2018

Mac: turn popups into new tabs while in fullscreen.

It's platform convention to show popups as new tabs while in
non-HTML5 fullscreen. (Popups cause tabs to lose HTML5 fullscreen.)

This was implemented for Cocoa in a BrowserWindow override, but
it makes sense to just stick it into Browser and remove a ton
of override code put in just to support this.

BUG= 858929 ,  868416 
TEST=as in bugs

Change-Id: I43471f242813ec1159d9c690bab73dab3e610b7d
Reviewed-on: https://chromium-review.googlesource.com/1153455
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#578755}(cherry picked from commit c552cd7b8a0862f6b3c8c6a07f98bda3721101eb)
Reviewed-on: https://chromium-review.googlesource.com/1154508
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#192}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/c18c88413f1a3210e77fd73ef1242637481d9fc8/chrome/browser/ui/browser.cc
[modify] https://crrev.com/c18c88413f1a3210e77fd73ef1242637481d9fc8/chrome/browser/ui/browser_navigator.cc
[modify] https://crrev.com/c18c88413f1a3210e77fd73ef1242637481d9fc8/chrome/browser/ui/browser_window.h
[modify] https://crrev.com/c18c88413f1a3210e77fd73ef1242637481d9fc8/chrome/browser/ui/cocoa/browser_window_cocoa.h
[modify] https://crrev.com/c18c88413f1a3210e77fd73ef1242637481d9fc8/chrome/browser/ui/cocoa/browser_window_cocoa.mm
[modify] https://crrev.com/c18c88413f1a3210e77fd73ef1242637481d9fc8/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/c18c88413f1a3210e77fd73ef1242637481d9fc8/chrome/browser/ui/views/frame/browser_view.h
[modify] https://crrev.com/c18c88413f1a3210e77fd73ef1242637481d9fc8/chrome/test/base/test_browser_window.cc
[modify] https://crrev.com/c18c88413f1a3210e77fd73ef1242637481d9fc8/chrome/test/base/test_browser_window.h
►

Sign in to add a comment