Title: [218955] trunk/Source/WebCore
- Revision
- 218955
- Author
- [email protected]
- Date
- 2017-06-29 12:24:52 -0700 (Thu, 29 Jun 2017)
Log Message
Unreviewed, rolling out r218944.
Optimization is incorrect
Reverted changeset:
"Avoid copying ResourceLoadStatistics objects"
https://bugs.webkit.org/show_bug.cgi?id=173972
http://trac.webkit.org/changeset/218944
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (218954 => 218955)
--- trunk/Source/WebCore/ChangeLog 2017-06-29 19:19:17 UTC (rev 218954)
+++ trunk/Source/WebCore/ChangeLog 2017-06-29 19:24:52 UTC (rev 218955)
@@ -1,3 +1,15 @@
+2017-06-29 Chris Dumez <[email protected]>
+
+ Unreviewed, rolling out r218944.
+
+ Optimization is incorrect
+
+ Reverted changeset:
+
+ "Avoid copying ResourceLoadStatistics objects"
+ https://bugs.webkit.org/show_bug.cgi?id=173972
+ http://trac.webkit.org/changeset/218944
+
2017-06-29 Carlos Garcia Campos <[email protected]>
REGRESSION(r218896): ASSERT in WebPageProxy::dataCallback
Modified: trunk/Source/WebCore/loader/ResourceLoadObserver.cpp (218954 => 218955)
--- trunk/Source/WebCore/loader/ResourceLoadObserver.cpp 2017-06-29 19:19:17 UTC (rev 218954)
+++ trunk/Source/WebCore/loader/ResourceLoadObserver.cpp 2017-06-29 19:24:52 UTC (rev 218955)
@@ -149,58 +149,59 @@
bool shouldFireDataModificationHandler = false;
{
- auto locker = holdLock(m_store->statisticsLock());
- ResourceLoadStatistics targetStatistics(targetPrimaryDomain);
+ auto locker = holdLock(m_store->statisticsLock());
+ // We must make a copy here, because later calls to 'ensureResourceStatisticsForPrimaryDomain' will invalidate the returned reference:
+ auto targetStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(targetPrimaryDomain);
- // Always fire if we have previously removed data records for this domain
- shouldFireDataModificationHandler = targetStatistics.dataRecordsRemoved > 0;
+ // Always fire if we have previously removed data records for this domain
+ shouldFireDataModificationHandler = targetStatistics.dataRecordsRemoved > 0;
- if (isMainFrame)
- targetStatistics.topFrameHasBeenNavigatedToBefore = true;
- else {
- targetStatistics.subframeHasBeenLoadedBefore = true;
+ if (isMainFrame)
+ targetStatistics.topFrameHasBeenNavigatedToBefore = true;
+ else {
+ targetStatistics.subframeHasBeenLoadedBefore = true;
- auto subframeUnderTopFrameOriginsResult = targetStatistics.subframeUnderTopFrameOrigins.add(mainFramePrimaryDomain);
- if (subframeUnderTopFrameOriginsResult.isNewEntry)
- shouldFireDataModificationHandler = true;
+ auto subframeUnderTopFrameOriginsResult = targetStatistics.subframeUnderTopFrameOrigins.add(mainFramePrimaryDomain);
+ if (subframeUnderTopFrameOriginsResult.isNewEntry)
+ shouldFireDataModificationHandler = true;
+ }
+
+ if (isRedirect) {
+ auto& redirectingOriginResourceStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(sourcePrimaryDomain);
+
+ if (m_store->isPrevalentResource(targetPrimaryDomain))
+ redirectingOriginResourceStatistics.redirectedToOtherPrevalentResourceOrigins.add(targetPrimaryDomain);
+
+ if (isMainFrame) {
+ ++targetStatistics.topFrameHasBeenRedirectedTo;
+ ++redirectingOriginResourceStatistics.topFrameHasBeenRedirectedFrom;
+ } else {
+ ++targetStatistics.subframeHasBeenRedirectedTo;
+ ++redirectingOriginResourceStatistics.subframeHasBeenRedirectedFrom;
+ redirectingOriginResourceStatistics.subframeUniqueRedirectsTo.add(targetPrimaryDomain);
+
+ ++targetStatistics.subframeSubResourceCount;
}
+ } else {
+ if (sourcePrimaryDomain.isNull() || sourcePrimaryDomain.isEmpty() || sourcePrimaryDomain == "nullOrigin") {
+ if (isMainFrame)
+ ++targetStatistics.topFrameInitialLoadCount;
+ else
+ ++targetStatistics.subframeSubResourceCount;
+ } else {
+ auto& sourceOriginResourceStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(sourcePrimaryDomain);
- if (isRedirect) {
- auto& redirectingOriginResourceStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(sourcePrimaryDomain);
-
- if (m_store->isPrevalentResource(targetPrimaryDomain))
- redirectingOriginResourceStatistics.redirectedToOtherPrevalentResourceOrigins.add(targetPrimaryDomain);
-
if (isMainFrame) {
- ++targetStatistics.topFrameHasBeenRedirectedTo;
- ++redirectingOriginResourceStatistics.topFrameHasBeenRedirectedFrom;
+ ++sourceOriginResourceStatistics.topFrameHasBeenNavigatedFrom;
+ ++targetStatistics.topFrameHasBeenNavigatedTo;
} else {
- ++targetStatistics.subframeHasBeenRedirectedTo;
- ++redirectingOriginResourceStatistics.subframeHasBeenRedirectedFrom;
- redirectingOriginResourceStatistics.subframeUniqueRedirectsTo.add(targetPrimaryDomain);
-
- ++targetStatistics.subframeSubResourceCount;
+ ++sourceOriginResourceStatistics.subframeHasBeenNavigatedFrom;
+ ++targetStatistics.subframeHasBeenNavigatedTo;
}
- } else {
- if (sourcePrimaryDomain.isNull() || sourcePrimaryDomain.isEmpty() || sourcePrimaryDomain == "nullOrigin") {
- if (isMainFrame)
- ++targetStatistics.topFrameInitialLoadCount;
- else
- ++targetStatistics.subframeSubResourceCount;
- } else {
- auto& sourceOriginResourceStatistics = m_store->ensureResourceStatisticsForPrimaryDomain(sourcePrimaryDomain);
-
- if (isMainFrame) {
- ++sourceOriginResourceStatistics.topFrameHasBeenNavigatedFrom;
- ++targetStatistics.topFrameHasBeenNavigatedTo;
- } else {
- ++sourceOriginResourceStatistics.subframeHasBeenNavigatedFrom;
- ++targetStatistics.subframeHasBeenNavigatedTo;
- }
- }
}
-
- m_store->setResourceStatisticsForPrimaryDomain(targetPrimaryDomain, WTFMove(targetStatistics));
+ }
+
+ m_store->setResourceStatisticsForPrimaryDomain(targetPrimaryDomain, WTFMove(targetStatistics));
} // Release lock
if (shouldFireDataModificationHandler)
Modified: trunk/Source/WebCore/loader/ResourceLoadStatistics.h (218954 => 218955)
--- trunk/Source/WebCore/loader/ResourceLoadStatistics.h 2017-06-29 19:19:17 UTC (rev 218954)
+++ trunk/Source/WebCore/loader/ResourceLoadStatistics.h 2017-06-29 19:24:52 UTC (rev 218955)
@@ -36,7 +36,7 @@
class KeyedEncoder;
struct ResourceLoadStatistics {
- explicit ResourceLoadStatistics(const String& primaryDomain)
+ ResourceLoadStatistics(const String& primaryDomain)
: highLevelDomain(primaryDomain)
{
}
@@ -43,11 +43,6 @@
ResourceLoadStatistics() = default;
- ResourceLoadStatistics(const ResourceLoadStatistics&) = delete;
- ResourceLoadStatistics& operator=(const ResourceLoadStatistics&) = delete;
- ResourceLoadStatistics(ResourceLoadStatistics&&) = default;
- ResourceLoadStatistics& operator=(ResourceLoadStatistics&&) = default;
-
void encode(KeyedEncoder&) const;
bool decode(KeyedDecoder&, unsigned version);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
