Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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.



Canonical link: https://commits.webkit.org/20235@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@25792 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
John Sullivan committed Sep 28, 2007
1 parent 2f8b3a7 commit 910fdbe
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 1 deletion.
21 changes: 21 additions & 0 deletions 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.
Expand Down
4 changes: 3 additions & 1 deletion WebKit/WebView/WebView.mm
Expand Up @@ -679,7 +679,6 @@ - (void)_close
{
if (!_private || _private->closed)
return;
_private->closed = YES;

FrameLoader* mainFrameLoader = [[self mainFrame] _frameLoader];
if (mainFrameLoader)
Expand All @@ -696,6 +695,9 @@ - (void)_close
[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];
Expand Down

0 comments on commit 910fdbe

Please sign in to comment.