Reviewed by Ollie

        - fixed <rdar://problem/5408186> REGRESSION (5522-5523.9): Safari leaks every browser window

        The leak started occurring when we removed the code to clear the delegates and the host window
        from Safari as part of the fix for 5479443. But it turns out that Safari code was masking a
        bug here in WebView: setHostWindow:nil needs to be called before setting _private->closed to
        YES, or it will do nothing at all, causing a world leak due to a circular reference between
        the window and the WebView.

        I toyed with a more complex fix, but this is the simplest one that retains the fix for 5479443
        while otherwise restoring the code order to be as close as possible to what it was before
        5479443 was fixed.

        * WebView/WebView.mm:
        (-[WebView _close]):
        Moved the call that sets _private->closed to YES to be after the code that clears the delegates
        and the host window. Added a comment about this order.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@25792 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebKit/ChangeLog b/WebKit/ChangeLog
index 060d2969437b3abe6dc336185d6c12411cb4d2f5..b9c71ee1d777ac71e674db7d3d11dda0641af780 100644 (file)
--- a/WebKit/ChangeLog
+++ b/WebKit/ChangeLog
@@ -1,3 +1,24 @@
+2007-09-27  John Sullivan  <sullivan@apple.com>
+
+        Reviewed by Ollie
+        
+        - fixed <rdar://problem/5408186> REGRESSION (5522-5523.9): Safari leaks every browser window
+        
+        The leak started occurring when we removed the code to clear the delegates and the host window
+        from Safari as part of the fix for 5479443. But it turns out that Safari code was masking a
+        bug here in WebView: setHostWindow:nil needs to be called before setting _private->closed to
+        YES, or it will do nothing at all, causing a world leak due to a circular reference between
+        the window and the WebView.
+        
+        I toyed with a more complex fix, but this is the simplest one that retains the fix for 5479443
+        while otherwise restoring the code order to be as close as possible to what it was before
+        5479443 was fixed.
+
+        * WebView/WebView.mm:
+        (-[WebView _close]):
+        Moved the call that sets _private->closed to YES to be after the code that clears the delegates
+        and the host window. Added a comment about this order.
+
 2007-09-27  Kevin Decker  <kdecker@apple.com>
 
         Rubber stamped by Darin.

diff --git a/WebKit/WebView/WebView.mm b/WebKit/WebView/WebView.mm
index fdcb5b8b5bce0030e58c9afcfc84948d17255e8b..33b5e04dab5f752c332b3249c4e45ad56f185a07 100644 (file)
--- a/WebKit/WebView/WebView.mm
+++ b/WebKit/WebView/WebView.mm
@@ -679,7 +679,6 @@ static bool debugWidget = true;
 {
     if (!_private || _private->closed)
         return;
-    _private->closed = YES;
 
     FrameLoader* mainFrameLoader = [[self mainFrame] _frameLoader];
     if (mainFrameLoader)
@@ -696,6 +695,9 @@ static bool debugWidget = true;
     [self setResourceLoadDelegate:nil];
     [self setScriptDebugDelegate:nil];
     [self setUIDelegate:nil];
+    
+    // setHostWindow:nil must be called before this value is set (see 5408186)
+    _private->closed = YES;
 
     // To avoid leaks, call removeDragCaret in case it wasn't called after moveDragCaretToPoint.
     [self removeDragCaret];