Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(558)

Issue 1169063002: MacViews: Retain non-Widget parent NSWindows, as well as the anchor NSView. (Closed)

Created:
5 years, 6 months ago by tapted
Modified:
5 years, 6 months ago
Reviewers:
jackhou1
CC:
chromium-reviews, tfarina, tdanderson+views_chromium.org, chrome-apps-syd-reviews_chromium.org, mac-views-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

MacViews: Retain non-Widget parent NSWindows, as well as the anchor NSView. Linking against the 10.6 SDK tickles an AppKit bug that causes a parent NSWindow to disappear from screen when it has a child window that closes. NativeWidgetMac tries to ensure windows are only closed when they have no parent window. However, for child Widgets with non-Widget parents, if the "anchor" NSView used to create the dialog was removed from its own view hierarchy while the child dialog is still alive, this step was being skipped. Specifically, this happens for tab-modal dialogs that have a fade out animation that can outlast the NSView it was initially anchored off. Fix by retaining the NSWindow parent when the parent-child relationship is initially created. BUG=485854 Committed: https://crrev.com/ff8d9e06d49636d3729192dbcf791469727a264d Cr-Commit-Position: refs/heads/master@{#333637}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -9 lines) Patch
M ui/views/cocoa/widget_owner_nswindow_adapter.h View 2 chunks +2 lines, -0 lines 0 comments Download
M ui/views/cocoa/widget_owner_nswindow_adapter.mm View 3 chunks +13 lines, -7 lines 0 comments Download
M ui/views/widget/native_widget_mac_unittest.mm View 2 chunks +12 lines, -2 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
tapted
Hi Jack, please take a look (also i've totally been forgetting to +cc mac-views-reviews@.. doh).
5 years, 6 months ago (2015-06-09 08:21:31 UTC) #2
jackhou1
lgtm
5 years, 6 months ago (2015-06-10 00:41:02 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1169063002/1
5 years, 6 months ago (2015-06-10 00:53:19 UTC) #5
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 6 months ago (2015-06-10 01:26:48 UTC) #6
commit-bot: I haz the power
5 years, 6 months ago (2015-06-10 01:28:40 UTC) #7
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/ff8d9e06d49636d3729192dbcf791469727a264d
Cr-Commit-Position: refs/heads/master@{#333637}

Powered by Google App Engine
This is Rietveld 408576698