WebKit Bugzilla
Attachment 343857 Details for
Bug 187143
: Make sure the WebResourceLoadStatisticsStore gets destroyed on the main thread
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-187143-20180628150904.patch (text/plain), 8.03 KB, created by
Chris Dumez
on 2018-06-28 15:08:39 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Chris Dumez
Created:
2018-06-28 15:08:39 PDT
Size:
8.03 KB
patch
obsolete
>Subversion Revision: 233320 >diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog >index c4e00376c2044e8239c3a347a7e62986bb682cef..b8a0e8ad08b8e9ee66956412fc30639e5607add5 100644 >--- a/Source/WebKit/ChangeLog >+++ b/Source/WebKit/ChangeLog >@@ -1,3 +1,27 @@ >+2018-06-28 Chris Dumez <cdumez@apple.com> >+ >+ Make sure the WebResourceLoadStatisticsStore gets destroyed on the main thread >+ https://bugs.webkit.org/show_bug.cgi?id=187143 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Have WebResourceLoadStatisticsStore subclass ThreadSafeRefCounted<WebResourceLoadStatisticsStore, WTF::DestructionThread::Main> >+ instead of IPC::Connection::WorkQueueMessageReceiver. This makes sure that the WebResourceLoadStatisticsStore >+ objects get destroyed on the main thread, even if the last ref was held by a background thread. >+ >+ Also, methods called by IPC are now called on the main thread instead of the background queue. I think it is clearer for all >+ of WebResourceLoadStatisticsStore usage to be on the main thread. Expensive work is still done on the background queue, inside >+ the persistent / memory store classes. >+ >+ * UIProcess/WebResourceLoadStatisticsStore.cpp: >+ (WebKit::WebResourceLoadStatisticsStore::~WebResourceLoadStatisticsStore): >+ (WebKit::WebResourceLoadStatisticsStore::flushAndDestroyPersistentStore): >+ (WebKit::WebResourceLoadStatisticsStore::resourceLoadStatisticsUpdated): >+ (WebKit::WebResourceLoadStatisticsStore::requestStorageAccessUnderOpener): >+ (WebKit::WebResourceLoadStatisticsStore::processWillOpenConnection): >+ (WebKit::WebResourceLoadStatisticsStore::processDidCloseConnection): >+ * UIProcess/WebResourceLoadStatisticsStore.h: >+ > 2018-06-28 Chris Dumez <cdumez@apple.com> > > Split memory store logic out of WebResourceLoadStatisticsStore to clarify threading model >diff --git a/Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp b/Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp >index f36450126adadf4dffe892f741a46c7a50e8346d..382d154454088c317e888f0cbb23af5fb843438c 100644 >--- a/Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp >+++ b/Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp >@@ -138,11 +138,15 @@ WebResourceLoadStatisticsStore::WebResourceLoadStatisticsStore(const String& res > > WebResourceLoadStatisticsStore::~WebResourceLoadStatisticsStore() > { >+ ASSERT(RunLoop::isMain()); >+ > flushAndDestroyPersistentStore(); > } > > void WebResourceLoadStatisticsStore::flushAndDestroyPersistentStore() > { >+ ASSERT(RunLoop::isMain()); >+ > if (!m_persistentStorage && !m_memoryStore) > return; > >@@ -177,22 +181,23 @@ void WebResourceLoadStatisticsStore::scheduleStatisticsAndDataRecordsProcessing( > }); > } > >-// On background queue due to IPC. > void WebResourceLoadStatisticsStore::resourceLoadStatisticsUpdated(Vector<WebCore::ResourceLoadStatistics>&& origins) > { >- ASSERT(!RunLoop::isMain()); >+ ASSERT(RunLoop::isMain()); > >- if (!m_memoryStore) >- return; >+ m_statisticsQueue->dispatch([this, protectedThis = makeRef(*this), origins = WTFMove(origins)]() mutable { >+ if (!m_memoryStore) >+ return; > >- m_memoryStore->mergeStatistics(WTFMove(origins)); >+ m_memoryStore->mergeStatistics(WTFMove(origins)); > >- // We can cancel any pending request to process statistics since we're doing it synchronously below. >- m_memoryStore->cancelPendingStatisticsProcessingRequest(); >+ // We can cancel any pending request to process statistics since we're doing it synchronously below. >+ m_memoryStore->cancelPendingStatisticsProcessingRequest(); > >- // Fire before processing statistics to propagate user interaction as fast as possible to the network process. >- m_memoryStore->updateCookiePartitioning([]() { }); >- m_memoryStore->processStatisticsAndDataRecords(); >+ // Fire before processing statistics to propagate user interaction as fast as possible to the network process. >+ m_memoryStore->updateCookiePartitioning([]() { }); >+ m_memoryStore->processStatisticsAndDataRecords(); >+ }); > } > > void WebResourceLoadStatisticsStore::hasStorageAccess(String&& subFrameHost, String&& topFrameHost, uint64_t frameID, uint64_t pageID, CompletionHandler<void (bool)>&& completionHandler) >@@ -250,12 +255,14 @@ void WebResourceLoadStatisticsStore::requestStorageAccess(String&& subFrameHost, > }); > } > >-// On background queue due to IPC. > void WebResourceLoadStatisticsStore::requestStorageAccessUnderOpener(String&& primaryDomainInNeedOfStorageAccess, uint64_t openerPageID, String&& openerPrimaryDomain, bool isTriggeredByUserGesture) > { >- ASSERT(!RunLoop::isMain()); >- if (m_memoryStore) >- m_memoryStore->requestStorageAccessUnderOpener(WTFMove(primaryDomainInNeedOfStorageAccess), openerPageID, WTFMove(openerPrimaryDomain), isTriggeredByUserGesture); >+ ASSERT(RunLoop::isMain()); >+ >+ m_statisticsQueue->dispatch([this, protectedThis = makeRef(*this), primaryDomainInNeedOfStorageAccess = WTFMove(primaryDomainInNeedOfStorageAccess), openerPageID, openerPrimaryDomain = WTFMove(openerPrimaryDomain), isTriggeredByUserGesture]() mutable { >+ if (m_memoryStore) >+ m_memoryStore->requestStorageAccessUnderOpener(WTFMove(primaryDomainInNeedOfStorageAccess), openerPageID, WTFMove(openerPrimaryDomain), isTriggeredByUserGesture); >+ }); > } > > void WebResourceLoadStatisticsStore::grantStorageAccess(String&& subFrameHost, String&& topFrameHost, uint64_t frameID, uint64_t pageID, bool userWasPromptedNow, CompletionHandler<void(bool)>&& completionHandler) >@@ -290,15 +297,16 @@ void WebResourceLoadStatisticsStore::removeAllStorageAccess() > > m_removeAllStorageAccessHandler(); > } >- >-void WebResourceLoadStatisticsStore::processWillOpenConnection(WebProcessProxy&, IPC::Connection& connection) >+ >+ >+void WebResourceLoadStatisticsStore::processWillOpenConnection(WebProcessProxy& process, IPC::Connection&) > { >- connection.addWorkQueueMessageReceiver(Messages::WebResourceLoadStatisticsStore::messageReceiverName(), m_statisticsQueue.get(), this); >+ process.addMessageReceiver(Messages::WebResourceLoadStatisticsStore::messageReceiverName(), *this); > } > >-void WebResourceLoadStatisticsStore::processDidCloseConnection(WebProcessProxy&, IPC::Connection& connection) >+void WebResourceLoadStatisticsStore::processDidCloseConnection(WebProcessProxy& process, IPC::Connection&) > { >- connection.removeWorkQueueMessageReceiver(Messages::WebResourceLoadStatisticsStore::messageReceiverName()); >+ process.removeMessageReceiver(Messages::WebResourceLoadStatisticsStore::messageReceiverName()); > } > > void WebResourceLoadStatisticsStore::applicationWillTerminate() >diff --git a/Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h b/Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h >index 4504e89071478d97e238878cd81f22e9030a8a21..d258f06e6506d68c2e382329e657b0d9ecf8608d 100644 >--- a/Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h >+++ b/Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.h >@@ -29,6 +29,7 @@ > #include "WebsiteDataType.h" > #include <wtf/CompletionHandler.h> > #include <wtf/RunLoop.h> >+#include <wtf/ThreadSafeRefCounted.h> > #include <wtf/Vector.h> > #include <wtf/WallTime.h> > #include <wtf/text/WTFString.h> >@@ -57,7 +58,7 @@ enum class StorageAccessStatus { > HasAccess > }; > >-class WebResourceLoadStatisticsStore final : public IPC::Connection::WorkQueueMessageReceiver { >+class WebResourceLoadStatisticsStore final : public ThreadSafeRefCounted<WebResourceLoadStatisticsStore, WTF::DestructionThread::Main>, private IPC::MessageReceiver { > public: > using UpdatePrevalentDomainsToPartitionOrBlockCookiesHandler = Function<void(const Vector<String>& domainsToPartition, const Vector<String>& domainsToBlock, const Vector<String>& domainsToNeitherPartitionNorBlock, ShouldClearFirst, CompletionHandler<void()>&&)>; > using HasStorageAccessForFrameHandler = Function<void(const String& resourceDomain, const String& firstPartyDomain, uint64_t frameID, uint64_t pageID, Function<void(bool hasAccess)>&& callback)>;
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 187143
:
343857
|
343868