Diff
Modified: trunk/Source/WebKit/ChangeLog (275392 => 275393)
--- trunk/Source/WebKit/ChangeLog 2021-04-01 23:06:51 UTC (rev 275392)
+++ trunk/Source/WebKit/ChangeLog 2021-04-01 23:32:22 UTC (rev 275393)
@@ -1,3 +1,35 @@
+2021-04-01 BJ Burg <[email protected]>
+
+ v2: REGRESSION(r266890): [Cocoa] Fix API::InspectorClient leak
+ https://bugs.webkit.org/show_bug.cgi?id=223899
+ <rdar://problem/75249282>
+
+ Reviewed by Devin Rousso.
+
+ Refactor to *not* use the helper ObjC class InspectorDelegate.
+ Instead, store the _WKInspectorDelegate in _WKInspector directly
+ using a WeakObjCPtr ivar. Move the C++ bridge class to be defined
+ inside _WKInspector.mm since it's only used there. Adapt it to
+ work better with a nil delegate.
+
+ * UIProcess/API/APIInspectorClient.h:
+ (API::InspectorClient::openURLExternally):
+
+ * UIProcess/API/Cocoa/_WKInspectorInternal.h:
+ * UIProcess/API/Cocoa/_WKInspector.mm:
+ (-[_WKInspector delegate]):
+ (-[_WKInspector setDelegate:]):
+ (-[_WKInspector dealloc]):
+
+ * SourcesCocoa.txt:
+ * WebKit.xcodeproj/project.pbxproj:
+ * UIProcess/Inspector/Cocoa/InspectorDelegate.h: Removed.
+ * UIProcess/Inspector/Cocoa/InspectorDelegate.mm: Removed.
+
+ * UIProcess/Inspector/mac/WKInspectorViewController.mm:
+ (-[WKInspectorViewController initWithConfiguration:inspectedPage:]):
+ Drive-by, fix the leak of _WKInspectorConfiguration.
+
2021-04-01 Chris Dumez <[email protected]>
Share same code between network process termination and crash handling
Modified: trunk/Source/WebKit/SourcesCocoa.txt (275392 => 275393)
--- trunk/Source/WebKit/SourcesCocoa.txt 2021-04-01 23:06:51 UTC (rev 275392)
+++ trunk/Source/WebKit/SourcesCocoa.txt 2021-04-01 23:32:22 UTC (rev 275393)
@@ -421,7 +421,6 @@
UIProcess/Gamepad/ios/UIGamepadProviderIOS.mm
UIProcess/Gamepad/mac/UIGamepadProviderMac.mm
-UIProcess/Inspector/Cocoa/InspectorDelegate.mm
UIProcess/Inspector/Cocoa/InspectorExtensionDelegate.mm
UIProcess/Inspector/ios/WKInspectorHighlightView.mm
Modified: trunk/Source/WebKit/UIProcess/API/APIInspectorClient.h (275392 => 275393)
--- trunk/Source/WebKit/UIProcess/API/APIInspectorClient.h 2021-04-01 23:06:51 UTC (rev 275392)
+++ trunk/Source/WebKit/UIProcess/API/APIInspectorClient.h 2021-04-01 23:32:22 UTC (rev 275393)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2020 Apple Inc. All rights reserved.
+ * Copyright (C) 2020-2021 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/_WKInspector.mm (275392 => 275393)
--- trunk/Source/WebKit/UIProcess/API/Cocoa/_WKInspector.mm 2021-04-01 23:06:51 UTC (rev 275392)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/_WKInspector.mm 2021-04-01 23:32:22 UTC (rev 275393)
@@ -26,12 +26,14 @@
#import "config.h"
#import "_WKInspectorInternal.h"
-#import "InspectorDelegate.h"
+#import "APIInspectorClient.h"
#import "WKError.h"
#import "WKWebViewInternal.h"
+#import "WebInspectorUIProxy.h"
#import "WebPageProxy.h"
#import "WebProcessProxy.h"
#import "_WKFrameHandleInternal.h"
+#import "_WKInspectorDelegate.h"
#import "_WKInspectorPrivateForTesting.h"
#import "_WKRemoteObjectRegistry.h"
#import <WebCore/FrameIdentifier.h>
@@ -47,6 +49,31 @@
#import <wtf/BlockPtr.h>
#endif
+class InspectorClient final : public API::InspectorClient {
+public:
+ explicit InspectorClient(id <_WKInspectorDelegate> delegate)
+ : m_delegate(delegate)
+ , m_respondsToInspectorOpenURLExternally([delegate respondsToSelector:@selector(inspector:openURLExternally:)])
+ {
+ }
+
+ ~InspectorClient() = default;
+
+private:
+ // API::InspectorClient
+ void openURLExternally(WebKit::WebInspectorUIProxy& inspector, const WTF::String& url) final
+ {
+ if (!m_delegate || !m_respondsToInspectorOpenURLExternally)
+ return;
+
+ [m_delegate inspector:wrapper(inspector) openURLExternally:[NSURL URLWithString:url]];
+ }
+
+ WeakObjCPtr<id <_WKInspectorDelegate> > m_delegate;
+
+ bool m_respondsToInspectorOpenURLExternally : 1;
+};
+
@implementation _WKInspector
// MARK: _WKInspector methods
@@ -53,15 +80,13 @@
- (id <_WKInspectorDelegate>)delegate
{
- return _delegate->delegate().autorelease();
+ return _delegate.get().get();
}
- (void)setDelegate:(id<_WKInspectorDelegate>)delegate
{
- if (!delegate && !_delegate)
- return;
-
- _delegate = makeUnique<WebKit::InspectorDelegate>(self, delegate);
+ _delegate = delegate;
+ _inspector->setInspectorClient(WTF::makeUnique<InspectorClient>(delegate));
}
- (WKWebView *)webView
@@ -176,6 +201,14 @@
return *_inspector;
}
+- (void)dealloc
+{
+ if (WebCoreObjCScheduleDeallocateOnMainRunLoop(_WKInspector.class, self))
+ return;
+
+ [super dealloc];
+}
+
// MARK: _WKInspectorExtensionHost methods
- (WKWebView *)extensionHostWebView
Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/_WKInspectorInternal.h (275392 => 275393)
--- trunk/Source/WebKit/UIProcess/API/Cocoa/_WKInspectorInternal.h 2021-04-01 23:06:51 UTC (rev 275392)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/_WKInspectorInternal.h 2021-04-01 23:32:22 UTC (rev 275393)
@@ -29,11 +29,7 @@
#import "WebInspectorUIProxy.h"
namespace WebKit {
-class InspectorDelegate;
-}
-namespace WebKit {
-
template<> struct WrapperTraits<WebInspectorUIProxy> {
using WrapperClass = _WKInspector;
};
@@ -43,6 +39,6 @@
@interface _WKInspector () <WKObject> {
@package
API::ObjectStorage<WebKit::WebInspectorUIProxy> _inspector;
- std::unique_ptr<WebKit::InspectorDelegate> _delegate;
+ WeakObjCPtr<id <_WKInspectorDelegate> > _delegate;
}
@end
Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/_WKInspectorTesting.mm (275392 => 275393)
--- trunk/Source/WebKit/UIProcess/API/Cocoa/_WKInspectorTesting.mm 2021-04-01 23:06:51 UTC (rev 275392)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/_WKInspectorTesting.mm 2021-04-01 23:32:22 UTC (rev 275393)
@@ -49,11 +49,11 @@
- (void)_openURLExternallyForTesting:(NSURL *)url useFrontendAPI:(BOOL)useFrontendAPI
{
if (useFrontendAPI)
- _inspector->evaluateInFrontendForTesting(_javascript_SnippetToOpenURLExternally(url));
+ _inspector->evaluateInFrontendForTesting(_javascript_SnippetToOpenURLExternally([url copy]));
else {
// Force the navigation request to be handled naturally through the
// internal NavigationDelegate of WKInspectorViewController.
- [self.inspectorWebView loadRequest:[NSURLRequest requestWithURL:url]];
+ [self.inspectorWebView loadRequest:[NSURLRequest requestWithURL:[url copy]]];
}
}
Deleted: trunk/Source/WebKit/UIProcess/Inspector/Cocoa/InspectorDelegate.h (275392 => 275393)
--- trunk/Source/WebKit/UIProcess/Inspector/Cocoa/InspectorDelegate.h 2021-04-01 23:06:51 UTC (rev 275392)
+++ trunk/Source/WebKit/UIProcess/Inspector/Cocoa/InspectorDelegate.h 2021-04-01 23:32:22 UTC (rev 275393)
@@ -1,70 +0,0 @@
-/*
- * Copyright (C) 2020 Apple Inc. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
- * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
- * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
- * THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#pragma once
-
-#import "APIInspectorClient.h"
-#import "WKFoundation.h"
-#import <wtf/WeakObjCPtr.h>
-
-@class _WKInspector;
-@protocol _WKInspectorDelegate;
-
-namespace WebKit {
-
-class WebInspectorUIProxy;
-
-class InspectorDelegate {
- WTF_MAKE_FAST_ALLOCATED;
-public:
- InspectorDelegate(_WKInspector *, id <_WKInspectorDelegate>);
- ~InspectorDelegate();
-
- RetainPtr<id <_WKInspectorDelegate>> delegate();
- void setDelegate(id <_WKInspectorDelegate>);
-
-private:
- class InspectorClient final : public API::InspectorClient {
- WTF_MAKE_FAST_ALLOCATED;
- public:
- explicit InspectorClient(InspectorDelegate&);
- ~InspectorClient();
-
- private:
- // API::InspectorClient
- void openURLExternally(WebInspectorUIProxy&, const String& url);
-
- InspectorDelegate& m_inspectorDelegate;
- };
-
- WeakObjCPtr<_WKInspector> m_inspector;
- WeakObjCPtr<id <_WKInspectorDelegate>> m_delegate;
-
- struct {
- bool inspectorOpenURLExternally : 1;
- } m_delegateMethods;
-};
-
-} // namespace WebKit
Deleted: trunk/Source/WebKit/UIProcess/Inspector/Cocoa/InspectorDelegate.mm (275392 => 275393)
--- trunk/Source/WebKit/UIProcess/Inspector/Cocoa/InspectorDelegate.mm 2021-04-01 23:06:51 UTC (rev 275392)
+++ trunk/Source/WebKit/UIProcess/Inspector/Cocoa/InspectorDelegate.mm 2021-04-01 23:32:22 UTC (rev 275393)
@@ -1,72 +0,0 @@
-/*
- * Copyright (C) 2020 Apple Inc. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
- * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
- * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
- * THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#import "config.h"
-#import "InspectorDelegate.h"
-
-#import "WebInspectorUIProxy.h"
-#import "_WKInspectorDelegate.h"
-#import "_WKInspectorInternal.h"
-
-namespace WebKit {
-
-InspectorDelegate::InspectorDelegate(_WKInspector *inspector, id <_WKInspectorDelegate> delegate)
- : m_inspector(inspector)
- , m_delegate(delegate)
-{
- m_delegateMethods.inspectorOpenURLExternally = [delegate respondsToSelector:@selector(inspector:openURLExternally:)];
-
- inspector->_inspector->setInspectorClient(delegate ? makeUnique<InspectorClient>(*this) : nullptr);
-}
-
-InspectorDelegate::~InspectorDelegate() = default;
-
-RetainPtr<id <_WKInspectorDelegate>> InspectorDelegate::delegate()
-{
- return m_delegate.get();
-}
-
-InspectorDelegate::InspectorClient::InspectorClient(InspectorDelegate& delegate)
- : m_inspectorDelegate(delegate)
-{
-}
-
-InspectorDelegate::InspectorClient::~InspectorClient()
-{
-}
-
-void InspectorDelegate::InspectorClient::openURLExternally(WebInspectorUIProxy&, const String& url)
-{
- if (!m_inspectorDelegate.m_delegateMethods.inspectorOpenURLExternally)
- return;
-
- auto& delegate = m_inspectorDelegate.m_delegate;
- if (!delegate)
- return;
-
- [delegate inspector:m_inspectorDelegate.m_inspector.get().get() openURLExternally:[NSURL URLWithString:url]];
-}
-
-} // namespace WebKit
Modified: trunk/Source/WebKit/UIProcess/Inspector/WebInspectorUIProxy.cpp (275392 => 275393)
--- trunk/Source/WebKit/UIProcess/Inspector/WebInspectorUIProxy.cpp 2021-04-01 23:06:51 UTC (rev 275392)
+++ trunk/Source/WebKit/UIProcess/Inspector/WebInspectorUIProxy.cpp 2021-04-01 23:32:22 UTC (rev 275393)
@@ -84,7 +84,7 @@
void WebInspectorUIProxy::setInspectorClient(std::unique_ptr<API::InspectorClient>&& inspectorClient)
{
if (!inspectorClient) {
- m_inspectorClient = makeUnique<API::InspectorClient>();
+ m_inspectorClient = nullptr;
return;
}
@@ -634,7 +634,8 @@
void WebInspectorUIProxy::openURLExternally(const String& url)
{
- m_inspectorClient->openURLExternally(*this, url);
+ if (m_inspectorClient)
+ m_inspectorClient->openURLExternally(*this, url);
}
void WebInspectorUIProxy::inspectedURLChanged(const String& urlString)
Modified: trunk/Source/WebKit/UIProcess/Inspector/mac/WKInspectorViewController.mm (275392 => 275393)
--- trunk/Source/WebKit/UIProcess/Inspector/mac/WKInspectorViewController.mm 2021-04-01 23:06:51 UTC (rev 275392)
+++ trunk/Source/WebKit/UIProcess/Inspector/mac/WKInspectorViewController.mm 2021-04-01 23:32:22 UTC (rev 275393)
@@ -56,7 +56,7 @@
NakedPtr<WebKit::WebPageProxy> _inspectedPage;
RetainPtr<WKInspectorWKWebView> _webView;
WeakObjCPtr<id <WKInspectorViewControllerDelegate>> _delegate;
- _WKInspectorConfiguration *_configuration;
+ RetainPtr<_WKInspectorConfiguration> _configuration;
}
- (instancetype)initWithConfiguration:(_WKInspectorConfiguration *)configuration inspectedPage:(NakedPtr<WebKit::WebPageProxy>)inspectedPage
@@ -64,7 +64,7 @@
if (!(self = [super init]))
return nil;
- _configuration = [configuration copy];
+ _configuration = adoptNS([configuration copy]);
// The (local) inspected page is nil if the controller is hosting a Remote Web Inspector view.
_inspectedPage = inspectedPage;
Modified: trunk/Source/WebKit/WebKit.xcodeproj/project.pbxproj (275392 => 275393)
--- trunk/Source/WebKit/WebKit.xcodeproj/project.pbxproj 2021-04-01 23:06:51 UTC (rev 275392)
+++ trunk/Source/WebKit/WebKit.xcodeproj/project.pbxproj 2021-04-01 23:32:22 UTC (rev 275393)
@@ -1429,7 +1429,6 @@
86F9536518FF58F5001DB2EF /* ProcessAssertion.h in Headers */ = {isa = PBXBuildFile; fileRef = 86F9536018FF4FD4001DB2EF /* ProcessAssertion.h */; };
8DC2EF530486A6940098B216 /* InfoPlist.strings in Resources */ = {isa = PBXBuildFile; fileRef = 089C1666FE841158C02AAC07 /* InfoPlist.strings */; };
909854ED12BC4E18000AD080 /* WebMemorySampler.h in Headers */ = {isa = PBXBuildFile; fileRef = 905620E912BC248B000799B6 /* WebMemorySampler.h */; };
- 9197940223DBC49000257892 /* InspectorDelegate.h in Headers */ = {isa = PBXBuildFile; fileRef = 9197940023DBC49000257892 /* InspectorDelegate.h */; };
9197940523DBC4BB00257892 /* InspectorBrowserAgent.h in Headers */ = {isa = PBXBuildFile; fileRef = 9197940323DBC4BB00257892 /* InspectorBrowserAgent.h */; };
9197940823DBC4CB00257892 /* WebPageInspectorAgentBase.h in Headers */ = {isa = PBXBuildFile; fileRef = 9197940723DBC4CA00257892 /* WebPageInspectorAgentBase.h */; };
9197940A23DBC4E000257892 /* APIInspectorClient.h in Headers */ = {isa = PBXBuildFile; fileRef = 9197940923DBC4E000257892 /* APIInspectorClient.h */; };
@@ -4756,8 +4755,6 @@
905620E512BC2476000799B6 /* WebMemorySampler.mac.mm */ = {isa = PBXFileReference; fileEncoding = 4; indentWidth = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WebMemorySampler.mac.mm; sourceTree = "<group>"; tabWidth = 8; usesTabs = 0; };
905620E812BC248B000799B6 /* WebMemorySampler.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WebMemorySampler.cpp; sourceTree = "<group>"; };
905620E912BC248B000799B6 /* WebMemorySampler.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WebMemorySampler.h; sourceTree = "<group>"; };
- 919793FF23DBC49000257892 /* InspectorDelegate.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = InspectorDelegate.mm; sourceTree = "<group>"; };
- 9197940023DBC49000257892 /* InspectorDelegate.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = InspectorDelegate.h; sourceTree = "<group>"; };
9197940323DBC4BB00257892 /* InspectorBrowserAgent.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = InspectorBrowserAgent.h; sourceTree = "<group>"; };
9197940423DBC4BB00257892 /* InspectorBrowserAgent.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = InspectorBrowserAgent.cpp; sourceTree = "<group>"; };
9197940723DBC4CA00257892 /* WebPageInspectorAgentBase.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WebPageInspectorAgentBase.h; sourceTree = "<group>"; };
@@ -9256,8 +9253,6 @@
919793FE23DBC47400257892 /* Cocoa */ = {
isa = PBXGroup;
children = (
- 9197940023DBC49000257892 /* InspectorDelegate.h */,
- 919793FF23DBC49000257892 /* InspectorDelegate.mm */,
996B2B9C25E257EE00719379 /* InspectorExtensionDelegate.h */,
996B2B9B25E257EE00719379 /* InspectorExtensionDelegate.mm */,
);
@@ -11856,7 +11851,6 @@
2DD45ADE1E5F8972006C355F /* InputViewUpdateDeferrer.h in Headers */,
CE550E152283752200D28791 /* InsertTextOptions.h in Headers */,
9197940523DBC4BB00257892 /* InspectorBrowserAgent.h in Headers */,
- 9197940223DBC49000257892 /* InspectorDelegate.h in Headers */,
996B2B9D25E257FF00719379 /* InspectorExtensionDelegate.h in Headers */,
A5E391FD2183C1F800C8FB31 /* InspectorTargetProxy.h in Headers */,
C5BCE5DF1C50766A00CDE3FA /* InteractionInformationAtPosition.h in Headers */,
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKInspectorDelegate.mm (275392 => 275393)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKInspectorDelegate.mm 2021-04-01 23:06:51 UTC (rev 275392)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKInspectorDelegate.mm 2021-04-01 23:32:22 UTC (rev 275393)
@@ -221,7 +221,7 @@
[[webView _inspector] show];
TestWebKitAPI::Util::run(&didAttachLocalInspectorCalled);
- urlToOpen = [NSURL URLWithString:@"https://www.webkit.org/"];
+ urlToOpen = adoptNS([NSURL URLWithString:@"https://www.webkit.org/"]);
// Check the case where the load is intercepted by the navigation delegate.
[[webView _inspector] _openURLExternallyForTesting:urlToOpen.get() useFrontendAPI:NO];
