Unreviewed, rolling out r220268.

This change caused assertion failures on macOS and iOS Debug
WK2.

Reverted changeset:

"Resource Load Statistics: Report user interaction
immediately, but only when needed"
https://bugs.webkit.org/show_bug.cgi?id=175090
http://trac.webkit.org/changeset/220268

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index da0fb59ccf2044b9799b10a13a57938a47463bad..9894b150b4cb7b364215d406a1e16030ed51dca6 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,17 @@
+2017-08-04  Ryan Haddad  <ryanhaddad@apple.com>
+
+        Unreviewed, rolling out r220268.
+
+        This change caused assertion failures on macOS and iOS Debug
+        WK2.
+
+        Reverted changeset:
+
+        "Resource Load Statistics: Report user interaction
+        immediately, but only when needed"
+        https://bugs.webkit.org/show_bug.cgi?id=175090
+        http://trac.webkit.org/changeset/220268
+
 2017-08-04  Chris Dumez  <cdumez@apple.com>
 
         Mark beacon-basic-string.html as slow.

diff --git a/LayoutTests/http/tests/loading/resourceLoadStatistics/user-interaction-in-cross-origin-sub-frame.html b/LayoutTests/http/tests/loading/resourceLoadStatistics/user-interaction-in-cross-origin-sub-frame.html
index c3ad86c9422df5cfeba2aacec513d903288952e9..abdd07eaf80b08d59b12043ed9476d092f2aeae0 100644 (file)
--- a/LayoutTests/http/tests/loading/resourceLoadStatistics/user-interaction-in-cross-origin-sub-frame.html
+++ b/LayoutTests/http/tests/loading/resourceLoadStatistics/user-interaction-in-cross-origin-sub-frame.html
@@ -25,8 +25,10 @@ function finishTest() {
 }
 
 onload = function() {
-    if (internals)
+    if (internals) {
         internals.setResourceLoadStatisticsEnabled(true);
+        internals.setResourceLoadStatisticsShouldThrottleObserverNotifications(false);
+    }
 
     if (testRunner) {
         testRunner.setStatisticsNotifyPagesWhenDataRecordsWereScanned(true);

diff --git a/LayoutTests/http/tests/loading/resourceLoadStatistics/user-interaction-only-reported-once-within-short-period-of-time-expected.txt b/LayoutTests/http/tests/loading/resourceLoadStatistics/user-interaction-only-reported-once-within-short-period-of-time-expected.txt
deleted file mode 100644 (file)
index 1360d7c..0000000
--- a/LayoutTests/http/tests/loading/resourceLoadStatistics/user-interaction-only-reported-once-within-short-period-of-time-expected.txt
+++ /dev/null
@@ -1,27 +0,0 @@
-main frame - didStartProvisionalLoadForFrame
-main frame - didCommitLoadForFrame
-frame "testFrame" - didStartProvisionalLoadForFrame
-main frame - didFinishDocumentLoadForFrame
-frame "testFrame" - didCommitLoadForFrame
-frame "testFrame" - didFinishDocumentLoadForFrame
-frame "testFrame" - didHandleOnloadEventsForFrame
-main frame - didHandleOnloadEventsForFrame
-frame "testFrame" - didFinishLoadForFrame
-main frame - didFinishLoadForFrame
-Tests that user interaction is not reported repeatedly within a short period of time.
-
-On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
-
-
-PASS document.location.origin is topFrameOrigin
-PASS testRunner.isStatisticsHasHadUserInteraction(topFrameOrigin) is false
-PASS testRunner.isStatisticsHasHadUserInteraction(subFrameOrigin) is false
-PASS testRunner.isStatisticsHasHadUserInteraction(topFrameOrigin) is true
-PASS testRunner.isStatisticsHasHadUserInteraction(subFrameOrigin) is false
-PASS testRunner.isStatisticsHasHadUserInteraction(topFrameOrigin) is false
-PASS testRunner.isStatisticsHasHadUserInteraction(topFrameOrigin) is false
-PASS testRunner.isStatisticsHasHadUserInteraction(subFrameOrigin) is false
-PASS successfullyParsed is true
-
-TEST COMPLETE
-

diff --git a/LayoutTests/http/tests/loading/resourceLoadStatistics/user-interaction-only-reported-once-within-short-period-of-time.html b/LayoutTests/http/tests/loading/resourceLoadStatistics/user-interaction-only-reported-once-within-short-period-of-time.html
deleted file mode 100644 (file)
index a1bf5df..0000000
--- a/LayoutTests/http/tests/loading/resourceLoadStatistics/user-interaction-only-reported-once-within-short-period-of-time.html
+++ /dev/null
@@ -1,67 +0,0 @@
-<!DOCTYPE html>
-<html>
-<body>
-<script src="/js-test-resources/js-test.js"></script>
-<script src="/js-test-resources/ui-helper.js"></script>
-<script>
-    description("Tests that user interaction is not reported repeatedly within a short period of time.");
-    jsTestIsAsync = true;
-
-    const topFrameOrigin = "http://127.0.0.1:8000";
-    const subFrameOrigin = "http://localhost:8000";
-
-    function activateElement(elementId) {
-        var element = document.getElementById(elementId);
-        var centerX = element.offsetLeft + element.offsetWidth / 2;
-        var centerY = element.offsetTop + element.offsetHeight / 2;
-        UIHelper.activateAt(centerX, centerY);
-    }
-
-    function firstInteraction() {
-        if (testRunner) {
-            testRunner.setStatisticsNotifyPagesWhenDataRecordsWereScanned(true);
-            testRunner.installStatisticsDidScanDataRecordsCallback(secondInteraction);
-
-            shouldBeFalse("testRunner.isStatisticsHasHadUserInteraction(topFrameOrigin)");
-            shouldBeFalse("testRunner.isStatisticsHasHadUserInteraction(subFrameOrigin)");
-        }
-
-        activateElement("testFrame");
-    }
-
-    function secondInteraction() {
-        if (testRunner) {
-            testRunner.installStatisticsDidScanDataRecordsCallback(finishTest);
-
-            shouldBeTrue("testRunner.isStatisticsHasHadUserInteraction(topFrameOrigin)");
-            shouldBeFalse("testRunner.isStatisticsHasHadUserInteraction(subFrameOrigin)");
-
-            testRunner.setStatisticsHasHadUserInteraction(topFrameOrigin, false);
-            shouldBeFalse("testRunner.isStatisticsHasHadUserInteraction(topFrameOrigin)");
-        }
-
-        activateElement("testFrame");
-
-        if (testRunner)
-            testRunner.statisticsProcessStatisticsAndDataRecords();
-    }
-
-    function finishTest() {
-        shouldBeFalse("testRunner.isStatisticsHasHadUserInteraction(topFrameOrigin)");
-        shouldBeFalse("testRunner.isStatisticsHasHadUserInteraction(subFrameOrigin)");
-
-        finishJSTest();
-    }
-
-    onload = function() {
-        if (internals)
-            internals.setResourceLoadStatisticsEnabled(true);
-
-        shouldBe("document.location.origin", "topFrameOrigin");
-
-        firstInteraction();
-    };
-</script>
-<iframe id="testFrame" src="http://localhost:8000/loading/resourceLoadStatistics/resources/dummy.html"></iframe>
-</body>
-</html>
\ No newline at end of file

diff --git a/LayoutTests/platform/mac-wk2/TestExpectations b/LayoutTests/platform/mac-wk2/TestExpectations
index 78b90283cb7ab3a6f7479b34d11e5f70a8a75fb8..0191cff470713fbe9a28098bf71cfdac50f970a0 100644 (file)
--- a/LayoutTests/platform/mac-wk2/TestExpectations
+++ b/LayoutTests/platform/mac-wk2/TestExpectations
@@ -719,9 +719,8 @@ webkit.org/b/167757 workers/bomb.html [ Pass Timeout ]
 
 webkit.org/b/172397 [ Sierra Debug ] animations/needs-layout.html [ Pass ImageOnlyFailure ]
 
-# Move to general wk2 expectations once webkit.org/b/175170 is resolved.
+# Currently only tests with click, not tap.
 http/tests/loading/resourceLoadStatistics/user-interaction-in-cross-origin-sub-frame.html [ Pass ]
-http/tests/loading/resourceLoadStatistics/user-interaction-only-reported-once-within-short-period-of-time.html [ Pass ]
 
 webkit.org/b/173861 [ Release ] http/tests/webrtc/filtering-ice-candidate-same-origin-frame.html [ Pass Timeout ]
 webkit.org/b/173861 [ Release ] http/tests/webrtc/filtering-ice-candidate-cross-origin-frame.html [ Pass Timeout ]

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 959d2ef32e03a5b3b4a5ae0261a1e86c9aa4b53b..bf2e7d53673ad026acb1189fd5108f61a860d03c 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,17 @@
+2017-08-04  Ryan Haddad  <ryanhaddad@apple.com>
+
+        Unreviewed, rolling out r220268.
+
+        This change caused assertion failures on macOS and iOS Debug
+        WK2.
+
+        Reverted changeset:
+
+        "Resource Load Statistics: Report user interaction
+        immediately, but only when needed"
+        https://bugs.webkit.org/show_bug.cgi?id=175090
+        http://trac.webkit.org/changeset/220268
+
 2017-08-04  Youenn Fablet  <youenn@apple.com>
 
         Remove STREAMS_API compilation guard

diff --git a/Source/WebCore/loader/ResourceLoadObserver.cpp b/Source/WebCore/loader/ResourceLoadObserver.cpp
index 4d1479fb9a79bf99510c4a91a340afbb5f3e34f2..37b710e51efc3ba244e050edb8bcc4cb02b9abd7 100644 (file)
--- a/Source/WebCore/loader/ResourceLoadObserver.cpp
+++ b/Source/WebCore/loader/ResourceLoadObserver.cpp
@@ -99,6 +99,19 @@ static bool areDomainsAssociated(Page* page, const String& firstDomain, const St
     return firstMetaDomainIdentifier == metaDomainIdentifiers.get().get(secondDomain);
 }
 
+void ResourceLoadObserver::setShouldThrottleObserverNotifications(bool shouldThrottle)
+{
+    m_shouldThrottleNotifications = shouldThrottle;
+
+    if (!m_notificationTimer.isActive())
+        return;
+
+    // If we change the notification state, we need to restart any notifications
+    // so they will be on the right schedule.
+    m_notificationTimer.stop();
+    scheduleNotificationIfNeeded();
+}
+
 void ResourceLoadObserver::setNotificationCallback(WTF::Function<void (Vector<ResourceLoadStatistics>&&)>&& notificationCallback)
 {
     ASSERT(!m_notificationCallback);
@@ -106,7 +119,7 @@ void ResourceLoadObserver::setNotificationCallback(WTF::Function<void (Vector<Re
 }
 
 ResourceLoadObserver::ResourceLoadObserver()
-    : m_notificationTimer(*this, &ResourceLoadObserver::notifyObserver)
+    : m_notificationTimer(*this, &ResourceLoadObserver::notificationTimerFired)
 {
 }
 
@@ -267,20 +280,16 @@ void ResourceLoadObserver::logUserInteractionWithReducedTimeResolution(const Doc
     if (url.isBlankURL() || url.isEmpty())
         return;
 
-    auto domain = primaryDomain(url);
+    auto& statistics = ensureResourceStatisticsForPrimaryDomain(primaryDomain(url));
     auto newTime = reduceToHourlyTimeResolution(WallTime::now());
-    auto lastReportedUserInteraction = m_lastReportedUserInteractionMap.get(domain);
-    if (newTime == lastReportedUserInteraction)
+    if (newTime == statistics.mostRecentUserInteractionTime)
         return;
 
-    m_lastReportedUserInteractionMap.set(domain, newTime);
-
-    auto& statistics = ensureResourceStatisticsForPrimaryDomain(domain);
     statistics.hadUserInteraction = true;
     statistics.lastSeen = newTime;
     statistics.mostRecentUserInteractionTime = newTime;
 
-    notifyObserver();
+    scheduleNotificationIfNeeded();
 }
 
 ResourceLoadStatistics& ResourceLoadObserver::ensureResourceStatisticsForPrimaryDomain(const String& primaryDomain)
@@ -300,13 +309,12 @@ void ResourceLoadObserver::scheduleNotificationIfNeeded()
     }
 
     if (!m_notificationTimer.isActive())
-        m_notificationTimer.startOneShot(minimumNotificationInterval);
+        m_notificationTimer.startOneShot(m_shouldThrottleNotifications ? minimumNotificationInterval : 0_s);
 }
 
-void ResourceLoadObserver::notifyObserver()
+void ResourceLoadObserver::notificationTimerFired()
 {
     ASSERT(m_notificationCallback);
-    m_notificationTimer.stop();
     m_notificationCallback(takeStatistics());
 }
 
@@ -331,10 +339,4 @@ Vector<ResourceLoadStatistics> ResourceLoadObserver::takeStatistics()
     return statistics;
 }
 
-void ResourceLoadObserver::clearState()
-{
-    m_resourceStatisticsMap.clear();
-    m_lastReportedUserInteractionMap.clear();
-}
-
 } // namespace WebCore

diff --git a/Source/WebCore/loader/ResourceLoadObserver.h b/Source/WebCore/loader/ResourceLoadObserver.h
index dcc29463428dd1308ca3aa1fb113b402d4748d54..32684bd3307a1f5a82129eec95009ff0f0d0e242 100644 (file)
--- a/Source/WebCore/loader/ResourceLoadObserver.h
+++ b/Source/WebCore/loader/ResourceLoadObserver.h
@@ -33,7 +33,6 @@
 namespace WTF {
 class Lock;
 class WorkQueue;
-class WallTime;
 }
 
 namespace WebCore {
@@ -63,7 +62,6 @@ public:
 
     WEBCORE_EXPORT void setNotificationCallback(WTF::Function<void (Vector<ResourceLoadStatistics>&&)>&&);
 
-    WEBCORE_EXPORT void clearState();
 private:
     ResourceLoadObserver();
 
@@ -71,13 +69,13 @@ private:
     ResourceLoadStatistics& ensureResourceStatisticsForPrimaryDomain(const String&);
 
     void scheduleNotificationIfNeeded();
-    void notifyObserver();
+    void notificationTimerFired();
     Vector<ResourceLoadStatistics> takeStatistics();
 
     HashMap<String, ResourceLoadStatistics> m_resourceStatisticsMap;
-    HashMap<String, WTF::WallTime> m_lastReportedUserInteractionMap;
     WTF::Function<void (Vector<ResourceLoadStatistics>&&)> m_notificationCallback;
     Timer m_notificationTimer;
+    bool m_shouldThrottleNotifications { true };
 };
     
 } // namespace WebCore

diff --git a/Source/WebCore/testing/Internals.cpp b/Source/WebCore/testing/Internals.cpp
index dbabb25613a2b2af65bbef3dc8a668aa4e6d9e5e..91f4d2760343bf894744666fbcaed352c110a77e 100644 (file)
--- a/Source/WebCore/testing/Internals.cpp
+++ b/Source/WebCore/testing/Internals.cpp
@@ -468,6 +468,8 @@ void Internals::resetToConsistentState(Page& page)
 #if USE(LIBWEBRTC)
     WebCore::useRealRTCPeerConnectionFactory();
 #endif
+    
+    ResourceLoadObserver::shared().setShouldThrottleObserverNotifications(true);
 }
 
 Internals::Internals(Document& document)
@@ -3854,6 +3856,11 @@ void Internals::setResourceLoadStatisticsEnabled(bool enable)
     Settings::setResourceLoadStatisticsEnabled(enable);
 }
 
+void Internals::setResourceLoadStatisticsShouldThrottleObserverNotifications(bool enable)
+{
+    ResourceLoadObserver::shared().setShouldThrottleObserverNotifications(enable);
+}
+    
 String Internals::composedTreeAsText(Node& node)
 {
     if (!is<ContainerNode>(node))

diff --git a/Source/WebCore/testing/Internals.h b/Source/WebCore/testing/Internals.h
index d18079b322469b28f286bf196dd0536a47c09f9e..0c0f9b60e471164554551a96328f565c03d66c6e 100644 (file)
--- a/Source/WebCore/testing/Internals.h
+++ b/Source/WebCore/testing/Internals.h
@@ -539,6 +539,7 @@ public:
 
     String resourceLoadStatisticsForOrigin(const String& origin);
     void setResourceLoadStatisticsEnabled(bool);
+    void setResourceLoadStatisticsShouldThrottleObserverNotifications(bool);
 
     bool isReadableStreamDisturbed(JSC::ExecState&, JSC::JSValue);
     JSC::JSValue cloneArrayBuffer(JSC::ExecState&, JSC::JSValue, JSC::JSValue, JSC::JSValue);

diff --git a/Source/WebCore/testing/Internals.idl b/Source/WebCore/testing/Internals.idl
index 6d901fe0c4443d1a292b968e367a11d62de3eb6a..2f7a482ec1853e37b9e24c2791f39214b800f9af 100644 (file)
--- a/Source/WebCore/testing/Internals.idl
+++ b/Source/WebCore/testing/Internals.idl
@@ -498,6 +498,7 @@ enum EventThrottlingBehavior {
 
     DOMString resourceLoadStatisticsForOrigin(DOMString domain);
     void setResourceLoadStatisticsEnabled(boolean enable);
+    void setResourceLoadStatisticsShouldThrottleObserverNotifications(boolean enable);
 
     [MayThrowException] void setCanShowModalDialogOverride(boolean allow);
 

diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
index 43ec5ac1d8b5056cc04dc1d693a5b6000ec2f9e2..792f420327ce5ebf41e9dc7291651ee58abe69ea 100644 (file)
--- a/Source/WebKit/ChangeLog
+++ b/Source/WebKit/ChangeLog
@@ -1,3 +1,17 @@
+2017-08-04  Ryan Haddad  <ryanhaddad@apple.com>
+
+        Unreviewed, rolling out r220268.
+
+        This change caused assertion failures on macOS and iOS Debug
+        WK2.
+
+        Reverted changeset:
+
+        "Resource Load Statistics: Report user interaction
+        immediately, but only when needed"
+        https://bugs.webkit.org/show_bug.cgi?id=175090
+        http://trac.webkit.org/changeset/220268
+
 2017-08-04  Andreas Kling  <akling@apple.com>
 
         NetworkLoad should always invoke its redirect completion handler

diff --git a/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundle.cpp b/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundle.cpp
index 34bce00338345c5635e6259916ac80b4af56a902..d0826dd9c1018d027737cb40d4c0ea76975fd65d 100644 (file)
--- a/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundle.cpp
+++ b/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundle.cpp
@@ -39,7 +39,6 @@
 #include "WebPage.h"
 #include "WebPageGroupProxy.h"
 #include <WebCore/DatabaseTracker.h>
-#include <WebCore/ResourceLoadObserver.h>
 
 using namespace WebCore;
 using namespace WebKit;
@@ -277,8 +276,3 @@ void WKBundleSetTabKeyCyclesThroughElements(WKBundleRef bundleRef, WKBundlePageR
 {
     toImpl(bundleRef)->setTabKeyCyclesThroughElements(toImpl(pageRef), enabled);
 }
-
-void WKBundleClearResourceLoadStatistics(WKBundleRef)
-{
-    ResourceLoadObserver::shared().clearState();
-}

diff --git a/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePrivate.h b/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePrivate.h
index 9b58a9e82ea2852604455cd3e66c8eb79486348a..ce8324373b3551d82aac7ee950df4fefc16a2000 100644 (file)
--- a/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePrivate.h
+++ b/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePrivate.h
@@ -91,8 +91,6 @@ WK_EXPORT bool WKBundleIsProcessingUserGesture(WKBundleRef bundle);
 
 WK_EXPORT void WKBundleSetTabKeyCyclesThroughElements(WKBundleRef bundle, WKBundlePageRef page, bool enabled);
 
-WK_EXPORT void WKBundleClearResourceLoadStatistics(WKBundleRef);
-
 #ifdef __cplusplus
 }
 #endif

diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index 4b22c964b34da5ae4130d117f378c9bcfd631a0d..203fd1615a5dd893e30cf62560ac6556612b4b13 100644 (file)
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,17 @@
+2017-08-04  Ryan Haddad  <ryanhaddad@apple.com>
+
+        Unreviewed, rolling out r220268.
+
+        This change caused assertion failures on macOS and iOS Debug
+        WK2.
+
+        Reverted changeset:
+
+        "Resource Load Statistics: Report user interaction
+        immediately, but only when needed"
+        https://bugs.webkit.org/show_bug.cgi?id=175090
+        http://trac.webkit.org/changeset/220268
+
 2017-08-04  Youenn Fablet  <youenn@apple.com>
 
         Remove STREAMS_API compilation guard

diff --git a/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp b/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp
index 96b0113fcef5ec18bd5624ae81a70553b3c11de1..7a58e23e1d4c5a71be75a78ecac0b72cfbfd2f18 100644 (file)
--- a/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp
+++ b/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp
@@ -367,7 +367,6 @@ void InjectedBundle::beginTesting(WKDictionaryRef settings)
     WKBundleClearAllDatabases(m_bundle);
     WKBundlePageClearApplicationCache(page()->page());
     WKBundleResetOriginAccessWhitelists(m_bundle);
-    WKBundleClearResourceLoadStatistics(m_bundle);
 
     // [WK2] REGRESSION(r128623): It made layout tests extremely slow
     // https://bugs.webkit.org/show_bug.cgi?id=96862