Diff
Modified: trunk/Source/WebCore/ChangeLog (208873 => 208874)
--- trunk/Source/WebCore/ChangeLog 2016-11-18 02:41:35 UTC (rev 208873)
+++ trunk/Source/WebCore/ChangeLog 2016-11-18 03:32:53 UTC (rev 208874)
@@ -1,3 +1,46 @@
+2016-11-17 John Wilander <wilan...@apple.com>
+
+ Resource load statistics: Cover further data records, count removed data records, and only fire handler when needed
+ https://bugs.webkit.org/show_bug.cgi?id=164659
+
+ Reviewed by Andy Estes.
+
+ No new tests. This feature is behind a flag and off by default. Tests require real domain names.
+
+ * loader/ResourceLoadObserver.cpp:
+ (WebCore::ResourceLoadObserver::logFrameNavigation):
+ (WebCore::ResourceLoadObserver::logSubresourceLoading):
+ (WebCore::ResourceLoadObserver::logWebSocketLoading):
+ All three functions are now more conservative in calls to
+ m_store->fireDataModificationHandler(). They only fire when an important statistic has
+ changed or data records have previously been removed for the domain in question.
+ * loader/ResourceLoadStatistics.cpp:
+ (WebCore::ResourceLoadStatistics::encode):
+ Added the dataRecordsRemoved statistic.
+ (WebCore::ResourceLoadStatistics::decode):
+ Now takes a version parameter to control which keys to expect.
+ Added the dataRecordsRemoved statistic.
+ (WebCore::appendHashCountedSet):
+ Removed stray linefeed.
+ (WebCore::ResourceLoadStatistics::toString):
+ Added the dataRecordsRemoved statistic.
+ (WebCore::ResourceLoadStatistics::merge):
+ Added the dataRecordsRemoved statistic.
+ * loader/ResourceLoadStatistics.h:
+ Added the dataRecordsRemoved statistic.
+ * loader/ResourceLoadStatisticsStore.cpp:
+ (WebCore::ResourceLoadStatisticsStore::createEncoderFromData):
+ Now encodes a version number for the statistics model.
+ (WebCore::ResourceLoadStatisticsStore::readDataFromDecoder):
+ Now tries to decode a version number and passes it on to statistics decoding.
+ (WebCore::ResourceLoadStatisticsStore::processStatistics):
+ No longer gates processing on the number of data captured.
+ (WebCore::ResourceLoadStatisticsStore::updateStatisticsForRemovedDataRecords):
+ Update function for the new dataRecordsRemoved statistic.
+ (WebCore::ResourceLoadStatisticsStore::hasEnoughDataForStatisticsProcessing): Deleted.
+ No longer needed since we no longer gate processing on the number of data captured.
+ * loader/ResourceLoadStatisticsStore.h:
+
2016-11-17 Alex Christensen <achristen...@webkit.org>
Fix WinCairo build after r208740
Modified: trunk/Source/WebCore/loader/ResourceLoadObserver.cpp (208873 => 208874)
--- trunk/Source/WebCore/loader/ResourceLoadObserver.cpp 2016-11-18 02:41:35 UTC (rev 208873)
+++ trunk/Source/WebCore/loader/ResourceLoadObserver.cpp 2016-11-18 03:32:53 UTC (rev 208874)
@@ -103,10 +103,13 @@
if (targetPrimaryDomain == mainFramePrimaryDomain || targetPrimaryDomain == sourcePrimaryDomain)
return;
-
+
auto targetOrigin = SecurityOrigin::create(targetURL);
auto targetStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(targetPrimaryDomain);
-
+
+ // Always fire if we have previously removed data records for this domain
+ bool shouldFireDataModificationHandler = targetStatistics.dataRecordsRemoved > 0;
+
if (isMainFrame)
targetStatistics.topFrameHasBeenNavigatedToBefore = true;
else {
@@ -113,7 +116,9 @@
targetStatistics.subframeHasBeenLoadedBefore = true;
auto mainFrameOrigin = SecurityOrigin::create(mainFrameURL);
- targetStatistics.subframeUnderTopFrameOrigins.add(mainFramePrimaryDomain);
+ auto subframeUnderTopFrameOriginsResult = targetStatistics.subframeUnderTopFrameOrigins.add(mainFramePrimaryDomain);
+ if (subframeUnderTopFrameOriginsResult.isNewEntry)
+ shouldFireDataModificationHandler = true;
}
if (isRedirect) {
@@ -152,7 +157,8 @@
}
m_store->setResourceStatisticsForPrimaryDomain(targetPrimaryDomain, WTFMove(targetStatistics));
- m_store->fireDataModificationHandler();
+ if (shouldFireDataModificationHandler)
+ m_store->fireDataModificationHandler();
}
void ResourceLoadObserver::logSubresourceLoading(const Frame* frame, const ResourceRequest& newRequest, const ResourceResponse& redirectResponse)
@@ -185,8 +191,13 @@
auto& targetStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(targetPrimaryDomain);
+ // Always fire if we have previously removed data records for this domain
+ bool shouldFireDataModificationHandler = targetStatistics.dataRecordsRemoved > 0;
+
auto mainFrameOrigin = SecurityOrigin::create(mainFrameURL);
- targetStatistics.subresourceUnderTopFrameOrigins.add(mainFramePrimaryDomain);
+ auto subresourceUnderTopFrameOriginsResult = targetStatistics.subresourceUnderTopFrameOrigins.add(mainFramePrimaryDomain);
+ if (subresourceUnderTopFrameOriginsResult.isNewEntry)
+ shouldFireDataModificationHandler = true;
if (isRedirect) {
auto& redirectingOriginStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(sourcePrimaryDomain);
@@ -200,7 +211,9 @@
++redirectingOriginStatistics.subresourceHasBeenRedirectedFrom;
++updatedTargetStatistics.subresourceHasBeenRedirectedTo;
- redirectingOriginStatistics.subresourceUniqueRedirectsTo.add(targetPrimaryDomain);
+ auto subresourceUniqueRedirectsToResult = redirectingOriginStatistics.subresourceUniqueRedirectsTo.add(targetPrimaryDomain);
+ if (subresourceUniqueRedirectsToResult.isNewEntry)
+ shouldFireDataModificationHandler = true;
++updatedTargetStatistics.subresourceHasBeenSubresourceCount;
@@ -214,8 +227,9 @@
targetStatistics.subresourceHasBeenSubresourceCountDividedByTotalNumberOfOriginsVisited = static_cast<double>(targetStatistics.subresourceHasBeenSubresourceCount) / totalVisited;
}
-
- m_store->fireDataModificationHandler();
+
+ if (shouldFireDataModificationHandler)
+ m_store->fireDataModificationHandler();
}
void ResourceLoadObserver::logWebSocketLoading(const Frame* frame, const URL& targetURL)
@@ -245,10 +259,15 @@
return;
auto& targetStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(targetPrimaryDomain);
+
+ // Always fire if we have previously removed data records for this domain
+ bool shouldFireDataModificationHandler = targetStatistics.dataRecordsRemoved > 0;
auto mainFrameOrigin = SecurityOrigin::create(mainFrameURL);
- targetStatistics.subresourceUnderTopFrameOrigins.add(mainFramePrimaryDomain);
-
+ auto subresourceUnderTopFrameOriginsResult = targetStatistics.subresourceUnderTopFrameOrigins.add(mainFramePrimaryDomain);
+ if (subresourceUnderTopFrameOriginsResult.isNewEntry)
+ shouldFireDataModificationHandler = true;
+
++targetStatistics.subresourceHasBeenSubresourceCount;
auto totalVisited = std::max(m_originsVisitedMap.size(), 1U);
@@ -255,7 +274,8 @@
targetStatistics.subresourceHasBeenSubresourceCountDividedByTotalNumberOfOriginsVisited = static_cast<double>(targetStatistics.subresourceHasBeenSubresourceCount) / totalVisited;
- m_store->fireDataModificationHandler();
+ if (shouldFireDataModificationHandler)
+ m_store->fireDataModificationHandler();
}
void ResourceLoadObserver::logUserInteraction(const Document& document)
Modified: trunk/Source/WebCore/loader/ResourceLoadStatistics.cpp (208873 => 208874)
--- trunk/Source/WebCore/loader/ResourceLoadStatistics.cpp 2016-11-18 02:41:35 UTC (rev 208873)
+++ trunk/Source/WebCore/loader/ResourceLoadStatistics.cpp 2016-11-18 03:32:53 UTC (rev 208874)
@@ -81,6 +81,7 @@
// Prevalent Resource
encodeHashCountedSet(encoder, "redirectedToOtherPrevalentResourceOrigins", redirectedToOtherPrevalentResourceOrigins);
encoder.encodeBool("isPrevalentResource", isPrevalentResource);
+ encoder.encodeUInt32("dataRecordsRemoved", dataRecordsRemoved);
}
static void decodeHashCountedSet(KeyedDecoder& decoder, const String& label, HashCountedSet<String>& hashCountedSet)
@@ -99,7 +100,7 @@
});
}
-bool ResourceLoadStatistics::decode(KeyedDecoder& decoder)
+bool ResourceLoadStatistics::decode(KeyedDecoder& decoder, unsigned version)
{
if (!decoder.decodeString("PrevalentResourceOrigin", highLevelDomain))
return false;
@@ -170,6 +171,12 @@
if (!decoder.decodeBool("isPrevalentResource", isPrevalentResource))
return false;
+
+ if (version < 2)
+ return true;
+
+ if (!decoder.decodeUInt32("dataRecordsRemoved", dataRecordsRemoved))
+ return false;
return true;
}
@@ -198,7 +205,6 @@
builder.appendNumber(entry.value);
builder.append('\n');
}
-
}
String ResourceLoadStatistics::toString() const
@@ -268,8 +274,12 @@
// Prevalent Resource
appendHashCountedSet(builder, "redirectedToOtherPrevalentResourceOrigins", redirectedToOtherPrevalentResourceOrigins);
appendBoolean(builder, "isPrevalentResource", isPrevalentResource);
+ builder.appendLiteral(" dataRecordsRemoved: ");
+ builder.appendNumber(dataRecordsRemoved);
builder.append('\n');
+ builder.append('\n');
+
return builder.toString();
}
@@ -314,6 +324,7 @@
// Prevalent resource stats
mergeHashCountedSet(redirectedToOtherPrevalentResourceOrigins, other.redirectedToOtherPrevalentResourceOrigins);
isPrevalentResource |= other.isPrevalentResource;
+ dataRecordsRemoved += other.dataRecordsRemoved;
}
}
Modified: trunk/Source/WebCore/loader/ResourceLoadStatistics.h (208873 => 208874)
--- trunk/Source/WebCore/loader/ResourceLoadStatistics.h 2016-11-18 02:41:35 UTC (rev 208873)
+++ trunk/Source/WebCore/loader/ResourceLoadStatistics.h 2016-11-18 03:32:53 UTC (rev 208874)
@@ -43,7 +43,7 @@
ResourceLoadStatistics() = default;
void encode(KeyedEncoder&) const;
- bool decode(KeyedDecoder&);
+ bool decode(KeyedDecoder&, unsigned version);
String toString() const;
@@ -83,6 +83,7 @@
// Prevalent resource stats
HashCountedSet<String> redirectedToOtherPrevalentResourceOrigins;
bool isPrevalentResource { false };
+ unsigned dataRecordsRemoved { 0 };
};
} // namespace WebCore
Modified: trunk/Source/WebCore/loader/ResourceLoadStatisticsStore.cpp (208873 => 208874)
--- trunk/Source/WebCore/loader/ResourceLoadStatisticsStore.cpp 2016-11-18 02:41:35 UTC (rev 208873)
+++ trunk/Source/WebCore/loader/ResourceLoadStatisticsStore.cpp 2016-11-18 03:32:53 UTC (rev 208874)
@@ -38,7 +38,7 @@
namespace WebCore {
-static const unsigned minimumOriginsLoadedForProcessing = 100;
+static const auto statisticsModelVersion = 2;
Ref<ResourceLoadStatisticsStore> ResourceLoadStatisticsStore::create()
{
@@ -73,7 +73,8 @@
std::unique_ptr<KeyedEncoder> ResourceLoadStatisticsStore::createEncoderFromData()
{
auto encoder = KeyedEncoder::encoder();
-
+
+ encoder->encodeUInt32("version", statisticsModelVersion);
encoder->encodeObjects("browsingStatistics", m_resourceStatisticsMap.begin(), m_resourceStatisticsMap.end(), [this](KeyedEncoder& encoderInner, const StatisticsValue& origin) {
origin.value.encode(encoderInner);
});
@@ -86,9 +87,12 @@
if (m_resourceStatisticsMap.size())
return;
+ unsigned version;
+ if (!decoder.decodeUInt32("version", version))
+ version = 1;
Vector<ResourceLoadStatistics> loadedStatistics;
- bool succeeded = decoder.decodeObjects("browsingStatistics", loadedStatistics, [this](KeyedDecoder& decoderInner, ResourceLoadStatistics& statistics) {
- return statistics.decode(decoderInner);
+ bool succeeded = decoder.decodeObjects("browsingStatistics", loadedStatistics, [this, version](KeyedDecoder& decoderInner, ResourceLoadStatistics& statistics) {
+ return statistics.decode(decoderInner, version);
});
if (!succeeded)
@@ -141,14 +145,8 @@
m_dataAddedHandler();
}
-bool ResourceLoadStatisticsStore::hasEnoughDataForStatisticsProcessing()
-{
- return m_resourceStatisticsMap.size() >= minimumOriginsLoadedForProcessing;
-}
-
void ResourceLoadStatisticsStore::processStatistics(std::function<void(ResourceLoadStatistics&)>&& processFunction)
{
- ASSERT(hasEnoughDataForStatisticsProcessing());
for (auto& resourceStatistic : m_resourceStatisticsMap.values())
processFunction(resourceStatistic);
}
@@ -162,4 +160,12 @@
}
return prevalentResources;
}
+
+void ResourceLoadStatisticsStore::updateStatisticsForRemovedDataRecords(const Vector<String>& prevalentResourceDomains)
+{
+ for (auto& prevalentResourceDomain : prevalentResourceDomains) {
+ ResourceLoadStatistics& statisic = ensureResourceStatisticsForPrimaryDomain(prevalentResourceDomain);
+ ++statisic.dataRecordsRemoved;
+ }
}
+}
Modified: trunk/Source/WebCore/loader/ResourceLoadStatisticsStore.h (208873 => 208874)
--- trunk/Source/WebCore/loader/ResourceLoadStatisticsStore.h 2016-11-18 02:41:35 UTC (rev 208873)
+++ trunk/Source/WebCore/loader/ResourceLoadStatisticsStore.h 2016-11-18 03:32:53 UTC (rev 208874)
@@ -60,10 +60,10 @@
void fireDataModificationHandler();
- WEBCORE_EXPORT bool hasEnoughDataForStatisticsProcessing();
WEBCORE_EXPORT void processStatistics(std::function<void(ResourceLoadStatistics&)>&&);
WEBCORE_EXPORT Vector<String> prevalentResourceDomainsWithoutUserInteraction();
+ WEBCORE_EXPORT void updateStatisticsForRemovedDataRecords(const Vector<String>& prevalentResourceDomains);
private:
ResourceLoadStatisticsStore() = default;
Modified: trunk/Source/WebKit2/ChangeLog (208873 => 208874)
--- trunk/Source/WebKit2/ChangeLog 2016-11-18 02:41:35 UTC (rev 208873)
+++ trunk/Source/WebKit2/ChangeLog 2016-11-18 03:32:53 UTC (rev 208874)
@@ -1,3 +1,22 @@
+2016-11-17 John Wilander <wilan...@apple.com>
+
+ Resource load statistics: Cover further data records, count removed data records, and only fire handler when needed
+ https://bugs.webkit.org/show_bug.cgi?id=164659
+
+ Reviewed by Andy Estes.
+
+ * UIProcess/WebResourceLoadStatisticsStore.cpp:
+ (WebKit::WebResourceLoadStatisticsStore::removeDataRecords):
+ Consistent naming with 'remove' rather than 'delete'.
+ Now removes localStorage, IndexDB, disk cache, and memory cache too.
+ Updates statistics with number of times it has removed data records.
+ (WebKit::WebResourceLoadStatisticsStore::resourceLoadStatisticsUpdated):
+ No longer checks whether it has enough data since the classification rules
+ are absolute, not relative.
+ (WebKit::WebResourceLoadStatisticsStore::clearDataRecords): Deleted.
+ * UIProcess/WebResourceLoadStatisticsStore.h:
+ Consistent naming with 'remove' rather than 'delete'.
+
2016-11-17 Brady Eidson <beid...@apple.com>
Add _WKIconLoadingDelegate SPI.
Modified: trunk/Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp (208873 => 208874)
--- trunk/Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp 2016-11-18 02:41:35 UTC (rev 208873)
+++ trunk/Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp 2016-11-18 03:32:53 UTC (rev 208874)
@@ -44,8 +44,9 @@
namespace WebKit {
-static const auto numberOfSecondsBetweenClearingDataRecords = 600;
+static const auto numberOfSecondsBetweenRemovingDataRecords = 60;
static const auto featureVectorLengthThreshold = 3;
+static OptionSet<WebKit::WebsiteDataType> dataTypesToRemove;
Ref<WebResourceLoadStatisticsStore> WebResourceLoadStatisticsStore::create(const String& resourceLoadStatisticsDirectory)
{
@@ -99,9 +100,9 @@
}
}
-void WebResourceLoadStatisticsStore::clearDataRecords()
+void WebResourceLoadStatisticsStore::removeDataRecords()
{
- if (m_dataStoreClearPending)
+ if (m_dataRecordsRemovalPending)
return;
Vector<String> prevalentResourceDomains = coreStore().prevalentResourceDomainsWithoutUserInteraction();
@@ -109,41 +110,53 @@
return;
double now = currentTime();
- if (!m_lastTimeDataRecordsWereCleared) {
- m_lastTimeDataRecordsWereCleared = now;
+ if (!m_lastTimeDataRecordsWereRemoved) {
+ m_lastTimeDataRecordsWereRemoved = now;
return;
}
- if (now < (m_lastTimeDataRecordsWereCleared + numberOfSecondsBetweenClearingDataRecords))
+ if (now < (m_lastTimeDataRecordsWereRemoved + numberOfSecondsBetweenRemovingDataRecords))
return;
- m_dataStoreClearPending = true;
- m_lastTimeDataRecordsWereCleared = now;
+ m_dataRecordsRemovalPending = true;
+ m_lastTimeDataRecordsWereRemoved = now;
+ if (dataTypesToRemove.isEmpty()) {
+ dataTypesToRemove |= WebsiteDataType::Cookies;
+ dataTypesToRemove |= WebsiteDataType::LocalStorage;
+ dataTypesToRemove |= WebsiteDataType::IndexedDBDatabases;
+ dataTypesToRemove |= WebsiteDataType::DiskCache;
+ dataTypesToRemove |= WebsiteDataType::MemoryCache;
+ }
+
// Switch to the main thread to get the default website data store
RunLoop::main().dispatch([prevalentResourceDomains = WTFMove(prevalentResourceDomains), this] () mutable {
auto& websiteDataStore = API::WebsiteDataStore::defaultDataStore()->websiteDataStore();
- websiteDataStore.fetchData(WebsiteDataType::Cookies, { }, [prevalentResourceDomains = WTFMove(prevalentResourceDomains), this](auto websiteDataRecords) {
+ websiteDataStore.fetchData(dataTypesToRemove, { }, [prevalentResourceDomains = WTFMove(prevalentResourceDomains), this](auto websiteDataRecords) {
Vector<WebsiteDataRecord> dataRecords;
+ Vector<String> prevalentResourceDomainsWithDataRecords;
for (auto& websiteDataRecord : websiteDataRecords) {
for (auto& prevalentResourceDomain : prevalentResourceDomains) {
if (websiteDataRecord.displayName.endsWithIgnoringASCIICase(prevalentResourceDomain)) {
auto suffixStart = websiteDataRecord.displayName.length() - prevalentResourceDomain.length();
- if (!suffixStart || websiteDataRecord.displayName[suffixStart - 1] == '.')
+ if (!suffixStart || websiteDataRecord.displayName[suffixStart - 1] == '.') {
dataRecords.append(websiteDataRecord);
+ prevalentResourceDomainsWithDataRecords.append(prevalentResourceDomain);
+ }
}
}
}
if (!dataRecords.size()) {
- m_dataStoreClearPending = false;
+ m_dataRecordsRemovalPending = false;
return;
}
auto& websiteDataStore = API::WebsiteDataStore::defaultDataStore()->websiteDataStore();
- websiteDataStore.removeData(WebsiteDataType::Cookies, { WTFMove(dataRecords) }, [this] {
- m_dataStoreClearPending = false;
+ websiteDataStore.removeData(dataTypesToRemove, dataRecords, [prevalentResourceDomainsWithDataRecords = WTFMove(prevalentResourceDomainsWithDataRecords), this] {
+ this->coreStore().updateStatisticsForRemovedDataRecords(prevalentResourceDomainsWithDataRecords);
+ m_dataRecordsRemovalPending = false;
});
});
});
@@ -153,13 +166,11 @@
{
coreStore().mergeStatistics(origins);
- if (coreStore().hasEnoughDataForStatisticsProcessing()) {
- coreStore().processStatistics([this] (ResourceLoadStatistics& resourceStatistic) {
- classifyResource(resourceStatistic);
- clearDataRecords();
- });
- }
-
+ coreStore().processStatistics([this] (ResourceLoadStatistics& resourceStatistic) {
+ classifyResource(resourceStatistic);
+ removeDataRecords();
+ });
+
auto encoder = coreStore().createEncoderFromData();
writeEncoderToDisk(*encoder.get(), "full_browsing_session");
Modified: trunk/Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.h (208873 => 208874)
--- trunk/Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.h 2016-11-18 02:41:35 UTC (rev 208873)
+++ trunk/Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.h 2016-11-18 03:32:53 UTC (rev 208874)
@@ -74,7 +74,7 @@
bool hasPrevalentResourceCharacteristics(const WebCore::ResourceLoadStatistics&);
void classifyResource(WebCore::ResourceLoadStatistics&);
- void clearDataRecords();
+ void removeDataRecords();
String persistentStoragePath(const String& label) const;
@@ -89,8 +89,8 @@
String m_storagePath;
bool m_resourceLoadStatisticsEnabled { false };
- double m_lastTimeDataRecordsWereCleared { 0 };
- bool m_dataStoreClearPending { false };
+ double m_lastTimeDataRecordsWereRemoved { 0 };
+ bool m_dataRecordsRemovalPending { false };
};
} // namespace WebKit
