Modified: trunk/Source/WebKit2/ChangeLog (218870 => 218871)
--- trunk/Source/WebKit2/ChangeLog 2017-06-28 07:20:23 UTC (rev 218870)
+++ trunk/Source/WebKit2/ChangeLog 2017-06-28 07:34:41 UTC (rev 218871)
@@ -1,3 +1,21 @@
+2017-06-28 Chris Dumez <cdu...@apple.com>
+
+ Avoid double thread dispatch in WebsiteDataStore::fetchDataForTopPrivatelyControlledDomains()
+ https://bugs.webkit.org/show_bug.cgi?id=173904
+
+ Reviewed by Brent Fulgham.
+
+ Avoid double thread dispatch in WebsiteDataStore::fetchDataForTopPrivatelyControlledDomains().
+ It calls fetchData() which dispatches to the main queue, then fetchDataForTopPrivatelyControlledDomains()
+ dispatches to the background queue. It should be possible to get fetchData() to dispatch directly
+ on the right queue.
+
+ * UIProcess/WebsiteData/WebsiteDataStore.cpp:
+ (WebKit::WebsiteDataStore::fetchData):
+ (WebKit::WebsiteDataStore::fetchDataAndApply):
+ (WebKit::WebsiteDataStore::fetchDataForTopPrivatelyControlledDomains):
+ * UIProcess/WebsiteData/WebsiteDataStore.h:
+
2017-06-27 Chris Dumez <cdu...@apple.com>
[ResourceLoadStatistics] Update minimumTimeBetweeenDataRecordsRemoval to 1 hour instead of 1 minute
Modified: trunk/Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.cpp (218870 => 218871)
--- trunk/Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.cpp 2017-06-28 07:20:23 UTC (rev 218870)
+++ trunk/Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.cpp 2017-06-28 07:34:41 UTC (rev 218871)
@@ -185,10 +185,16 @@
void WebsiteDataStore::fetchData(OptionSet<WebsiteDataType> dataTypes, OptionSet<WebsiteDataFetchOption> fetchOptions, Function<void(Vector<WebsiteDataRecord>)>&& completionHandler)
{
+ fetchDataAndApply(dataTypes, fetchOptions, nullptr, WTFMove(completionHandler));
+}
+
+void WebsiteDataStore::fetchDataAndApply(OptionSet<WebsiteDataType> dataTypes, OptionSet<WebsiteDataFetchOption> fetchOptions, RefPtr<WorkQueue>&& queue, Function<void(Vector<WebsiteDataRecord>)>&& apply)
+{
struct CallbackAggregator final : ThreadSafeRefCounted<CallbackAggregator> {
- explicit CallbackAggregator(OptionSet<WebsiteDataFetchOption> fetchOptions, Function<void(Vector<WebsiteDataRecord>)>&& completionHandler)
+ explicit CallbackAggregator(OptionSet<WebsiteDataFetchOption> fetchOptions, RefPtr<WorkQueue>&& queue, Function<void(Vector<WebsiteDataRecord>)>&& apply)
: fetchOptions(fetchOptions)
- , completionHandler(WTFMove(completionHandler))
+ , queue(WTFMove(queue))
+ , apply(WTFMove(apply))
{
}
@@ -267,27 +273,31 @@
if (pendingCallbacks)
return;
- RunLoop::main().dispatch([callbackAggregator = makeRef(*this)]() mutable {
+ Vector<WebsiteDataRecord> records;
+ records.reserveInitialCapacity(m_websiteDataRecords.size());
+ for (auto& record : m_websiteDataRecords.values())
+ records.uncheckedAppend(WTFMove(record));
- WTF::Vector<WebsiteDataRecord> records;
- records.reserveInitialCapacity(callbackAggregator->m_websiteDataRecords.size());
+ auto processRecords = [apply = WTFMove(apply), records = WTFMove(records)] () mutable {
+ apply(WTFMove(records));
+ };
- for (auto& record : callbackAggregator->m_websiteDataRecords.values())
- records.uncheckedAppend(WTFMove(record));
-
- callbackAggregator->completionHandler(WTFMove(records));
- });
+ if (queue)
+ queue->dispatch(WTFMove(processRecords));
+ else
+ RunLoop::main().dispatch(WTFMove(processRecords));
}
const OptionSet<WebsiteDataFetchOption> fetchOptions;
unsigned pendingCallbacks = 0;
- Function<void(Vector<WebsiteDataRecord>)> completionHandler;
+ RefPtr<WorkQueue> queue;
+ Function<void(Vector<WebsiteDataRecord>)> apply;
HashMap<String, WebsiteDataRecord> m_websiteDataRecords;
};
- RefPtr<CallbackAggregator> callbackAggregator = adoptRef(new CallbackAggregator(fetchOptions, WTFMove(completionHandler)));
+ RefPtr<CallbackAggregator> callbackAggregator = adoptRef(new CallbackAggregator(fetchOptions, WTFMove(queue), WTFMove(apply)));
#if ENABLE(VIDEO)
if (dataTypes.contains(WebsiteDataType::DiskCache)) {
@@ -510,22 +520,22 @@
void WebsiteDataStore::fetchDataForTopPrivatelyControlledDomains(OptionSet<WebsiteDataType> dataTypes, OptionSet<WebsiteDataFetchOption> fetchOptions, const Vector<String>& topPrivatelyControlledDomains, Function<void(Vector<WebsiteDataRecord>&&, HashSet<String>&&)>&& completionHandler)
{
- fetchData(dataTypes, fetchOptions, [queue = m_queue.copyRef(), topPrivatelyControlledDomains = CrossThreadCopier<Vector<String>>::copy(topPrivatelyControlledDomains), completionHandler = WTFMove(completionHandler)] (auto&& existingDataRecords) mutable {
- queue->dispatch([queue = WTFMove(queue), topPrivatelyControlledDomains = WTFMove(topPrivatelyControlledDomains), existingDataRecords = WTFMove(existingDataRecords), completionHandler = WTFMove(completionHandler)] () mutable {
- Vector<WebsiteDataRecord> matchingDataRecords;
- HashSet<String> domainsWithMatchingDataRecords;
- for (auto&& dataRecord : existingDataRecords) {
- for (auto& topPrivatelyControlledDomain : topPrivatelyControlledDomains) {
- if (dataRecord.matchesTopPrivatelyControlledDomain(topPrivatelyControlledDomain)) {
- matchingDataRecords.append(WTFMove(dataRecord));
- domainsWithMatchingDataRecords.add(topPrivatelyControlledDomain.isolatedCopy());
- break;
- }
+ fetchDataAndApply(dataTypes, fetchOptions, m_queue.copyRef(), [topPrivatelyControlledDomains = CrossThreadCopier<Vector<String>>::copy(topPrivatelyControlledDomains), completionHandler = WTFMove(completionHandler)] (auto&& existingDataRecords) mutable {
+ ASSERT(!RunLoop::isMain());
+
+ Vector<WebsiteDataRecord> matchingDataRecords;
+ HashSet<String> domainsWithMatchingDataRecords;
+ for (auto&& dataRecord : existingDataRecords) {
+ for (auto& topPrivatelyControlledDomain : topPrivatelyControlledDomains) {
+ if (dataRecord.matchesTopPrivatelyControlledDomain(topPrivatelyControlledDomain)) {
+ matchingDataRecords.append(WTFMove(dataRecord));
+ domainsWithMatchingDataRecords.add(topPrivatelyControlledDomain.isolatedCopy());
+ break;
}
}
- RunLoop::main().dispatch([completionHandler = WTFMove(completionHandler), matchingDataRecords = WTFMove(matchingDataRecords), domainsWithMatchingDataRecords = WTFMove(domainsWithMatchingDataRecords)] () mutable {
- completionHandler(WTFMove(matchingDataRecords), WTFMove(domainsWithMatchingDataRecords));
- });
+ }
+ RunLoop::main().dispatch([completionHandler = WTFMove(completionHandler), matchingDataRecords = WTFMove(matchingDataRecords), domainsWithMatchingDataRecords = WTFMove(domainsWithMatchingDataRecords)] () mutable {
+ completionHandler(WTFMove(matchingDataRecords), WTFMove(domainsWithMatchingDataRecords));
});
});
}
Modified: trunk/Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.h (218870 => 218871)
--- trunk/Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.h 2017-06-28 07:20:23 UTC (rev 218870)
+++ trunk/Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.h 2017-06-28 07:34:41 UTC (rev 218871)
@@ -128,6 +128,8 @@
explicit WebsiteDataStore(WebCore::SessionID);
explicit WebsiteDataStore(Configuration, WebCore::SessionID);
+ void fetchDataAndApply(OptionSet<WebsiteDataType>, OptionSet<WebsiteDataFetchOption>, RefPtr<WorkQueue>&&, Function<void(Vector<WebsiteDataRecord>)>&& apply);
+
// WebProcessLifetimeObserver.
void webPageWasAdded(WebPageProxy&) override;
void webPageWasRemoved(WebPageProxy&) override;
