Changeset 245975 in webkit

Timestamp:

May 31, 2019 11:56:36 AM (5 years ago)

Author:

ap@apple.com

Message:

WebKitTestRunner sometimes freezes under -[NSWindow release]
​https://bugs.webkit.org/show_bug.cgi?id=198422

Reviewed by Tim Horton.

The window remains key until it's out of the allWindows vector, and AppKit is not
happy about deallocating key windows. Fixed by updating allWindows in -close
instead of -release.

Added isMainFrame assertions in code that manipulates allWindows for a good measure.

• WebKitTestRunner/mac/WebKitTestRunnerWindow.mm:

(+[WebKitTestRunnerWindow _WTR_keyWindow]):
(-[WebKitTestRunnerWindow initWithContentRect:styleMask:backing:defer:]):
(-[WebKitTestRunnerWindow close]):
(-[WebKitTestRunnerWindow dealloc]):

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• WebKitTestRunner/mac/WebKitTestRunnerWindow.mm (modified) (3 diffs)

trunk/Tools/ChangeLog

@@ +1 @@
+2019-05-31  Alexey Proskuryakov  <ap@apple.com>
+
+        WebKitTestRunner sometimes freezes under -[NSWindow release]
+        https://bugs.webkit.org/show_bug.cgi?id=198422
+
+        Reviewed by Tim Horton.
+
+        The window remains key until it's out of the allWindows vector, and AppKit is not
+        happy about deallocating key windows. Fixed by updating allWindows in -close
+        instead of -release.
+
+        Added isMainFrame assertions in code that manipulates allWindows for a good measure.
+
+        * WebKitTestRunner/mac/WebKitTestRunnerWindow.mm:
+        (+[WebKitTestRunnerWindow _WTR_keyWindow]):
+        (-[WebKitTestRunnerWindow initWithContentRect:styleMask:backing:defer:]):
+        (-[WebKitTestRunnerWindow close]):
+        (-[WebKitTestRunnerWindow dealloc]):
+
 2019-05-31  Geoffrey Garen  <ggaren@apple.com>
 

trunk/Tools/WebKitTestRunner/mac/WebKitTestRunnerWindow.mm

@@ -28 +28 @@
 
 #import "PlatformWebView.h"
+#import <wtf/MainThread.h>
 #import <wtf/Vector.h>
 
@@ -38 +39 @@
 + (WebKitTestRunnerWindow *)_WTR_keyWindow
 {
+    ASSERT(isMainThread());
     for (auto window : allWindows) {
         if ([window isKeyWindow])
@@ -47 +49 @@
 - (instancetype)initWithContentRect:(NSRect)contentRect styleMask:(NSUInteger)windowStyle backing:(NSBackingStoreType)bufferingType defer:(BOOL)deferCreation
 {
+    ASSERT(isMainThread());
     allWindows.append(self);
     return [super initWithContentRect:contentRect styleMask:windowStyle backing:bufferingType defer:deferCreation];
 }
 
+- (void)close
+{
+    ASSERT(isMainThread());
+    ASSERT(allWindows.contains(self));
+    allWindows.removeFirst(self);
+    ASSERT(!allWindows.contains(self));
+    [super close];
+}
+
 - (void)dealloc
 {
-    allWindows.removeFirst(self);
-    ASSERT(!allWindows.contains(self));
+    ASSERT(isMainThread());
+    ASSERT(!allWindows.contains(self)); // The window needs to stop being key before deallocation, otherwise AppKit spins waiting for this to happen (see <rdar://problem/50948871>).
     [super dealloc];
 }