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

Issue 650538 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

Mac: Crashing on exception: Error (1000) creating CGSWindow' under ExclusiveAccessController::Show() under FullscreenController::ExitFullscreenModeForTab(content::WebContents*)

Project Member Reported by tapted@chromium.org, Sep 27 2016

Issue description

I'm investigating this collection of crashes

https://crash.corp.google.com/browse?q=product.name%3D%27Chrome_Mac%27%20AND%20custom_data.ChromeCrashProto.ptype%3D%27browser%27%20AND%20custom_data.ChromeCrashProto.magic_signature_1.component%3D%27src%2Fui%2Fviews%2Fcocoa%27%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D%27-%5BNativeWidgetMacNSWindow%20initWithContentRect%3AstyleMask%3Abacking%3Adefer%3A%5D%27%20AND%20product.version%3D%2752.0.2743.116%27%20OMIT%20RECORD%20IF%20SOME(ProductData.key%3D%27list_annotations%27%20AND%20ProductData.value%3D%27Crashing%20on%20exception%3A%20Error%20(1000)%20creating%20CGSWindow%27)%20!%3D%200&ignore_case=false&enable_rewrite=false&omit_field_name=&omit_field_value=&omit_field_opt=


The only occur on 10.9 and 10.10 (not 10.11 or 10.12 [yet]). But this is effectively the "top crasher" for MacViews with around 202 crash reports across all Chrome versions.

The exception is not a new issue - see e.g. Issue 647574


What *is* interesting is that the stacks look like:


 -native_widget_mac_nswindow.mm:39 )  -[NativeWidgetMacNSWindow initWithContentRect:styleMask:backing:defer:]
 -native_widget_mac.mm:607 )  views::NativeWidgetMac::CreateNSWindow(views::Widget::InitParams const&)
 -native_widget_mac.mm:123 )  views::NativeWidgetMac::InitNativeWidget(views::Widget::InitParams const&)
 -widget.cc:339 ) views::Widget::Init(views::Widget::InitParams const&)
 -subtle_notification_view.cc:199 ) SubtleNotificationView::CreatePopupWidget(NSView*, SubtleNotificationView*, bool)
 -exclusive_access_bubble_views.cc:69 ) ExclusiveAccessBubbleViews::ExclusiveAccessBubbleViews(ExclusiveAccessBubbleViewsContext*, GURL BubbleType)
Google Chrome Framework -exclusive_access_controller_views.mm:43 ) ExclusiveAccessController::Show()
 -exclusive_access_controller_views.mm:160 )  ExclusiveAccessController::UpdateExclusiveAccessExitBubbleContent(GURL const&, pe)
Google Chrome Framework -exclusive_access_manager.cc:93 )  ExclusiveAccessManager::UpdateExclusiveAccessExitBubbleContent()
 -fullscreen_controller.cc:200 )  FullscreenController::ExitFullscreenModeForTab(content::WebContents*)
 -exclusive_access_controller_base.cc:47 )  ExclusiveAccessControllerBase::OnTabClosing(content::WebContents*)
 -exclusive_access_manager.cc:125 ) ExclusiveAccessManager::OnTabClosing(content::WebContents*)
 -browser.cc:1012 ) Browser::TabClosingAt(TabStripModel*, content::WebContents*, int)
 -tab_strip_model.cc:1220 ) TabStripModel::InternalCloseTab(content::WebContents*, int, bool)
 -tab_strip_model.cc:1205 ) TabStripModel::InternalCloseTabs(std::__1::vector<int, std::__1::allocator<int> > const&, unsigned int)
 -tab_strip_model.cc:512 )  TabStripModel::CloseAllTabs()
 -browser.cc:746 )  Browser::OnWindowClosing()
 -unload_controller.cc:364 )  chrome::UnloadController::CanCloseContents(content::WebContents*)
 -browser.cc:1615 ) non-virtual thunk to Browser::CloseContents(content::WebContents*)
 -tuple.h:166 ) bool IPC::MessageT<ViewHostMsg_ClosePage_ACK_Meta, std::__1::tuple<>, void>::Dispatch<content::RenderViewHostImpl, Impl, void, void (content::RenderViewHostImpl::*)()>(IPC::Message const*, content::RenderViewHostImpl*, content::RenderViewHostImpl*, void*, void (content::RenderViewHostImpl::*)())
  -render_view_host_impl.cc:913 )  content::RenderViewHostImpl::OnMessageReceived(IPC::Message const&)


So why is the ExclusiveAccessController trying to *show* the fullscreen bubble in response to a Browser window *closing*? That doesn't seem right.
 

Comment 1 by tapted@chromium.org, Sep 27 2016

Here's another crash stack coming in via TabStringModel::CloseAllTabs() into ExclusiveAccessBubbleViews::RepositionIfVisible()

http://go/crash/b64ca33c00000000

-[NSWindow _doOrderWindow:relativeTo:findKey:forCounter:force:isModal:]
CGSWindowShmemGetWindowSeed
CGSDeviceGetGeometry
ripd_Geometry
ripc_GetClipState
ripc_GetRenderingState
ripc_DrawRects
CGContextFillRects
NSRectFill
-[NSView _drawRect:clip:]
-[NSView _recursiveDisplayAllDirtyWithLockFocus:visRect:]
-[NSView _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:]
-[NSNextStepFrame _recursiveDisplayRectIfNeededIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:topView:]
-[NSView _displayRectIgnoringOpacity:isVisibleRect:rectIsVisibleRectForView:]
-[NSView displayIfNeeded]
-[NSNextStepFrame displayIfNeeded]
-[NSWindow _setFrameCommon:display:stashSize:]
-[NSWindow setFrame:display:animate:]
views::BridgedNativeWidget::SetBounds(gfx::Rect const&)
ExclusiveAccessBubbleViews::RepositionIfVisible()
ExclusiveAccessController::Layout(double)
-[BrowserWindowController(Private) applyLayout:]
-[BrowserWindowController(Private) layoutSubviews]
-[BrowserWindowController(Fullscreen) updateFullscreenExitBubble]
ExclusiveAccessController::UpdateExclusiveAccessExitBubbleContent(GURL const&, ExclusiveAccessBubbleType)
ExclusiveAccessManager::UpdateExclusiveAccessExitBubbleContent()
FullscreenController::NotifyTabExclusiveAccessLost()
FullscreenController::ToggleFullscreenModeInternal(FullscreenController::FullscreenInternalOption)
ExclusiveAccessControllerBase::OnTabClosing(content::WebContents*)
ExclusiveAccessManager::OnTabClosing(content::WebContents*)
Browser::TabClosingAt(TabStripModel*, content::WebContents*, int)
TabStripModel::InternalCloseTab(content::WebContents*, int, bool)
TabStripModel::InternalCloseTabs(std::__1::vector<int, std::__1::allocator<int> > const&, unsigned int)
TabStripModel::CloseAllTabs()
Browser::OnWindowClosing()
chrome::UnloadController::CanCloseContents(content::WebContents*)
non-virtual thunk to Browser::CloseContents(content::WebContents*)
bool IPC::MessageT<ViewHostMsg_ClosePage_ACK_Meta, std::__1::tuple<>, void>::Dispatch<content::RenderViewHostImpl, content::RenderViewHostImpl, void, void (content::RenderViewHostImpl::*)()>(IPC::Message const*, content::RenderViewHostImpl*, content::RenderViewHostImpl*, void*, void (content::RenderViewHostImpl::*)())

Comment 2 by tapted@chromium.org, Sep 28 2016

Status: Started (was: Available)
There is some vestigial #if(mac) code in fullscreen_controller.cc which seems to put this machinery into unwanted states.

https://codereview.chromium.org/2373253002

Comment 3 by tapted@chromium.org, Sep 28 2016

Owner: tapted@chromium.org

Comment 4 by tapted@chromium.org, Sep 28 2016

And in fact there's more obsolete layout triggers in BrowserWindowController as well - separate CL for that in https://codereview.chromium.org/2373943002


Project Member

Comment 5 by bugdroid1@chromium.org, Sep 29 2016

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

commit 1bd32f9d092dca9657f4862916c96df7fb3a202f
Author: tapted <tapted@chromium.org>
Date: Thu Sep 29 04:07:40 2016

Remove vestigial #ifmac code from fullscreen_controller.cc

There's some code to force an update to the Cocoa UI on Mac, but the
Cocoa fullscreen UI is gone now. This isn't needed, and seems to cause
some crashes within AppKit window management code on older OS versions
because it can be triggered during the closure of the parent window.

BUG= 650538 
TEST=Manually, and ensuring no regressions in `interactive_ui_tests
--gtest_filter=FullscreenControllerInteractiveTest.*
--gtest_also_run_disabled_tests`

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

[modify] https://crrev.com/1bd32f9d092dca9657f4862916c96df7fb3a202f/chrome/browser/ui/exclusive_access/fullscreen_controller.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 29 2016

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

commit 19b0e2dd976fb5fc1ec028c1cbbc584b4ca7dde2
Author: tapted <tapted@chromium.org>
Date: Thu Sep 29 23:14:06 2016

Mac: Don't call -[BWC layoutSubviews] before updating the fullscreen bubble.

ExclusiveAccessController always repositions the bubble according to the
size of the browser window. So invoking -[BrowserWindowController
layoutSubviews] isn't necessary to update the bubble

Doing layoutSubviews is expensive, unnecessary, and can cause a
repositioning attempt of a bubble that is about to go away, since the
bubble type isn't checked until updateFullscreenExitBubble calls
BWC(Private) updateFullscreenExitBubble (which it does immediately
*after* layoutSubviews). This sequence of events seems to be responsible
for some AppKit crashes on older versions of OSX.

ExclusiveAccessController::Layout() can be removed completely (it
already ignores it's max_y argument, so remove some plumbing for that).

BUG= 650538 
TEST=Manually tested fullscreen flows, and ensured there were no
regressions running `interactive_ui_tests
--gtest_filter=FullscreenControllerInteractiveTest.*
--gtest_also_run_disabled_tests` locally.

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

[modify] https://crrev.com/19b0e2dd976fb5fc1ec028c1cbbc584b4ca7dde2/chrome/browser/ui/cocoa/browser/exclusive_access_controller_views.h
[modify] https://crrev.com/19b0e2dd976fb5fc1ec028c1cbbc584b4ca7dde2/chrome/browser/ui/cocoa/browser/exclusive_access_controller_views.mm
[modify] https://crrev.com/19b0e2dd976fb5fc1ec028c1cbbc584b4ca7dde2/chrome/browser/ui/cocoa/browser_window_controller.mm
[modify] https://crrev.com/19b0e2dd976fb5fc1ec028c1cbbc584b4ca7dde2/chrome/browser/ui/cocoa/browser_window_controller_private.mm
[modify] https://crrev.com/19b0e2dd976fb5fc1ec028c1cbbc584b4ca7dde2/chrome/browser/ui/cocoa/browser_window_layout.h
[modify] https://crrev.com/19b0e2dd976fb5fc1ec028c1cbbc584b4ca7dde2/chrome/browser/ui/cocoa/browser_window_layout.mm
[modify] https://crrev.com/19b0e2dd976fb5fc1ec028c1cbbc584b4ca7dde2/chrome/browser/ui/cocoa/browser_window_layout_unittest.mm

Comment 8 by tapted@chromium.org, Dec 12 2016

Status: Fixed (was: Started)
> These are obscure so I won't count all my chickens until Stable, but it looks promising.

/me counts zero chickens in m55.

Sign in to add a comment