Skip to content

Commit

Permalink
_WKWebExtensionMessagePort is being deallocated on a background thread.
Browse files Browse the repository at this point in the history
https://webkit.org/b/270012
rdar://123529626

Reviewed by Darin Adler.

Introduce WK_OBJECT_DEALLOC_ON_MAIN_THREAD() and WK_OBJECT_DEALLOC_IMPL_ON_MAIN_THREAD()
macros that use _class_setCustomDeallocInitiation() to be able to dealloc on the main
thread for both ARC and non-ARC objects.

Start using these macros in the Web Extensions classes since they stopped using
WebCoreObjCScheduleDeallocateOnMainRunLoop(). All objects currently using the WebCore
dealloc helper can be switched over to use these new macros.

* Source/WTF/wtf/PlatformHave.h: Added HAVE_OBJC_CUSTOM_DEALLOC.
* Source/WTF/wtf/spi/cocoa/objcSPI.h: Added _class_setCustomDeallocInitiation and
_objc_deallocOnMainThreadHelper.
* Source/WebKit/Shared/Cocoa/WKObject.h: Added WK_OBJECT_DEALLOC_ON_MAIN_THREAD and
WK_OBJECT_DEALLOC_IMPL_ON_MAIN_THREAD.
* Source/WebKit/UIProcess/API/Cocoa/_WKWebExtension.mm:
(-[_WKWebExtension dealloc]): Deleted.
* Source/WebKit/UIProcess/API/Cocoa/_WKWebExtensionAction.mm:
(-[_WKWebExtensionAction dealloc]): Deleted.
* Source/WebKit/UIProcess/API/Cocoa/_WKWebExtensionCommand.mm:
(-[_WKWebExtensionCommand dealloc]): Deleted.
* Source/WebKit/UIProcess/API/Cocoa/_WKWebExtensionContext.mm:
(-[_WKWebExtensionContext initForExtension:]):
(-[_WKWebExtensionContext dealloc]): Deleted.
* Source/WebKit/UIProcess/API/Cocoa/_WKWebExtensionController.mm:
(-[_WKWebExtensionController dealloc]): Deleted.
* Source/WebKit/UIProcess/API/Cocoa/_WKWebExtensionControllerConfiguration.mm:
(-[_WKWebExtensionControllerConfiguration dealloc]): Deleted.
* Source/WebKit/UIProcess/API/Cocoa/_WKWebExtensionDataRecord.mm:
(-[_WKWebExtensionDataRecord dealloc]): Deleted.
* Source/WebKit/UIProcess/API/Cocoa/_WKWebExtensionMatchPattern.mm:
(-[_WKWebExtensionMatchPattern initWithCoder:]):
(-[_WKWebExtensionMatchPattern encodeWithCoder:]):
(-[_WKWebExtensionMatchPattern copyWithZone:]):
(-[_WKWebExtensionMatchPattern dealloc]): Deleted.
* Source/WebKit/UIProcess/API/Cocoa/_WKWebExtensionMessagePort.mm:
(-[_WKWebExtensionMessagePort dealloc]): Deleted.

Canonical link: https://commits.webkit.org/275272@main
  • Loading branch information
xeenon committed Feb 24, 2024
1 parent 0a519d8 commit dc20f3f
Show file tree
Hide file tree
Showing 12 changed files with 114 additions and 73 deletions.
9 changes: 9 additions & 0 deletions Source/WTF/wtf/PlatformHave.h
Original file line number Diff line number Diff line change
Expand Up @@ -1740,3 +1740,12 @@
&& PLATFORM(COCOA) && !PLATFORM(MACCATALYST)
#define HAVE_SECTRUST_COPYPROPERTIES 1
#endif

#if !defined(HAVE_OBJC_CUSTOM_DEALLOC) \
&& ((PLATFORM(MAC) && __MAC_OS_X_VERSION_MAX_ALLOWED >= 130000) \
|| ((PLATFORM(IOS) || PLATFORM(MACCATALYST)) && __IPHONE_OS_VERSION_MAX_ALLOWED >= 160000) \
|| (PLATFORM(APPLETV) && __TV_OS_VERSION_MAX_ALLOWED >= 160000) \
|| (PLATFORM(WATCHOS) && __WATCH_OS_VERSION_MAX_ALLOWED >= 90000) \
|| PLATFORM(VISION))
#define HAVE_OBJC_CUSTOM_DEALLOC 1
#endif
3 changes: 3 additions & 0 deletions Source/WTF/wtf/spi/cocoa/objcSPI.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,7 @@ void objc_destroyWeak(id*);
void objc_copyWeak(id*, id*);
void objc_moveWeak(id*, id*);

void _class_setCustomDeallocInitiation(Class);
void _objc_deallocOnMainThreadHelper(void* object);

WTF_EXTERN_C_END
54 changes: 54 additions & 0 deletions Source/WebKit/Shared/Cocoa/WKObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@
#import "WKFoundation.h"

#import <type_traits>
#import <wtf/ObjCRuntimeExtras.h>
#import <wtf/RefPtr.h>
#import <wtf/RetainPtr.h>
#import <wtf/spi/cocoa/objcSPI.h>

namespace API {

Expand Down Expand Up @@ -105,3 +107,55 @@ using WebKit::wrapper;
- (NSObject *)_web_createTarget NS_RETURNS_RETAINED;

@end

#if HAVE(OBJC_CUSTOM_DEALLOC)

// This macro ensures WebKit ObjC objects of a specified class are deallocated on the main thread.
// Use this macro in the ObjC implementation file.

#define WK_OBJECT_DEALLOC_ON_MAIN_THREAD(objcClass) \
+ (void)initialize \
{ \
if (self == objcClass.class) \
_class_setCustomDeallocInitiation(self); \
} \
\
- (void)_objc_initiateDealloc \
{ \
if (isMainRunLoop()) \
_objc_deallocOnMainThreadHelper((__bridge void *)self); \
else \
dispatch_async_f(dispatch_get_main_queue(), (__bridge void *)self, _objc_deallocOnMainThreadHelper); \
} \
\
using __thisIsHereToForceASemicolonAfterThisMacro UNUSED_TYPE_ALIAS = int

// This macro ensures WebKit ObjC objects and their C++ implementation are safely deallocated on the main thread.
// Use this macro in the ObjC implementation file if you don't require a custom dealloc method.

#define WK_OBJECT_DEALLOC_IMPL_ON_MAIN_THREAD(objcClass, implClass, storageVar) \
WK_OBJECT_DEALLOC_ON_MAIN_THREAD(objcClass); \
\
- (void)dealloc \
{ \
ASSERT(isMainRunLoop()); \
storageVar->~implClass(); \
} \
\
using __thisIsHereToForceASemicolonAfterThisMacro UNUSED_TYPE_ALIAS = int

#else

#define WK_OBJECT_DEALLOC_ON_MAIN_THREAD(objcClass) \
using __thisIsHereToForceASemicolonAfterThisMacro UNUSED_TYPE_ALIAS = int

#define WK_OBJECT_DEALLOC_IMPL_ON_MAIN_THREAD(objcClass, implClass, storageVar) \
- (void)dealloc \
{ \
ASSERT(isMainRunLoop()); \
storageVar->~implClass(); \
} \
\
using __thisIsHereToForceASemicolonAfterThisMacro UNUSED_TYPE_ALIAS = int

#endif // HAVE(OBJC_CUSTOM_DEALLOC)
9 changes: 2 additions & 7 deletions Source/WebKit/UIProcess/API/Cocoa/_WKWebExtension.mm
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ @implementation _WKWebExtension

#if ENABLE(WK_WEB_EXTENSIONS)

WK_OBJECT_DEALLOC_IMPL_ON_MAIN_THREAD(_WKWebExtension, WebExtension, _webExtension);

+ (instancetype)extensionWithAppExtensionBundle:(NSBundle *)appExtensionBundle
{
NSParameterAssert([appExtensionBundle isKindOfClass:NSBundle.class]);
Expand Down Expand Up @@ -146,13 +148,6 @@ - (instancetype)_initWithResources:(NSDictionary<NSString *, id> *)resources
return self;
}

- (void)dealloc
{
ASSERT(isMainRunLoop());

_webExtension->~WebExtension();
}

- (NSDictionary<NSString *, id> *)manifest
{
return _webExtension->manifest();
Expand Down
7 changes: 1 addition & 6 deletions Source/WebKit/UIProcess/API/Cocoa/_WKWebExtensionAction.mm
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,7 @@ @implementation _WKWebExtensionAction

#if ENABLE(WK_WEB_EXTENSIONS)

- (void)dealloc
{
ASSERT(isMainRunLoop());

_webExtensionAction->~WebExtensionAction();
}
WK_OBJECT_DEALLOC_IMPL_ON_MAIN_THREAD(_WKWebExtensionAction, WebExtensionAction, _webExtensionAction);

- (BOOL)isEqual:(id)object
{
Expand Down
7 changes: 1 addition & 6 deletions Source/WebKit/UIProcess/API/Cocoa/_WKWebExtensionCommand.mm
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,7 @@ @implementation _WKWebExtensionCommand

#if ENABLE(WK_WEB_EXTENSIONS)

- (void)dealloc
{
ASSERT(isMainRunLoop());

_webExtensionCommand->~WebExtensionCommand();
}
WK_OBJECT_DEALLOC_IMPL_ON_MAIN_THREAD(_WKWebExtensionCommand, WebExtensionCommand, _webExtensionCommand);

- (NSUInteger)hash
{
Expand Down
9 changes: 2 additions & 7 deletions Source/WebKit/UIProcess/API/Cocoa/_WKWebExtensionContext.mm
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ @implementation _WKWebExtensionContext

#if ENABLE(WK_WEB_EXTENSIONS)

WK_OBJECT_DEALLOC_IMPL_ON_MAIN_THREAD(_WKWebExtensionContext, WebExtensionContext, _webExtensionContext);

+ (instancetype)contextForExtension:(_WKWebExtension *)extension
{
NSParameterAssert([extension isKindOfClass:_WKWebExtension.class]);
Expand All @@ -89,13 +91,6 @@ - (instancetype)initForExtension:(_WKWebExtension *)extension
return self;
}

- (void)dealloc
{
ASSERT(isMainRunLoop());

_webExtensionContext->~WebExtensionContext();
}

- (_WKWebExtension *)webExtension
{
return wrapper(&_webExtensionContext->extension());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ @implementation _WKWebExtensionController

#if ENABLE(WK_WEB_EXTENSIONS)

WK_OBJECT_DEALLOC_IMPL_ON_MAIN_THREAD(_WKWebExtensionController, WebExtensionController, _webExtensionController);

- (instancetype)init
{
if (!(self = [super init]))
Expand All @@ -68,13 +70,6 @@ - (instancetype)initWithConfiguration:(_WKWebExtensionControllerConfiguration *)
return self;
}

- (void)dealloc
{
ASSERT(isMainRunLoop());

_webExtensionController->~WebExtensionController();
}

- (_WKWebExtensionControllerConfiguration *)configuration
{
return _webExtensionController->configuration().copy()->wrapper();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,14 @@
#import "WKWebsiteDataStoreInternal.h"
#import "WebExtensionControllerConfiguration.h"

#if ENABLE(WK_WEB_EXTENSIONS)
static NSString * const persistentCodingKey = @"persistent";
static NSString * const temporaryCodingKey = @"temporary";
static NSString * const temporaryDirectoryCodingKey = @"temporaryDirectory";
static NSString * const identifierCodingKey = @"identifier";
static NSString * const webViewConfigurationCodingKey = @"webViewConfiguration";
static NSString * const defaultWebsiteDataStoreCodingKey = @"defaultWebsiteDataStore";
#endif

@implementation _WKWebExtensionControllerConfiguration

Expand All @@ -51,6 +53,8 @@ + (BOOL)supportsSecureCoding

#if ENABLE(WK_WEB_EXTENSIONS)

WK_OBJECT_DEALLOC_IMPL_ON_MAIN_THREAD(_WKWebExtensionControllerConfiguration, WebExtensionControllerConfiguration, _webExtensionControllerConfiguration);

+ (instancetype)defaultConfiguration
{
return WebKit::WebExtensionControllerConfiguration::createDefault()->wrapper();
Expand Down Expand Up @@ -136,13 +140,6 @@ - (id)copyWithZone:(NSZone *)zone
return _webExtensionControllerConfiguration->copy()->wrapper();
}

- (void)dealloc
{
ASSERT(isMainRunLoop());

_webExtensionControllerConfiguration->~WebExtensionControllerConfiguration();
}

- (BOOL)isEqual:(id)object
{
if (self == object)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,7 @@ @implementation _WKWebExtensionDataRecord

#if ENABLE(WK_WEB_EXTENSIONS)

- (void)dealloc
{
_webExtensionDataRecord->~WebExtensionDataRecord();
}
WK_OBJECT_DEALLOC_IMPL_ON_MAIN_THREAD(_WKWebExtensionDataRecord, WebExtensionDataRecord, _webExtensionDataRecord);

- (BOOL)isEqual:(id)object
{
Expand Down
57 changes: 34 additions & 23 deletions Source/WebKit/UIProcess/API/Cocoa/_WKWebExtensionMatchPattern.mm
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@
#import "WebExtensionMatchPattern.h"
#import <wtf/URLParser.h>

#if ENABLE(WK_WEB_EXTENSIONS)
static NSString * const stringCodingKey = @"string";
#endif

NSErrorDomain const _WKWebExtensionMatchPatternErrorDomain = @"_WKWebExtensionMatchPatternErrorDomain";

Expand All @@ -44,28 +46,10 @@ + (BOOL)supportsSecureCoding
return YES;
}

- (instancetype)initWithCoder:(NSCoder *)coder
{
NSParameterAssert([coder isKindOfClass:NSCoder.class]);

return [self initWithString:[coder decodeObjectOfClass:[NSString class] forKey:stringCodingKey] error:nullptr];
}

- (void)encodeWithCoder:(NSCoder *)coder
{
NSParameterAssert([coder isKindOfClass:NSCoder.class]);

[coder encodeObject:self.string forKey:stringCodingKey];
}

- (instancetype)copyWithZone:(NSZone *)zone
{
// _WKWebExtensionMatchPattern is immutable.
return self;
}

#if ENABLE(WK_WEB_EXTENSIONS)

WK_OBJECT_DEALLOC_IMPL_ON_MAIN_THREAD(_WKWebExtensionMatchPattern, WebExtensionMatchPattern, _webExtensionMatchPattern);

+ (void)registerCustomURLScheme:(NSString *)urlScheme
{
NSParameterAssert([urlScheme isKindOfClass:NSString.class]);
Expand Down Expand Up @@ -136,11 +120,24 @@ - (instancetype)initWithScheme:(NSString *)scheme host:(NSString *)host path:(NS
return _webExtensionMatchPattern->isValid() ? self : nil;
}

- (void)dealloc
- (instancetype)initWithCoder:(NSCoder *)coder
{
ASSERT(isMainRunLoop());
NSParameterAssert([coder isKindOfClass:NSCoder.class]);

_webExtensionMatchPattern->~WebExtensionMatchPattern();
return [self initWithString:[coder decodeObjectOfClass:[NSString class] forKey:stringCodingKey] error:nullptr];
}

- (void)encodeWithCoder:(NSCoder *)coder
{
NSParameterAssert([coder isKindOfClass:NSCoder.class]);

[coder encodeObject:self.string forKey:stringCodingKey];
}

- (instancetype)copyWithZone:(NSZone *)zone
{
// _WKWebExtensionMatchPattern is immutable.
return self;
}

- (NSUInteger)hash
Expand Down Expand Up @@ -304,6 +301,20 @@ - (instancetype)initWithScheme:(NSString *)scheme host:(NSString *)host path:(NS
return nil;
}

- (void)encodeWithCoder:(NSCoder *)coder
{
}

- (instancetype)initWithCoder:(NSCoder *)coder
{
return [self initWithString:@"" error:nullptr];
}

- (id)copyWithZone:(NSZone *)zone
{
return nil;
}

- (NSString *)scheme
{
return nil;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,7 @@ @implementation _WKWebExtensionMessagePort

#if ENABLE(WK_WEB_EXTENSIONS)

- (void)dealloc
{
ASSERT(isMainRunLoop());

_webExtensionMessagePort->~WebExtensionMessagePort();
}
WK_OBJECT_DEALLOC_IMPL_ON_MAIN_THREAD(_WKWebExtensionMessagePort, WebExtensionMessagePort, _webExtensionMessagePort);

- (BOOL)isEqual:(id)object
{
Expand Down

0 comments on commit dc20f3f

Please sign in to comment.