- Revision
- 248935
- Author
- bshaf...@apple.com
- Date
- 2019-08-20 22:59:57 -0700 (Tue, 20 Aug 2019)
Log Message
Cherry-pick r248902. rdar://problem/54543355
Unsafe usage of CookieStorageObserver from a background thread
https://bugs.webkit.org/show_bug.cgi?id=200920
Reviewed by Alex Christensen.
Source/WebCore:
Unsafe usage of CookieStorageObserver from a background thread. CookieStorageObserver gets
constructed / destructed on the main thread. However, CookieStorageObserver::cookiesDidChange()
gets called on a background thread and tries to ref |this|. Even though CookieStorageObserver
is ThreadSafeRefCounted, this is still unsafe because the CookieStorageObserver destructor may
already be running on the main thread when CookieStorageObserver::cookiesDidChange() gets called
on the background thread.
* platform/network/NetworkStorageSession.h:
* platform/network/cocoa/CookieStorageObserver.h:
* platform/network/cocoa/CookieStorageObserver.mm:
(WebCore::CookieStorageObserver::CookieStorageObserver):
(WebCore::CookieStorageObserver::cookiesDidChange):
(WebCore::CookieStorageObserver::create): Deleted.
* platform/network/cocoa/NetworkStorageSessionCocoa.mm:
(WebCore::NetworkStorageSession::cookieStorageObserver const):
Source/WebKit:
* UIProcess/API/APIHTTPCookieStore.h:
* UIProcess/API/Cocoa/APIHTTPCookieStoreCocoa.mm:
(API::HTTPCookieStore::startObservingChangesToDefaultUIProcessCookieStore):
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@248902 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Diff
Modified: branches/safari-608-branch/Source/WebCore/ChangeLog (248934 => 248935)
--- branches/safari-608-branch/Source/WebCore/ChangeLog 2019-08-21 05:59:53 UTC (rev 248934)
+++ branches/safari-608-branch/Source/WebCore/ChangeLog 2019-08-21 05:59:57 UTC (rev 248935)
@@ -1,3 +1,62 @@
+2019-08-20 Babak Shafiei <bshaf...@apple.com>
+
+ Cherry-pick r248902. rdar://problem/54543355
+
+ Unsafe usage of CookieStorageObserver from a background thread
+ https://bugs.webkit.org/show_bug.cgi?id=200920
+
+ Reviewed by Alex Christensen.
+
+ Source/WebCore:
+
+ Unsafe usage of CookieStorageObserver from a background thread. CookieStorageObserver gets
+ constructed / destructed on the main thread. However, CookieStorageObserver::cookiesDidChange()
+ gets called on a background thread and tries to ref |this|. Even though CookieStorageObserver
+ is ThreadSafeRefCounted, this is still unsafe because the CookieStorageObserver destructor may
+ already be running on the main thread when CookieStorageObserver::cookiesDidChange() gets called
+ on the background thread.
+
+ * platform/network/NetworkStorageSession.h:
+ * platform/network/cocoa/CookieStorageObserver.h:
+ * platform/network/cocoa/CookieStorageObserver.mm:
+ (WebCore::CookieStorageObserver::CookieStorageObserver):
+ (WebCore::CookieStorageObserver::cookiesDidChange):
+ (WebCore::CookieStorageObserver::create): Deleted.
+ * platform/network/cocoa/NetworkStorageSessionCocoa.mm:
+ (WebCore::NetworkStorageSession::cookieStorageObserver const):
+
+ Source/WebKit:
+
+ * UIProcess/API/APIHTTPCookieStore.h:
+ * UIProcess/API/Cocoa/APIHTTPCookieStoreCocoa.mm:
+ (API::HTTPCookieStore::startObservingChangesToDefaultUIProcessCookieStore):
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@248902 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-08-20 Chris Dumez <cdu...@apple.com>
+
+ Unsafe usage of CookieStorageObserver from a background thread
+ https://bugs.webkit.org/show_bug.cgi?id=200920
+
+ Reviewed by Alex Christensen.
+
+ Unsafe usage of CookieStorageObserver from a background thread. CookieStorageObserver gets
+ constructed / destructed on the main thread. However, CookieStorageObserver::cookiesDidChange()
+ gets called on a background thread and tries to ref |this|. Even though CookieStorageObserver
+ is ThreadSafeRefCounted, this is still unsafe because the CookieStorageObserver destructor may
+ already be running on the main thread when CookieStorageObserver::cookiesDidChange() gets called
+ on the background thread.
+
+ * platform/network/NetworkStorageSession.h:
+ * platform/network/cocoa/CookieStorageObserver.h:
+ * platform/network/cocoa/CookieStorageObserver.mm:
+ (WebCore::CookieStorageObserver::CookieStorageObserver):
+ (WebCore::CookieStorageObserver::cookiesDidChange):
+ (WebCore::CookieStorageObserver::create): Deleted.
+ * platform/network/cocoa/NetworkStorageSessionCocoa.mm:
+ (WebCore::NetworkStorageSession::cookieStorageObserver const):
+
2019-08-18 Babak Shafiei <bshaf...@apple.com>
Cherry-pick r248815. rdar://problem/54454993
Modified: branches/safari-608-branch/Source/WebCore/platform/network/NetworkStorageSession.h (248934 => 248935)
--- branches/safari-608-branch/Source/WebCore/platform/network/NetworkStorageSession.h 2019-08-21 05:59:53 UTC (rev 248934)
+++ branches/safari-608-branch/Source/WebCore/platform/network/NetworkStorageSession.h 2019-08-21 05:59:57 UTC (rev 248935)
@@ -199,7 +199,7 @@
CookieStorageObserver& cookieStorageObserver() const;
private:
- mutable RefPtr<CookieStorageObserver> m_cookieStorageObserver;
+ mutable std::unique_ptr<CookieStorageObserver> m_cookieStorageObserver;
#endif
static bool m_processMayUseCookieAPI;
};
Modified: branches/safari-608-branch/Source/WebCore/platform/network/cocoa/CookieStorageObserver.h (248934 => 248935)
--- branches/safari-608-branch/Source/WebCore/platform/network/cocoa/CookieStorageObserver.h 2019-08-21 05:59:53 UTC (rev 248934)
+++ branches/safari-608-branch/Source/WebCore/platform/network/cocoa/CookieStorageObserver.h 2019-08-21 05:59:57 UTC (rev 248935)
@@ -28,7 +28,7 @@
#include <pal/spi/cf/CFNetworkSPI.h>
#include <wtf/Function.h>
#include <wtf/RetainPtr.h>
-#include <wtf/ThreadSafeRefCounted.h>
+#include <wtf/WeakPtr.h>
OBJC_CLASS NSHTTPCookieStorage;
OBJC_CLASS WebCookieObserverAdapter;
@@ -35,9 +35,11 @@
namespace WebCore {
-class WEBCORE_EXPORT CookieStorageObserver : public ThreadSafeRefCounted<CookieStorageObserver> {
+class WEBCORE_EXPORT CookieStorageObserver : public CanMakeWeakPtr<CookieStorageObserver> {
+ WTF_MAKE_FAST_ALLOCATED;
+ WTF_MAKE_NONCOPYABLE(CookieStorageObserver);
public:
- static Ref<CookieStorageObserver> create(NSHTTPCookieStorage *);
+ explicit CookieStorageObserver(NSHTTPCookieStorage *);
~CookieStorageObserver();
void startObserving(Function<void()>&& callback);
@@ -46,8 +48,8 @@
void cookiesDidChange();
private:
- CookieStorageObserver(NSHTTPCookieStorage *);
+ WeakPtr<CookieStorageObserver> m_weakThis;
RetainPtr<NSHTTPCookieStorage> m_cookieStorage;
bool m_hasRegisteredInternalsForNotifications { false };
RetainPtr<WebCookieObserverAdapter> m_observerAdapter;
Modified: branches/safari-608-branch/Source/WebCore/platform/network/cocoa/CookieStorageObserver.mm (248934 => 248935)
--- branches/safari-608-branch/Source/WebCore/platform/network/cocoa/CookieStorageObserver.mm 2019-08-21 05:59:53 UTC (rev 248934)
+++ branches/safari-608-branch/Source/WebCore/platform/network/cocoa/CookieStorageObserver.mm 2019-08-21 05:59:57 UTC (rev 248935)
@@ -74,13 +74,9 @@
namespace WebCore {
-Ref<CookieStorageObserver> CookieStorageObserver::create(NSHTTPCookieStorage *cookieStorage)
-{
- return adoptRef(*new CookieStorageObserver(cookieStorage));
-}
-
CookieStorageObserver::CookieStorageObserver(NSHTTPCookieStorage *cookieStorage)
- : m_cookieStorage(cookieStorage)
+ : m_weakThis(makeWeakPtr(*this))
+ , m_cookieStorage(cookieStorage)
{
ASSERT(isMainThread());
ASSERT(m_cookieStorage);
@@ -134,9 +130,9 @@
void CookieStorageObserver::cookiesDidChange()
{
- callOnMainThread([protectedThis = makeRef(*this), this] {
- if (m_cookieChangeCallback)
- m_cookieChangeCallback();
+ callOnMainThread([weakThis = m_weakThis] {
+ if (weakThis && weakThis->m_cookieChangeCallback)
+ weakThis->m_cookieChangeCallback();
});
}
Modified: branches/safari-608-branch/Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm (248934 => 248935)
--- branches/safari-608-branch/Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm 2019-08-21 05:59:53 UTC (rev 248934)
+++ branches/safari-608-branch/Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm 2019-08-21 05:59:57 UTC (rev 248935)
@@ -121,7 +121,7 @@
CookieStorageObserver& NetworkStorageSession::cookieStorageObserver() const
{
if (!m_cookieStorageObserver)
- m_cookieStorageObserver = CookieStorageObserver::create(nsCookieStorage());
+ m_cookieStorageObserver = makeUnique<CookieStorageObserver>(nsCookieStorage());
return *m_cookieStorageObserver;
}
Modified: branches/safari-608-branch/Source/WebKit/ChangeLog (248934 => 248935)
--- branches/safari-608-branch/Source/WebKit/ChangeLog 2019-08-21 05:59:53 UTC (rev 248934)
+++ branches/safari-608-branch/Source/WebKit/ChangeLog 2019-08-21 05:59:57 UTC (rev 248935)
@@ -1,3 +1,50 @@
+2019-08-20 Babak Shafiei <bshaf...@apple.com>
+
+ Cherry-pick r248902. rdar://problem/54543355
+
+ Unsafe usage of CookieStorageObserver from a background thread
+ https://bugs.webkit.org/show_bug.cgi?id=200920
+
+ Reviewed by Alex Christensen.
+
+ Source/WebCore:
+
+ Unsafe usage of CookieStorageObserver from a background thread. CookieStorageObserver gets
+ constructed / destructed on the main thread. However, CookieStorageObserver::cookiesDidChange()
+ gets called on a background thread and tries to ref |this|. Even though CookieStorageObserver
+ is ThreadSafeRefCounted, this is still unsafe because the CookieStorageObserver destructor may
+ already be running on the main thread when CookieStorageObserver::cookiesDidChange() gets called
+ on the background thread.
+
+ * platform/network/NetworkStorageSession.h:
+ * platform/network/cocoa/CookieStorageObserver.h:
+ * platform/network/cocoa/CookieStorageObserver.mm:
+ (WebCore::CookieStorageObserver::CookieStorageObserver):
+ (WebCore::CookieStorageObserver::cookiesDidChange):
+ (WebCore::CookieStorageObserver::create): Deleted.
+ * platform/network/cocoa/NetworkStorageSessionCocoa.mm:
+ (WebCore::NetworkStorageSession::cookieStorageObserver const):
+
+ Source/WebKit:
+
+ * UIProcess/API/APIHTTPCookieStore.h:
+ * UIProcess/API/Cocoa/APIHTTPCookieStoreCocoa.mm:
+ (API::HTTPCookieStore::startObservingChangesToDefaultUIProcessCookieStore):
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@248902 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-08-20 Chris Dumez <cdu...@apple.com>
+
+ Unsafe usage of CookieStorageObserver from a background thread
+ https://bugs.webkit.org/show_bug.cgi?id=200920
+
+ Reviewed by Alex Christensen.
+
+ * UIProcess/API/APIHTTPCookieStore.h:
+ * UIProcess/API/Cocoa/APIHTTPCookieStoreCocoa.mm:
+ (API::HTTPCookieStore::startObservingChangesToDefaultUIProcessCookieStore):
+
2019-08-18 Babak Shafiei <bshaf...@apple.com>
Cherry-pick r248832. rdar://problem/54454547
Modified: branches/safari-608-branch/Source/WebKit/UIProcess/API/APIHTTPCookieStore.h (248934 => 248935)
--- branches/safari-608-branch/Source/WebKit/UIProcess/API/APIHTTPCookieStore.h 2019-08-21 05:59:53 UTC (rev 248934)
+++ branches/safari-608-branch/Source/WebKit/UIProcess/API/APIHTTPCookieStore.h 2019-08-21 05:59:57 UTC (rev 248935)
@@ -98,7 +98,7 @@
uint64_t m_processPoolCreationListenerIdentifier { 0 };
#if PLATFORM(COCOA)
- RefPtr<WebCore::CookieStorageObserver> m_defaultUIProcessObserver;
+ std::unique_ptr<WebCore::CookieStorageObserver> m_defaultUIProcessObserver;
#endif
};
Modified: branches/safari-608-branch/Source/WebKit/UIProcess/API/Cocoa/APIHTTPCookieStoreCocoa.mm (248934 => 248935)
--- branches/safari-608-branch/Source/WebKit/UIProcess/API/Cocoa/APIHTTPCookieStoreCocoa.mm 2019-08-21 05:59:53 UTC (rev 248934)
+++ branches/safari-608-branch/Source/WebKit/UIProcess/API/Cocoa/APIHTTPCookieStoreCocoa.mm 2019-08-21 05:59:57 UTC (rev 248935)
@@ -60,7 +60,7 @@
void HTTPCookieStore::startObservingChangesToDefaultUIProcessCookieStore(Function<void()>&& function)
{
stopObservingChangesToDefaultUIProcessCookieStore();
- m_defaultUIProcessObserver = WebCore::CookieStorageObserver::create([NSHTTPCookieStorage sharedHTTPCookieStorage]);
+ m_defaultUIProcessObserver = makeUnique<WebCore::CookieStorageObserver>([NSHTTPCookieStorage sharedHTTPCookieStorage]);
m_defaultUIProcessObserver->startObserving(WTFMove(function));
}
