Title: [273460] branches/safari-611-branch/Source/WebKit
Revision
273460
Author
alanc...@apple.com
Date
2021-02-24 16:40:00 -0800 (Wed, 24 Feb 2021)

Log Message

Cherry-pick r272935. rdar://problem/74500849

    REGRESSION(r266890): [Cocoa] Fix InspectorDelegate / API::InspectorClient leak
    https://bugs.webkit.org/show_bug.cgi?id=221988
    <rdar://73594555>

    Reviewed by Devin Rousso.

    There was a logic error introduced into the new delegate situation after
    moving _WKInspectorDelegate from WKWebView to _WKInspector. When setting
    .delegate to nil, we shouldn't allocate dummy API::InspectorClient/
    InspectorDelegate instances. As written, these instances form their own retain
    cycle and cause a leak if Web Inspector has been opened or if
    WKWebView._inspector is accessed (which lazily creates the delegates).

    * UIProcess/API/Cocoa/_WKInspector.mm:
    (-[_WKInspector setDelegate:]):
    Adopt new constructor and pass in the ObjC delegate.

    * UIProcess/Inspector/Cocoa/InspectorDelegate.h:
    * UIProcess/Inspector/Cocoa/InspectorDelegate.mm:
    (WebKit::InspectorDelegate::InspectorDelegate):
    (WebKit::InspectorDelegate::createInspectorClient): Deleted.
    (WebKit::InspectorDelegate::setDelegate): Deleted.
    Clean up this class so that we always receive the ObjC delegate
    via the constructor. If a nil delegate is passed---for example, when
    closing the WKWebView---then don't create an API::InspectorClient
    and set the WebInspectorProxy's client to nullptr.

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

Modified Paths

Diff

Modified: branches/safari-611-branch/Source/WebKit/ChangeLog (273459 => 273460)


--- branches/safari-611-branch/Source/WebKit/ChangeLog	2021-02-25 00:39:57 UTC (rev 273459)
+++ branches/safari-611-branch/Source/WebKit/ChangeLog	2021-02-25 00:40:00 UTC (rev 273460)
@@ -1,5 +1,68 @@
 2021-02-24  Russell Epstein  <repst...@apple.com>
 
+        Cherry-pick r272935. rdar://problem/74500849
+
+    REGRESSION(r266890): [Cocoa] Fix InspectorDelegate / API::InspectorClient leak
+    https://bugs.webkit.org/show_bug.cgi?id=221988
+    <rdar://73594555>
+    
+    Reviewed by Devin Rousso.
+    
+    There was a logic error introduced into the new delegate situation after
+    moving _WKInspectorDelegate from WKWebView to _WKInspector. When setting
+    .delegate to nil, we shouldn't allocate dummy API::InspectorClient/
+    InspectorDelegate instances. As written, these instances form their own retain
+    cycle and cause a leak if Web Inspector has been opened or if
+    WKWebView._inspector is accessed (which lazily creates the delegates).
+    
+    * UIProcess/API/Cocoa/_WKInspector.mm:
+    (-[_WKInspector setDelegate:]):
+    Adopt new constructor and pass in the ObjC delegate.
+    
+    * UIProcess/Inspector/Cocoa/InspectorDelegate.h:
+    * UIProcess/Inspector/Cocoa/InspectorDelegate.mm:
+    (WebKit::InspectorDelegate::InspectorDelegate):
+    (WebKit::InspectorDelegate::createInspectorClient): Deleted.
+    (WebKit::InspectorDelegate::setDelegate): Deleted.
+    Clean up this class so that we always receive the ObjC delegate
+    via the constructor. If a nil delegate is passed---for example, when
+    closing the WKWebView---then don't create an API::InspectorClient
+    and set the WebInspectorProxy's client to nullptr.
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@272935 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2021-02-16  BJ Burg  <bb...@apple.com>
+
+            REGRESSION(r266890): [Cocoa] Fix InspectorDelegate / API::InspectorClient leak
+            https://bugs.webkit.org/show_bug.cgi?id=221988
+            <rdar://73594555>
+
+            Reviewed by Devin Rousso.
+
+            There was a logic error introduced into the new delegate situation after
+            moving _WKInspectorDelegate from WKWebView to _WKInspector. When setting
+            .delegate to nil, we shouldn't allocate dummy API::InspectorClient/
+            InspectorDelegate instances. As written, these instances form their own retain
+            cycle and cause a leak if Web Inspector has been opened or if
+            WKWebView._inspector is accessed (which lazily creates the delegates).
+
+            * UIProcess/API/Cocoa/_WKInspector.mm:
+            (-[_WKInspector setDelegate:]):
+            Adopt new constructor and pass in the ObjC delegate.
+
+            * UIProcess/Inspector/Cocoa/InspectorDelegate.h:
+            * UIProcess/Inspector/Cocoa/InspectorDelegate.mm:
+            (WebKit::InspectorDelegate::InspectorDelegate):
+            (WebKit::InspectorDelegate::createInspectorClient): Deleted.
+            (WebKit::InspectorDelegate::setDelegate): Deleted.
+            Clean up this class so that we always receive the ObjC delegate
+            via the constructor. If a nil delegate is passed---for example, when
+            closing the WKWebView---then don't create an API::InspectorClient
+            and set the WebInspectorProxy's client to nullptr.
+
+2021-02-24  Russell Epstein  <repst...@apple.com>
+
         Cherry-pick r272629. rdar://problem/74410251
 
     Make sure we are no longer show the previous page when running a JS prompt

Modified: branches/safari-611-branch/Source/WebKit/UIProcess/API/Cocoa/_WKInspector.mm (273459 => 273460)


--- branches/safari-611-branch/Source/WebKit/UIProcess/API/Cocoa/_WKInspector.mm	2021-02-25 00:39:57 UTC (rev 273459)
+++ branches/safari-611-branch/Source/WebKit/UIProcess/API/Cocoa/_WKInspector.mm	2021-02-25 00:40:00 UTC (rev 273460)
@@ -57,11 +57,10 @@
 
 - (void)setDelegate:(id<_WKInspectorDelegate>)delegate
 {
-    if (!_delegate)
-        _delegate = makeUnique<WebKit::InspectorDelegate>(self);
+    if (!delegate && !_delegate)
+        return;
 
-    _inspector->setInspectorClient(_delegate->createInspectorClient());
-    _delegate->setDelegate(delegate);
+    _delegate = makeUnique<WebKit::InspectorDelegate>(self, delegate);
 }
 
 - (WKWebView *)webView

Modified: branches/safari-611-branch/Source/WebKit/UIProcess/Inspector/Cocoa/InspectorDelegate.h (273459 => 273460)


--- branches/safari-611-branch/Source/WebKit/UIProcess/Inspector/Cocoa/InspectorDelegate.h	2021-02-25 00:39:57 UTC (rev 273459)
+++ branches/safari-611-branch/Source/WebKit/UIProcess/Inspector/Cocoa/InspectorDelegate.h	2021-02-25 00:40:00 UTC (rev 273460)
@@ -40,10 +40,9 @@
 class InspectorDelegate {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    explicit InspectorDelegate(_WKInspector *);
+    InspectorDelegate(_WKInspector *, id <_WKInspectorDelegate>);
+    ~InspectorDelegate();
 
-    std::unique_ptr<API::InspectorClient> createInspectorClient();
-
     RetainPtr<id <_WKInspectorDelegate>> delegate();
     void setDelegate(id <_WKInspectorDelegate>);
 

Modified: branches/safari-611-branch/Source/WebKit/UIProcess/Inspector/Cocoa/InspectorDelegate.mm (273459 => 273460)


--- branches/safari-611-branch/Source/WebKit/UIProcess/Inspector/Cocoa/InspectorDelegate.mm	2021-02-25 00:39:57 UTC (rev 273459)
+++ branches/safari-611-branch/Source/WebKit/UIProcess/Inspector/Cocoa/InspectorDelegate.mm	2021-02-25 00:40:00 UTC (rev 273460)
@@ -33,30 +33,24 @@
 
 namespace WebKit {
 
-InspectorDelegate::InspectorDelegate(_WKInspector *inspector)
+InspectorDelegate::InspectorDelegate(_WKInspector *inspector, id <_WKInspectorDelegate> delegate)
     : m_inspector(inspector)
+    , m_delegate(delegate)
 {
-}
+    m_delegateMethods.inspectorDidEnableBrowserDomain = [delegate respondsToSelector:@selector(inspectorDidEnableBrowserDomain:)];
+    m_delegateMethods.inspectorDidDisableBrowserDomain = [delegate respondsToSelector:@selector(inspectorDidDisableBrowserDomain:)];
+    m_delegateMethods.inspectorOpenURLExternally = [delegate respondsToSelector:@selector(inspector:openURLExternally:)];
 
-std::unique_ptr<API::InspectorClient> InspectorDelegate::createInspectorClient()
-{
-    return makeUnique<InspectorClient>(*this);
+    inspector->_inspector->setInspectorClient(delegate ? makeUnique<InspectorClient>(*this) : nullptr);
 }
 
+InspectorDelegate::~InspectorDelegate() = default;
+
 RetainPtr<id <_WKInspectorDelegate>> InspectorDelegate::delegate()
 {
     return m_delegate.get();
 }
 
-void InspectorDelegate::setDelegate(id <_WKInspectorDelegate> delegate)
-{
-    m_delegate = delegate;
-
-    m_delegateMethods.inspectorDidEnableBrowserDomain = [delegate respondsToSelector:@selector(inspectorDidEnableBrowserDomain:)];
-    m_delegateMethods.inspectorDidDisableBrowserDomain = [delegate respondsToSelector:@selector(inspectorDidDisableBrowserDomain:)];
-    m_delegateMethods.inspectorOpenURLExternally = [delegate respondsToSelector:@selector(inspector:openURLExternally:)];
-}
-
 InspectorDelegate::InspectorClient::InspectorClient(InspectorDelegate& delegate)
     : m_inspectorDelegate(delegate)
 {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to