Fix for intermittent Layout Test fail http/tests/loading/resourceLoadStatistics/telem...
authorwilander@apple.com <wilander@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 29 Jun 2017 22:22:35 +0000 (22:22 +0000)
committerwilander@apple.com <wilander@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 29 Jun 2017 22:22:35 +0000 (22:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=173940
<rdar://problem/33018125>

Reviewed by Brent Fulgham.

Source/WebCore:

No new tests. This change enables the exiting test to pass.

* loader/ResourceLoadStatisticsStore.cpp:
(WebCore::ResourceLoadStatisticsStore::sortedPrevalentResourceTelemetry):
    Added an assert.

Source/WebKit2:

This change allows the TestController to turn off
regular resource load statistics telemetry submission
and to manually control when telemetry is calculated
and submitted.

* UIProcess/API/C/WKResourceLoadStatisticsManager.cpp:
(WKResourceLoadStatisticsManagerSetShouldSubmitTelemetry):
    New test infrastructure.
* UIProcess/API/C/WKResourceLoadStatisticsManager.h:
* UIProcess/WebResourceLoadStatisticsManager.cpp:
(WebKit::WebResourceLoadStatisticsManager::setShouldSubmitTelemetry):
    New test infrastructure.
* UIProcess/WebResourceLoadStatisticsManager.h:
* UIProcess/WebResourceLoadStatisticsStore.cpp:
(WebKit::WebResourceLoadStatisticsStore::setShouldSubmitTelemetry):
    New test infrastructure.
(WebKit::WebResourceLoadStatisticsStore::registerSharedResourceLoadObserver):
    The test function now calls
    WebResourceLoadStatisticsStore::submitTelemetry() directly
    instead of firing the timer.
(WebKit::WebResourceLoadStatisticsStore::telemetryTimerFired):
    Now checks whether it should submit telemetry or not.
(WebKit::WebResourceLoadStatisticsStore::submitTelemetry):
    Split out so that the test code doesn't have to fire the timer.
* UIProcess/WebResourceLoadStatisticsStore.h:
* UIProcess/WebResourceLoadStatisticsTelemetry.cpp:
(WebKit::WebResourceLoadStatisticsTelemetry::calculateAndSubmit):
    Now doesn't submit if it's executed by test code.

Tools:

This change allows the TestController to turn off
regular resource load statistics telemetry submission
and to manually control when telemetry is calculated
and submitted.

* WebKitTestRunner/TestController.cpp:
(WTR::TestController::initialize):
    Turns off automatic resource load statistics telemetry submission.

LayoutTests:

* platform/wk2/TestExpectations:
    http/tests/loading/resourceLoadStatistics/telemetry-generation.html
    is now expected to pass.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@218965 268f45cc-cd09-0410-ab3c-d52691b4dbfc

14 files changed:
LayoutTests/ChangeLog
LayoutTests/platform/wk2/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/loader/ResourceLoadStatisticsStore.cpp
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/API/C/WKResourceLoadStatisticsManager.cpp
Source/WebKit2/UIProcess/API/C/WKResourceLoadStatisticsManager.h
Source/WebKit2/UIProcess/WebResourceLoadStatisticsManager.cpp
Source/WebKit2/UIProcess/WebResourceLoadStatisticsManager.h
Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp
Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.h
Source/WebKit2/UIProcess/WebResourceLoadStatisticsTelemetry.cpp
Tools/ChangeLog
Tools/WebKitTestRunner/TestController.cpp

index eb1e841..2fabe2b 100644 (file)
@@ -1,3 +1,15 @@
+2017-06-29  John Wilander  <wilander@apple.com>
+
+        Fix for intermittent Layout Test fail http/tests/loading/resourceLoadStatistics/telemetry-generation.html
+        https://bugs.webkit.org/show_bug.cgi?id=173940
+        <rdar://problem/33018125>
+
+        Reviewed by Brent Fulgham.
+
+        * platform/wk2/TestExpectations:
+            http/tests/loading/resourceLoadStatistics/telemetry-generation.html
+            is now expected to pass.
+
 2017-06-29  Sam Weinig  <sam@webkit.org>
 
         [WebIDL] Add a new extended attribute to model the forced return value optimization used on Node and Crypto
index c2aa681..e6cd8d5 100644 (file)
@@ -705,7 +705,7 @@ http/tests/loading/resourceLoadStatistics/classify-as-prevalent-based-on-subreso
 http/tests/loading/resourceLoadStatistics/classify-as-prevalent-based-on-subresource-unique-redirects-to.html [ Pass ]
 http/tests/loading/resourceLoadStatistics/clear-in-memory-and-persistent-store.html [ Pass ]
 webkit.org/b/172452 http/tests/loading/resourceLoadStatistics/grandfathering.html [ Pass Failure Timeout ]
-webkit.org/b/173499 http/tests/loading/resourceLoadStatistics/telemetry-generation.html [ Pass Failure ]
+webkit.org/b/173499 http/tests/loading/resourceLoadStatistics/telemetry-generation.html [ Pass ]
 
 ### END OF (5) Progressions, expected successes that are expected failures in WebKit1.
 ########################################
index 263bf12..60f4b54 100644 (file)
@@ -1,3 +1,17 @@
+2017-06-29  John Wilander  <wilander@apple.com>
+
+        Fix for intermittent Layout Test fail http/tests/loading/resourceLoadStatistics/telemetry-generation.html
+        https://bugs.webkit.org/show_bug.cgi?id=173940
+        <rdar://problem/33018125>
+
+        Reviewed by Brent Fulgham.
+
+        No new tests. This change enables the exiting test to pass.
+
+        * loader/ResourceLoadStatisticsStore.cpp:
+        (WebCore::ResourceLoadStatisticsStore::sortedPrevalentResourceTelemetry):
+            Added an assert.
+
 2017-06-29  Youenn Fablet  <youenn@apple.com>
 
         Support PeerConnectionStates::BundlePolicy::MaxBundle when setting rtc configuration
index 720a9d1..efa84bf 100644 (file)
@@ -190,6 +190,7 @@ Vector<ResourceLoadStatistics> ResourceLoadStatisticsStore::takeStatistics()
 
 void ResourceLoadStatisticsStore::mergeStatistics(const Vector<ResourceLoadStatistics>& statistics)
 {
+    ASSERT(!isMainThread());
     auto locker = holdLock(m_statisticsLock);
     for (auto& statistic : statistics) {
         auto result = m_resourceStatisticsMap.ensure(statistic.highLevelDomain, [&statistic] {
@@ -377,6 +378,7 @@ Vector<String> ResourceLoadStatisticsStore::topPrivatelyControlledDomainsToRemov
     
 Vector<PrevalentResourceTelemetry> ResourceLoadStatisticsStore::sortedPrevalentResourceTelemetry() const
 {
+    ASSERT(!isMainThread());
     auto locker = holdLock(m_statisticsLock);
     Vector<PrevalentResourceTelemetry> sorted;
     for (auto& statistic : m_resourceStatisticsMap.values()) {
index 728d263..105a96f 100644 (file)
@@ -1,3 +1,40 @@
+2017-06-29  John Wilander  <wilander@apple.com>
+
+        Fix for intermittent Layout Test fail http/tests/loading/resourceLoadStatistics/telemetry-generation.html
+        https://bugs.webkit.org/show_bug.cgi?id=173940
+        <rdar://problem/33018125>
+
+        Reviewed by Brent Fulgham.
+
+        This change allows the TestController to turn off
+        regular resource load statistics telemetry submission
+        and to manually control when telemetry is calculated
+        and submitted.
+
+        * UIProcess/API/C/WKResourceLoadStatisticsManager.cpp:
+        (WKResourceLoadStatisticsManagerSetShouldSubmitTelemetry):
+            New test infrastructure.
+        * UIProcess/API/C/WKResourceLoadStatisticsManager.h:
+        * UIProcess/WebResourceLoadStatisticsManager.cpp:
+        (WebKit::WebResourceLoadStatisticsManager::setShouldSubmitTelemetry):
+            New test infrastructure.
+        * UIProcess/WebResourceLoadStatisticsManager.h:
+        * UIProcess/WebResourceLoadStatisticsStore.cpp:
+        (WebKit::WebResourceLoadStatisticsStore::setShouldSubmitTelemetry):
+            New test infrastructure.
+        (WebKit::WebResourceLoadStatisticsStore::registerSharedResourceLoadObserver):
+            The test function now calls
+            WebResourceLoadStatisticsStore::submitTelemetry() directly
+            instead of firing the timer.
+        (WebKit::WebResourceLoadStatisticsStore::telemetryTimerFired):
+            Now checks whether it should submit telemetry or not.
+        (WebKit::WebResourceLoadStatisticsStore::submitTelemetry):
+            Split out so that the test code doesn't have to fire the timer.
+        * UIProcess/WebResourceLoadStatisticsStore.h:
+        * UIProcess/WebResourceLoadStatisticsTelemetry.cpp:
+        (WebKit::WebResourceLoadStatisticsTelemetry::calculateAndSubmit):
+            Now doesn't submit if it's executed by test code.
+
 2017-06-29  Chris Dumez  <cdumez@apple.com>
 
         Slight clean up of WebResourceLoadStatisticsStore
index 517d29d..18a8827 100644 (file)
@@ -137,6 +137,11 @@ void WKResourceLoadStatisticsManagerSetNotifyPagesWhenTelemetryWasCaptured(bool
     WebResourceLoadStatisticsManager::setNotifyPagesWhenTelemetryWasCaptured(value);
 }
 
+void WKResourceLoadStatisticsManagerSetShouldSubmitTelemetry(bool value)
+{
+    WebResourceLoadStatisticsManager::setShouldSubmitTelemetry(value);
+}
+
 void WKResourceLoadStatisticsManagerClearInMemoryAndPersistentStore()
 {
     WebResourceLoadStatisticsManager::clearInMemoryAndPersistentStore();
index 87bc845..d833b43 100644 (file)
@@ -53,6 +53,7 @@ extern "C" {
     WK_EXPORT void WKResourceLoadStatisticsManagerSetNotifyPagesWhenDataRecordsWereScanned(bool value);
     WK_EXPORT void WKResourceLoadStatisticsManagerSetShouldClassifyResourcesBeforeDataRecordsRemoval(bool value);
     WK_EXPORT void WKResourceLoadStatisticsManagerSetNotifyPagesWhenTelemetryWasCaptured(bool value);
+    WK_EXPORT void WKResourceLoadStatisticsManagerSetShouldSubmitTelemetry(bool value);
     WK_EXPORT void WKResourceLoadStatisticsManagerClearInMemoryAndPersistentStore();
     WK_EXPORT void WKResourceLoadStatisticsManagerClearInMemoryAndPersistentStoreModifiedSinceHours(unsigned);
     WK_EXPORT void WKResourceLoadStatisticsManagerResetToConsistentState();
index 150b387..975f311 100644 (file)
@@ -139,6 +139,11 @@ void WebResourceLoadStatisticsManager::setNotifyPagesWhenTelemetryWasCaptured(bo
     WebResourceLoadStatisticsTelemetry::setNotifyPagesWhenTelemetryWasCaptured(value);
 }
     
+void WebResourceLoadStatisticsManager::setShouldSubmitTelemetry(bool value)
+{
+    WebResourceLoadStatisticsStore::setShouldSubmitTelemetry(value);
+}
+    
 void WebResourceLoadStatisticsManager::setShouldClassifyResourcesBeforeDataRecordsRemoval(bool value)
 {
     WebResourceLoadStatisticsStore::setShouldClassifyResourcesBeforeDataRecordsRemoval(value);
index 5041ec8..39910fc 100644 (file)
@@ -59,6 +59,7 @@ public:
     static void fireTelemetryHandler();
     static void setNotifyPagesWhenDataRecordsWereScanned(bool);
     static void setNotifyPagesWhenTelemetryWasCaptured(bool value);
+    static void setShouldSubmitTelemetry(bool value);
     static void setShouldClassifyResourcesBeforeDataRecordsRemoval(bool value);
     static void clearInMemoryAndPersistentStore();
     static void clearInMemoryAndPersistentStoreModifiedSinceHours(unsigned);
index d21870d..64e6070 100644 (file)
@@ -57,6 +57,7 @@ namespace WebKit {
 
 static bool notifyPagesWhenDataRecordsWereScanned = false;
 static bool shouldClassifyResourcesBeforeDataRecordsRemoval = true;
+static auto shouldSubmitTelemetry = true;
 
 static const OptionSet<WebsiteDataType>& dataTypesToRemove()
 {
@@ -112,6 +113,11 @@ void WebResourceLoadStatisticsStore::setShouldClassifyResourcesBeforeDataRecords
     shouldClassifyResourcesBeforeDataRecordsRemoval = value;
 }
 
+void WebResourceLoadStatisticsStore::setShouldSubmitTelemetry(bool value)
+{
+    shouldSubmitTelemetry = value;
+}
+    
 void WebResourceLoadStatisticsStore::classifyResource(ResourceLoadStatistics& resourceStatistic)
 {
     if (!resourceStatistic.isPrevalentResource && m_resourceLoadStatisticsClassifier.hasPrevalentResourceCharacteristics(resourceStatistic))
@@ -225,8 +231,7 @@ void WebResourceLoadStatisticsStore::registerSharedResourceLoadObserver()
         });
     });
     m_resourceLoadStatisticsStore->setFireTelemetryCallback([this, protectedThis = makeRef(*this)] {
-        // This cancels the one shot timer and is only intended for testing purposes.
-        m_telemetryOneShotTimer.startOneShot(100_ms);
+        submitTelemetry();
     });
 #if PLATFORM(COCOA)
     WebResourceLoadStatisticsManager::registerUserDefaultsIfNeeded();
@@ -488,6 +493,14 @@ void WebResourceLoadStatisticsStore::telemetryTimerFired()
 {
     ASSERT(RunLoop::isMain());
     
+    if (!shouldSubmitTelemetry)
+        return;
+    
+    submitTelemetry();
+}
+
+void WebResourceLoadStatisticsStore::submitTelemetry()
+{
     m_statisticsQueue->dispatch([this, protectedThis = makeRef(*this)] {
         auto locker = holdLock(coreStore().statisticsLock());
         WebResourceLoadStatisticsTelemetry::calculateAndSubmit(coreStore());
index 02bf422..8a20ef3 100644 (file)
@@ -60,6 +60,7 @@ public:
     static void setNotifyPagesWhenDataRecordsWereScanned(bool);
     static void setShouldClassifyResourcesBeforeDataRecordsRemoval(bool);
     static void setMinimumTimeBetweeenDataRecordsRemoval(Seconds);
+    static void setShouldSubmitTelemetry(bool);
     virtual ~WebResourceLoadStatisticsStore();
     
     void setResourceLoadStatisticsEnabled(bool);
@@ -104,6 +105,7 @@ private:
     void syncWithExistingStatisticsStorageIfNeeded();
     void refreshFromDisk();
     void telemetryTimerFired();
+    void submitTelemetry();
 
     Ref<WebCore::ResourceLoadStatisticsStore> m_resourceLoadStatisticsStore;
 #if HAVE(CORE_PREDICTION)
index 6eb9255..b60e1f5 100644 (file)
@@ -211,6 +211,12 @@ void WebResourceLoadStatisticsTelemetry::calculateAndSubmit(const ResourceLoadSt
         return;
     }
     
+    if (notifyPagesWhenTelemetryWasCaptured) {
+        notifyPages(sortedPrevalentResources, sortedPrevalentResourcesWithoutUserInteraction, prevalentResourcesDaysSinceUserInteraction.size());
+        // The notify pages function is for testing so we don't need to do an actual submission.
+        return;
+    }
+    
     webPageProxy->logDiagnosticMessageWithValue(DiagnosticLoggingKeys::resourceLoadStatisticsTelemetryKey(), ASCIILiteral("totalNumberOfPrevalentResources"), sortedPrevalentResources.size(), 0, ShouldSample::No);
     webPageProxy->logDiagnosticMessageWithValue(DiagnosticLoggingKeys::resourceLoadStatisticsTelemetryKey(), ASCIILiteral("totalNumberOfPrevalentResourcesWithUserInteraction"), prevalentResourcesDaysSinceUserInteraction.size(), 0, ShouldSample::No);
     
@@ -220,9 +226,6 @@ void WebResourceLoadStatisticsTelemetry::calculateAndSubmit(const ResourceLoadSt
         webPageProxy->logDiagnosticMessageWithValue(DiagnosticLoggingKeys::resourceLoadStatisticsTelemetryKey(), ASCIILiteral("medianPrevalentResourcesWithUserInteractionDaysSinceUserInteraction"), median(prevalentResourcesDaysSinceUserInteraction), 0, ShouldSample::No);
     
     submitTopLists(sortedPrevalentResources, sortedPrevalentResourcesWithoutUserInteraction, *webPageProxy);
-    
-    if (notifyPagesWhenTelemetryWasCaptured)
-        notifyPages(sortedPrevalentResources, sortedPrevalentResourcesWithoutUserInteraction, prevalentResourcesDaysSinceUserInteraction.size());
 }
     
 void WebResourceLoadStatisticsTelemetry::setNotifyPagesWhenTelemetryWasCaptured(bool always)
index 2e839f3..c3b8ddb 100644 (file)
@@ -1,3 +1,20 @@
+2017-06-29  John Wilander  <wilander@apple.com>
+
+        Fix for intermittent Layout Test fail http/tests/loading/resourceLoadStatistics/telemetry-generation.html
+        https://bugs.webkit.org/show_bug.cgi?id=173940
+        <rdar://problem/33018125>
+
+        Reviewed by Brent Fulgham.
+
+        This change allows the TestController to turn off
+        regular resource load statistics telemetry submission
+        and to manually control when telemetry is calculated
+        and submitted.
+
+        * WebKitTestRunner/TestController.cpp:
+        (WTR::TestController::initialize):
+            Turns off automatic resource load statistics telemetry submission.
+
 2017-06-29  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r218512.
index 93c3deb..97839df 100644 (file)
@@ -388,6 +388,9 @@ void TestController::initialize(int argc, const char* argv[])
 #if PLATFORM(MAC)
     WebCoreTestSupport::installMockGamepadProvider();
 #endif
+    
+    WKResourceLoadStatisticsManagerSetShouldSubmitTelemetry(false);
+
     WKRetainPtr<WKStringRef> pageGroupIdentifier(AdoptWK, WKStringCreateWithUTF8CString("WebKitTestRunnerPageGroup"));
     m_pageGroup.adopt(WKPageGroupCreateWithIdentifier(pageGroupIdentifier.get()));
 }