Stop using CheckedPtr with ScriptExecutionContext

https://bugs.webkit.org/show_bug.cgi?id=266525 Reviewed by Darin Adler. Stop using CheckedPtr with ScriptExecutionContext. Use WeakPtr instead to generate more actionable crashes. This tested as performance neutral on Speedometer 2 & 3. * Source/WebCore/Headers.cmake: * Source/WebCore/Modules/encryptedmedia/CDM.cpp: * Source/WebCore/Modules/encryptedmedia/MediaKeySession.h: * Source/WebCore/Modules/encryptedmedia/legacy/WebKitMediaKeySession.h: * Source/WebCore/Modules/indexeddb/IDBActiveDOMObject.h: * Source/WebCore/Modules/notifications/Notification.h: * Source/WebCore/Modules/webcodecs/WebCodecsAudioData.cpp: * Source/WebCore/WebCore.xcodeproj/project.pbxproj: * Source/WebCore/animation/WebAnimation.h: * Source/WebCore/crypto/SubtleCrypto.cpp: * Source/WebCore/dom/AbortSignal.h: * Source/WebCore/dom/ContextDestructionObserver.h: (WebCore::ContextDestructionObserver::scriptExecutionContext const): Deleted. * Source/WebCore/dom/ContextDestructionObserverInlines.h: Added. (WebCore::ContextDestructionObserver::scriptExecutionContext const): * Source/WebCore/dom/Document.h: * Source/WebCore/dom/MessagePort.h: * Source/WebCore/dom/RejectedPromiseTracker.h: * Source/WebCore/dom/ScriptExecutionContext.h: * Source/WebCore/html/track/TextTrack.h: * Source/WebCore/page/LocalDOMWindow.h: * Source/WebCore/page/ScreenOrientation.h: * Source/WebCore/page/csp/ContentSecurityPolicy.h: Canonical link: https://commits.webkit.org/272186@main
WebKit · Dec 17, 2023 · d8c9557 · d8c9557
1 parent 678305d
commit d8c9557
Show file tree

Hide file tree

Showing 29 changed files with 72 additions and 11 deletions.
diff --git a/Source/WebCore/Headers.cmake b/Source/WebCore/Headers.cmake
@@ -915,6 +915,7 @@ set(WebCore_PRIVATE_FRAMEWORK_HEADERS
     dom/Comment.h
     dom/ContainerNode.h
     dom/ContextDestructionObserver.h
+    dom/ContextDestructionObserverInlines.h
     dom/CrossOriginMode.h
     dom/CustomElementReactionQueue.h
     dom/DOMException.h

diff --git a/Source/WebCore/Modules/encryptedmedia/CDM.cpp b/Source/WebCore/Modules/encryptedmedia/CDM.cpp
@@ -30,6 +30,7 @@
 
 #include "CDMFactory.h"
 #include "CDMPrivate.h"
+#include "ContextDestructionObserverInlines.h"
 #include "Document.h"
 #include "InitDataRegistry.h"
 #include "MediaKeysRequirement.h"

diff --git a/Source/WebCore/Modules/encryptedmedia/MediaKeySession.h b/Source/WebCore/Modules/encryptedmedia/MediaKeySession.h
@@ -32,6 +32,7 @@
 
 #include "ActiveDOMObject.h"
 #include "CDMInstanceSession.h"
+#include "ContextDestructionObserverInlines.h"
 #include "EventTarget.h"
 #include "IDLTypes.h"
 #include "MediaKeyMessageType.h"

diff --git a/Source/WebCore/Modules/encryptedmedia/legacy/WebKitMediaKeySession.h b/Source/WebCore/Modules/encryptedmedia/legacy/WebKitMediaKeySession.h
@@ -28,6 +28,7 @@
 #if ENABLE(LEGACY_ENCRYPTED_MEDIA)
 
 #include "ActiveDOMObject.h"
+#include "ContextDestructionObserverInlines.h"
 #include "EventTarget.h"
 #include "ExceptionOr.h"
 #include "LegacyCDMSession.h"

diff --git a/Source/WebCore/Modules/indexeddb/IDBActiveDOMObject.h b/Source/WebCore/Modules/indexeddb/IDBActiveDOMObject.h
@@ -26,6 +26,7 @@
 #pragma once
 
 #include "ActiveDOMObject.h"
+#include "ContextDestructionObserverInlines.h"
 #include "ScriptExecutionContext.h"
 #include <wtf/Threading.h>
 

diff --git a/Source/WebCore/Modules/notifications/Notification.h b/Source/WebCore/Modules/notifications/Notification.h
@@ -34,6 +34,7 @@
 #if ENABLE(NOTIFICATIONS)
 
 #include "ActiveDOMObject.h"
+#include "ContextDestructionObserverInlines.h"
 #include "EventTarget.h"
 #include "NotificationDirection.h"
 #include "NotificationPayload.h"

diff --git a/Source/WebCore/Modules/webcodecs/WebCodecsAudioData.cpp b/Source/WebCore/Modules/webcodecs/WebCodecsAudioData.cpp
@@ -29,6 +29,7 @@
 
 #if ENABLE(WEB_CODECS)
 
+#include "ContextDestructionObserverInlines.h"
 #include "ExceptionOr.h"
 #include "JSDOMPromiseDeferred.h"
 #include "PlatformRawAudioData.h"

diff --git a/Source/WebCore/WebCore.xcodeproj/project.pbxproj b/Source/WebCore/WebCore.xcodeproj/project.pbxproj
@@ -1724,6 +1724,7 @@
 		46DE9BB5269DF93E0024C5A6 /* BroadcastChannelRegistry.h in Headers */ = {isa = PBXBuildFile; fileRef = 46DE9BB4269DF9320024C5A6 /* BroadcastChannelRegistry.h */; settings = {ATTRIBUTES = (Private, ); }; };
 		46DFF4981DC2603100B80B48 /* ShadowRootMode.h in Headers */ = {isa = PBXBuildFile; fileRef = 46DFF4961DC2601300B80B48 /* ShadowRootMode.h */; settings = {ATTRIBUTES = (Private, ); }; };
 		46DFF49C1DC2620B00B80B48 /* JSShadowRootMode.h in Headers */ = {isa = PBXBuildFile; fileRef = 46DFF49A1DC261F900B80B48 /* JSShadowRootMode.h */; };
+		46E01FAB2B2BB75B00DC3A97 /* ContextDestructionObserverInlines.h in Headers */ = {isa = PBXBuildFile; fileRef = 46E01FAA2B2BB74F00DC3A97 /* ContextDestructionObserverInlines.h */; settings = {ATTRIBUTES = (Private, ); }; };
 		46E0C0DE23C006E9005E47AE /* DragEvent.h in Headers */ = {isa = PBXBuildFile; fileRef = 46E0C0DA23C006B3005E47AE /* DragEvent.h */; settings = {ATTRIBUTES = (Private, ); }; };
 		46E139F823D8B8EB0075848E /* HTTPCookieAcceptPolicyCocoa.h in Headers */ = {isa = PBXBuildFile; fileRef = 46E139F623D8B8E70075848E /* HTTPCookieAcceptPolicyCocoa.h */; settings = {ATTRIBUTES = (Private, ); }; };
 		46E6F1932AFAB7EC002807E2 /* SourceBrushLogicalGradient.h in Headers */ = {isa = PBXBuildFile; fileRef = 46E6F1922AFAB7EC002807E2 /* SourceBrushLogicalGradient.h */; settings = {ATTRIBUTES = (Private, ); }; };
@@ -10675,6 +10676,7 @@
 		46DFF4991DC261F900B80B48 /* JSShadowRootMode.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSShadowRootMode.cpp; sourceTree = "<group>"; };
 		46DFF49A1DC261F900B80B48 /* JSShadowRootMode.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSShadowRootMode.h; sourceTree = "<group>"; };
 		46E016AD1F72D61E00282B2C /* DOMHighResTimeStamp.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DOMHighResTimeStamp.h; sourceTree = "<group>"; };
+		46E01FAA2B2BB74F00DC3A97 /* ContextDestructionObserverInlines.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ContextDestructionObserverInlines.h; sourceTree = "<group>"; };
 		46E0C0DA23C006B3005E47AE /* DragEvent.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DragEvent.h; sourceTree = "<group>"; };
 		46E0C0DC23C006B4005E47AE /* DragEvent.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = DragEvent.cpp; sourceTree = "<group>"; };
 		46E0C0DD23C006B4005E47AE /* DragEvent.idl */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; path = DragEvent.idl; sourceTree = "<group>"; };
@@ -36129,6 +36131,7 @@
 				4405C3E92981859700F0A098 /* ContentVisibilityDocumentState.h */,
 				97627B8B14FB3CEE002CDCA1 /* ContextDestructionObserver.cpp */,
 				97627B8C14FB3CEE002CDCA1 /* ContextDestructionObserver.h */,
+				46E01FAA2B2BB74F00DC3A97 /* ContextDestructionObserverInlines.h */,
 				4688EE3A26DD260C002AF5C4 /* CrossOriginMode.h */,
 				7CF570C62492BD49008EB33C /* CurrentScriptIncrementer.h */,
 				9B15F0D328C46D17006B7F32 /* CustomElementDefaultARIA.cpp */,
@@ -37918,6 +37921,7 @@
 				CDAD1A5A27AFC27C00699285 /* ContentTypeUtilities.h in Headers */,
 				4480E06B2A67D86A0081EAE8 /* ContentVisibilityAutoStateChangeEvent.h in Headers */,
 				97627B8E14FB3CEE002CDCA1 /* ContextDestructionObserver.h in Headers */,
+				46E01FAB2B2BB75B00DC3A97 /* ContextDestructionObserverInlines.h in Headers */,
 				93B6A0E60B0BCA5C00F5027A /* ContextMenu.h in Headers */,
 				065AD4F50B0C2EDA005A2B1D /* ContextMenuClient.h in Headers */,
 				5106D7BE18BDB76F000AB166 /* ContextMenuContext.h in Headers */,

diff --git a/Source/WebCore/animation/WebAnimation.h b/Source/WebCore/animation/WebAnimation.h
@@ -29,6 +29,7 @@
 #include "AnimationFrameRate.h"
 #include "AnimationFrameRatePreset.h"
 #include "CSSNumericValue.h"
+#include "ContextDestructionObserverInlines.h"
 #include "EventTarget.h"
 #include "ExceptionOr.h"
 #include "IDLTypes.h"

diff --git a/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm b/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
@@ -6713,6 +6713,7 @@ sub GenerateCallbackImplementationContent
     my $className = "JS${name}";
 
     $includesRef->{"ScriptExecutionContext.h"} = 1;
+    $includesRef->{"ContextDestructionObserverInlines.h"} = 1;
 
     # Constructor
     push(@$contentRef, "${className}::${className}(JSObject* callback, JSDOMGlobalObject* globalObject)\n");

diff --git a/Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunction.cpp b/Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunction.cpp
@@ -21,6 +21,7 @@
 #include "config.h"
 #include "JSTestCallbackFunction.h"
 
+#include "ContextDestructionObserverInlines.h"
 #include "JSDOMConvertNumbers.h"
 #include "JSDOMConvertStrings.h"
 #include "JSDOMExceptionHandling.h"

diff --git a/Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunctionRethrow.cpp b/Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunctionRethrow.cpp
@@ -21,6 +21,7 @@
 #include "config.h"
 #include "JSTestCallbackFunctionRethrow.h"
 
+#include "ContextDestructionObserverInlines.h"
 #include "JSDOMConvertNumbers.h"
 #include "JSDOMConvertSequences.h"
 #include "JSDOMConvertStrings.h"

diff --git a/Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunctionWithThisObject.cpp b/Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunctionWithThisObject.cpp
@@ -21,6 +21,7 @@
 #include "config.h"
 #include "JSTestCallbackFunctionWithThisObject.h"
 
+#include "ContextDestructionObserverInlines.h"
 #include "JSDOMConvertBase.h"
 #include "JSDOMConvertInterface.h"
 #include "JSDOMConvertSequences.h"

diff --git a/Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunctionWithTypedefs.cpp b/Source/WebCore/bindings/scripts/test/JS/JSTestCallbackFunctionWithTypedefs.cpp
@@ -21,6 +21,7 @@
 #include "config.h"
 #include "JSTestCallbackFunctionWithTypedefs.h"
 
+#include "ContextDestructionObserverInlines.h"
 #include "JSDOMConvertBase.h"
 #include "JSDOMConvertNullable.h"
 #include "JSDOMConvertNumbers.h"

diff --git a/Source/WebCore/bindings/scripts/test/JS/JSTestCallbackInterface.cpp b/Source/WebCore/bindings/scripts/test/JS/JSTestCallbackInterface.cpp
@@ -24,6 +24,7 @@
 
 #include "JSTestCallbackInterface.h"
 
+#include "ContextDestructionObserverInlines.h"
 #include "JSDOMConstructorNotConstructable.h"
 #include "JSDOMConvertBase.h"
 #include "JSDOMConvertBoolean.h"

diff --git a/Source/WebCore/bindings/scripts/test/JS/JSTestVoidCallbackFunction.cpp b/Source/WebCore/bindings/scripts/test/JS/JSTestVoidCallbackFunction.cpp
@@ -24,6 +24,7 @@
 
 #include "JSTestVoidCallbackFunction.h"
 
+#include "ContextDestructionObserverInlines.h"
 #include "JSDOMConvertBase.h"
 #include "JSDOMConvertBoolean.h"
 #include "JSDOMConvertBufferSource.h"

diff --git a/Source/WebCore/crypto/SubtleCrypto.cpp b/Source/WebCore/crypto/SubtleCrypto.cpp
@@ -28,6 +28,7 @@
 
 #if ENABLE(WEB_CRYPTO)
 
+#include "ContextDestructionObserverInlines.h"
 #include "CryptoAlgorithm.h"
 #include "CryptoAlgorithmRegistry.h"
 #include "CryptoAlgorithmX25519Params.h"

diff --git a/Source/WebCore/dom/AbortSignal.h b/Source/WebCore/dom/AbortSignal.h
@@ -25,7 +25,7 @@
 
 #pragma once
 
-#include "ContextDestructionObserver.h"
+#include "ContextDestructionObserverInlines.h"
 #include "EventTarget.h"
 #include "JSValueInWrappedObject.h"
 #include <wtf/Function.h>

diff --git a/Source/WebCore/dom/ContextDestructionObserver.cpp b/Source/WebCore/dom/ContextDestructionObserver.cpp
@@ -48,7 +48,7 @@ void ContextDestructionObserver::observeContext(ScriptExecutionContext* scriptEx
         m_scriptExecutionContext->willDestroyDestructionObserver(*this);
     }
 
-    m_scriptExecutionContext = scriptExecutionContext;
+    m_scriptExecutionContext = WeakPtr { scriptExecutionContext, EnableWeakPtrThreadingAssertions::No };
 
     if (m_scriptExecutionContext) {
         ASSERT(m_scriptExecutionContext->isContextThread());

diff --git a/Source/WebCore/dom/ContextDestructionObserver.h b/Source/WebCore/dom/ContextDestructionObserver.h
@@ -26,8 +26,8 @@
 
 #pragma once
 
-#include <wtf/CheckedPtr.h>
 #include <wtf/Forward.h>
+#include <wtf/WeakPtr.h>
 
 namespace WebCore {
 
@@ -37,7 +37,7 @@ class ContextDestructionObserver {
 public:
     WEBCORE_EXPORT virtual void contextDestroyed();
 
-    ScriptExecutionContext* scriptExecutionContext() const { return m_scriptExecutionContext.get(); }
+    ScriptExecutionContext* scriptExecutionContext() const; // Defined in ContextDestructionObserverInlines.h.
     RefPtr<ScriptExecutionContext> protectedScriptExecutionContext() const;
 
 protected:
@@ -46,7 +46,7 @@ class ContextDestructionObserver {
     void observeContext(ScriptExecutionContext*);
 
 private:
-    CheckedPtr<ScriptExecutionContext> m_scriptExecutionContext;
+    WeakPtr<ScriptExecutionContext> m_scriptExecutionContext;
 };
 
 } // namespace WebCore
diff --git a/Source/WebCore/dom/ContextDestructionObserverInlines.h b/Source/WebCore/dom/ContextDestructionObserverInlines.h
@@ -0,0 +1,38 @@
+/*
+ * Copyright (C) 2023 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
+
+#include "ContextDestructionObserver.h"
+#include "ScriptExecutionContext.h"
+
+namespace WebCore {
+
+inline ScriptExecutionContext* ContextDestructionObserver::scriptExecutionContext() const
+{
+    return m_scriptExecutionContext.get();
+}
+
+} // namespace WebCore
diff --git a/Source/WebCore/dom/Document.h b/Source/WebCore/dom/Document.h
@@ -30,6 +30,7 @@
 #include "CanvasObserver.h"
 #include "Color.h"
 #include "ContainerNode.h"
+#include "ContextDestructionObserverInlines.h"
 #include "DocumentEventTiming.h"
 #include "FontSelectorClient.h"
 #include "FrameDestructionObserver.h"

diff --git a/Source/WebCore/dom/MessagePort.h b/Source/WebCore/dom/MessagePort.h
@@ -27,6 +27,7 @@
 #pragma once
 
 #include "ActiveDOMObject.h"
+#include "ContextDestructionObserverInlines.h"
 #include "EventTarget.h"
 #include "ExceptionOr.h"
 #include "MessagePortChannel.h"

diff --git a/Source/WebCore/dom/RejectedPromiseTracker.h b/Source/WebCore/dom/RejectedPromiseTracker.h
@@ -32,6 +32,7 @@
 #include <wtf/Forward.h>
 #include <wtf/Noncopyable.h>
 #include <wtf/Vector.h>
+#include <wtf/WeakRef.h>
 
 namespace JSC {
 class VM;
@@ -60,7 +61,7 @@ class RejectedPromiseTracker : public CanMakeCheckedPtr {
     void reportUnhandledRejections(Vector<UnhandledPromise>&&);
     void reportRejectionHandled(Ref<DOMPromise>&&);
 
-    CheckedRef<ScriptExecutionContext> m_context;
+    WeakRef<ScriptExecutionContext> m_context;
     Vector<UnhandledPromise> m_aboutToBeNotifiedRejectedPromises;
     JSC::WeakGCMap<JSC::JSPromise*, JSC::JSPromise> m_outstandingRejectedPromises;
 };

diff --git a/Source/WebCore/dom/ScriptExecutionContext.h b/Source/WebCore/dom/ScriptExecutionContext.h
@@ -94,7 +94,7 @@ namespace IDBClient {
 class IDBConnectionProxy;
 }
 
-class ScriptExecutionContext : public SecurityContext, public CanMakeCheckedPtr, public TimerAlignment {
+class ScriptExecutionContext : public SecurityContext, public TimerAlignment {
 public:
     explicit ScriptExecutionContext(ScriptExecutionContextIdentifier = { });
     virtual ~ScriptExecutionContext();

diff --git a/Source/WebCore/html/track/TextTrack.h b/Source/WebCore/html/track/TextTrack.h
@@ -28,7 +28,7 @@
 
 #if ENABLE(VIDEO)
 
-#include "ContextDestructionObserver.h"
+#include "ContextDestructionObserverInlines.h"
 #include "PlatformTimeRanges.h"
 #include "TextTrackCue.h"
 #include "TrackBase.h"

diff --git a/Source/WebCore/page/LocalDOMWindow.h b/Source/WebCore/page/LocalDOMWindow.h
@@ -27,7 +27,7 @@
 #pragma once
 
 #include "Base64Utilities.h"
-#include "ContextDestructionObserver.h"
+#include "ContextDestructionObserverInlines.h"
 #include "DOMWindow.h"
 #include "ExceptionOr.h"
 #include "ImageBitmap.h"

diff --git a/Source/WebCore/page/ScreenOrientation.h b/Source/WebCore/page/ScreenOrientation.h
@@ -26,8 +26,8 @@
 #pragma once
 
 #include "ActiveDOMObject.h"
+#include "ContextDestructionObserverInlines.h"
 #include "EventTarget.h"
-
 #include "ScreenOrientationLockType.h"
 #include "ScreenOrientationManager.h"
 #include "ScreenOrientationType.h"

diff --git a/Source/WebCore/page/csp/ContentSecurityPolicy.h b/Source/WebCore/page/csp/ContentSecurityPolicy.h
@@ -246,7 +246,7 @@ class ContentSecurityPolicy : public CanMakeThreadSafeCheckedPtr {
     void reportBlockedScriptExecutionToInspector(const String& directiveText) const;
 
     // We can never have both a script execution context and a ContentSecurityPolicyClient.
-    CheckedPtr<ScriptExecutionContext> m_scriptExecutionContext;
+    WeakPtr<ScriptExecutionContext> m_scriptExecutionContext;
     ContentSecurityPolicyClient* m_client { nullptr };
     mutable ReportingClient* m_reportingClient { nullptr };