Resource Load Statistics: Report user interaction immediately, but only when needed
authorwilander@apple.com <wilander@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Aug 2017 14:20:02 +0000 (14:20 +0000)
committerwilander@apple.com <wilander@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Aug 2017 14:20:02 +0000 (14:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=175090
<rdar://problem/33685546>

Reviewed by Chris Dumez.

Source/WebCore:

Test: http/tests/loading/resourceLoadStatistics/user-interaction-only-reported-once-within-short-period-of-time.html

* loader/ResourceLoadObserver.cpp:
(WebCore::ResourceLoadObserver::ResourceLoadObserver):
(WebCore::ResourceLoadObserver::logUserInteractionWithReducedTimeResolution):
    Now tells the UI process immediately but also records that it has
    done so to avoid doing it when not needed.
(WebCore::ResourceLoadObserver::scheduleNotificationIfNeeded):
    Conditional throttling gone, now always throttles.
(WebCore::ResourceLoadObserver::notifyObserver):
    Renamed from ResourceLoadObserver::notificationTimerFired().
(WebCore::ResourceLoadObserver::clearState):
    New function to allow the test runner to reset the web process'
    statistics state now that we keep track of whether or not we've
    reported user interaction to the UI process.
(WebCore::ResourceLoadObserver::setShouldThrottleObserverNotifications): Deleted.
(WebCore::ResourceLoadObserver::notificationTimerFired): Deleted.
* loader/ResourceLoadObserver.h:
(): Deleted.
* testing/Internals.cpp:
(WebCore::Internals::resetToConsistentState):
(WebCore::Internals::setResourceLoadStatisticsShouldThrottleObserverNotifications): Deleted.
    No longer needed since user interaction is always communicated
    immediately.
* testing/Internals.h:
* testing/Internals.idl:

Source/WebKit:

* WebProcess/InjectedBundle/API/c/WKBundle.cpp:
(WKBundleClearResourceLoadStatistics):
    Test infrastructure. Ends up calling
    WebCore::ResourceLoadObserver::clearState().
* WebProcess/InjectedBundle/API/c/WKBundlePrivate.h:

Tools:

* WebKitTestRunner/InjectedBundle/InjectedBundle.cpp:
(WTR::InjectedBundle::beginTesting):
    Now calls WebCore::ResourceLoadObserver::clearState().

LayoutTests:

* http/tests/loading/resourceLoadStatistics/user-interaction-in-cross-origin-sub-frame.html:
    Now no longer needs to disable throttling since reports of
    user interaction happen immediately (when needed).
* http/tests/loading/resourceLoadStatistics/user-interaction-only-reported-once-within-short-period-of-time-expected.txt: Added.
* http/tests/loading/resourceLoadStatistics/user-interaction-only-reported-once-within-short-period-of-time.html: Added.
* platform/mac-wk2/TestExpectations:
    user-interaction-only-reported-once-within-short-period-of-time.html marked as [ Pass ].

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

16 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/loading/resourceLoadStatistics/user-interaction-in-cross-origin-sub-frame.html
LayoutTests/http/tests/loading/resourceLoadStatistics/user-interaction-only-reported-once-within-short-period-of-time-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/loading/resourceLoadStatistics/user-interaction-only-reported-once-within-short-period-of-time.html [new file with mode: 0644]
LayoutTests/platform/mac-wk2/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/loader/ResourceLoadObserver.cpp
Source/WebCore/loader/ResourceLoadObserver.h
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundle.cpp
Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePrivate.h
Tools/ChangeLog
Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp

index fa8860c8c0a9925b90f243e84cce15e38bc18e4e..e20138746bc98317c2deaf0e0b53f4112504195e 100644 (file)
@@ -1,3 +1,19 @@
+2017-08-04  John Wilander  <wilander@apple.com>
+
+        Resource Load Statistics: Report user interaction immediately, but only when needed
+        https://bugs.webkit.org/show_bug.cgi?id=175090
+        <rdar://problem/33685546>
+
+        Reviewed by Chris Dumez.
+
+        * http/tests/loading/resourceLoadStatistics/user-interaction-in-cross-origin-sub-frame.html:
+            Now no longer needs to disable throttling since reports of
+            user interaction happen immediately (when needed).
+        * http/tests/loading/resourceLoadStatistics/user-interaction-only-reported-once-within-short-period-of-time-expected.txt: Added.
+        * http/tests/loading/resourceLoadStatistics/user-interaction-only-reported-once-within-short-period-of-time.html: Added.
+        * platform/mac-wk2/TestExpectations:
+            user-interaction-only-reported-once-within-short-period-of-time.html marked as [ Pass ].
+
 2017-08-04  Ms2ger  <Ms2ger@igalia.com>
 
         [GTK] Test gardening around MOUSE_CURSOR_SCALE.
index abdd07eaf80b08d59b12043ed9476d092f2aeae0..c3ad86c9422df5cfeba2aacec513d903288952e9 100644 (file)
@@ -25,10 +25,8 @@ 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
new file mode 100644 (file)
index 0000000..1360d7c
--- /dev/null
@@ -0,0 +1,27 @@
+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
new file mode 100644 (file)
index 0000000..a1bf5df
--- /dev/null
@@ -0,0 +1,67 @@
+<!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
index 0191cff470713fbe9a28098bf71cfdac50f970a0..78b90283cb7ab3a6f7479b34d11e5f70a8a75fb8 100644 (file)
@@ -719,8 +719,9 @@ webkit.org/b/167757 workers/bomb.html [ Pass Timeout ]
 
 webkit.org/b/172397 [ Sierra Debug ] animations/needs-layout.html [ Pass ImageOnlyFailure ]
 
-# Currently only tests with click, not tap.
+# Move to general wk2 expectations once webkit.org/b/175170 is resolved.
 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 ]
index 644749f4a94ab37f791c61c5ebaa38b03acaf09f..9ef9c0cbb79593feb25b75bba59afa96657035e7 100644 (file)
@@ -1,3 +1,38 @@
+2017-08-04  John Wilander  <wilander@apple.com>
+
+        Resource Load Statistics: Report user interaction immediately, but only when needed
+        https://bugs.webkit.org/show_bug.cgi?id=175090
+        <rdar://problem/33685546>
+
+        Reviewed by Chris Dumez.
+
+        Test: http/tests/loading/resourceLoadStatistics/user-interaction-only-reported-once-within-short-period-of-time.html
+
+        * loader/ResourceLoadObserver.cpp:
+        (WebCore::ResourceLoadObserver::ResourceLoadObserver):
+        (WebCore::ResourceLoadObserver::logUserInteractionWithReducedTimeResolution):
+            Now tells the UI process immediately but also records that it has
+            done so to avoid doing it when not needed.
+        (WebCore::ResourceLoadObserver::scheduleNotificationIfNeeded):
+            Conditional throttling gone, now always throttles.
+        (WebCore::ResourceLoadObserver::notifyObserver):
+            Renamed from ResourceLoadObserver::notificationTimerFired().
+        (WebCore::ResourceLoadObserver::clearState):
+            New function to allow the test runner to reset the web process'
+            statistics state now that we keep track of whether or not we've
+            reported user interaction to the UI process.
+        (WebCore::ResourceLoadObserver::setShouldThrottleObserverNotifications): Deleted.
+        (WebCore::ResourceLoadObserver::notificationTimerFired): Deleted.
+        * loader/ResourceLoadObserver.h:
+        (): Deleted.
+        * testing/Internals.cpp:
+        (WebCore::Internals::resetToConsistentState):
+        (WebCore::Internals::setResourceLoadStatisticsShouldThrottleObserverNotifications): Deleted.
+            No longer needed since user interaction is always communicated
+            immediately.
+        * testing/Internals.h:
+        * testing/Internals.idl:
+
 2017-08-04  Zan Dobersek  <zdobersek@igalia.com>
 
         [EME] Push CDMFactory into the platform layer
index 37b710e51efc3ba244e050edb8bcc4cb02b9abd7..4d1479fb9a79bf99510c4a91a340afbb5f3e34f2 100644 (file)
@@ -99,19 +99,6 @@ 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);
@@ -119,7 +106,7 @@ void ResourceLoadObserver::setNotificationCallback(WTF::Function<void (Vector<Re
 }
 
 ResourceLoadObserver::ResourceLoadObserver()
-    : m_notificationTimer(*this, &ResourceLoadObserver::notificationTimerFired)
+    : m_notificationTimer(*this, &ResourceLoadObserver::notifyObserver)
 {
 }
 
@@ -280,16 +267,20 @@ void ResourceLoadObserver::logUserInteractionWithReducedTimeResolution(const Doc
     if (url.isBlankURL() || url.isEmpty())
         return;
 
-    auto& statistics = ensureResourceStatisticsForPrimaryDomain(primaryDomain(url));
+    auto domain = primaryDomain(url);
     auto newTime = reduceToHourlyTimeResolution(WallTime::now());
-    if (newTime == statistics.mostRecentUserInteractionTime)
+    auto lastReportedUserInteraction = m_lastReportedUserInteractionMap.get(domain);
+    if (newTime == lastReportedUserInteraction)
         return;
 
+    m_lastReportedUserInteractionMap.set(domain, newTime);
+
+    auto& statistics = ensureResourceStatisticsForPrimaryDomain(domain);
     statistics.hadUserInteraction = true;
     statistics.lastSeen = newTime;
     statistics.mostRecentUserInteractionTime = newTime;
 
-    scheduleNotificationIfNeeded();
+    notifyObserver();
 }
 
 ResourceLoadStatistics& ResourceLoadObserver::ensureResourceStatisticsForPrimaryDomain(const String& primaryDomain)
@@ -309,12 +300,13 @@ void ResourceLoadObserver::scheduleNotificationIfNeeded()
     }
 
     if (!m_notificationTimer.isActive())
-        m_notificationTimer.startOneShot(m_shouldThrottleNotifications ? minimumNotificationInterval : 0_s);
+        m_notificationTimer.startOneShot(minimumNotificationInterval);
 }
 
-void ResourceLoadObserver::notificationTimerFired()
+void ResourceLoadObserver::notifyObserver()
 {
     ASSERT(m_notificationCallback);
+    m_notificationTimer.stop();
     m_notificationCallback(takeStatistics());
 }
 
@@ -339,4 +331,10 @@ Vector<ResourceLoadStatistics> ResourceLoadObserver::takeStatistics()
     return statistics;
 }
 
+void ResourceLoadObserver::clearState()
+{
+    m_resourceStatisticsMap.clear();
+    m_lastReportedUserInteractionMap.clear();
+}
+
 } // namespace WebCore
index 32684bd3307a1f5a82129eec95009ff0f0d0e242..dcc29463428dd1308ca3aa1fb113b402d4748d54 100644 (file)
@@ -33,6 +33,7 @@
 namespace WTF {
 class Lock;
 class WorkQueue;
+class WallTime;
 }
 
 namespace WebCore {
@@ -62,6 +63,7 @@ public:
 
     WEBCORE_EXPORT void setNotificationCallback(WTF::Function<void (Vector<ResourceLoadStatistics>&&)>&&);
 
+    WEBCORE_EXPORT void clearState();
 private:
     ResourceLoadObserver();
 
@@ -69,13 +71,13 @@ private:
     ResourceLoadStatistics& ensureResourceStatisticsForPrimaryDomain(const String&);
 
     void scheduleNotificationIfNeeded();
-    void notificationTimerFired();
+    void notifyObserver();
     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
index ee2f91f14de603bf86955dc597d05d614f4c3984..4b25a91c962bedd62c328ea47c007e8cbc3c36fb 100644 (file)
@@ -468,8 +468,6 @@ void Internals::resetToConsistentState(Page& page)
 #if USE(LIBWEBRTC)
     WebCore::useRealRTCPeerConnectionFactory();
 #endif
-    
-    ResourceLoadObserver::shared().setShouldThrottleObserverNotifications(true);
 }
 
 Internals::Internals(Document& document)
@@ -3860,11 +3858,6 @@ 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))
index ec2f22b8f769c1f53b5f1720eb50e1fc52b6735b..0964184d5483af78b55400c8691faeeca79d8bcf 100644 (file)
@@ -539,7 +539,6 @@ public:
 
     String resourceLoadStatisticsForOrigin(const String& origin);
     void setResourceLoadStatisticsEnabled(bool);
-    void setResourceLoadStatisticsShouldThrottleObserverNotifications(bool);
 
 #if ENABLE(STREAMS_API)
     bool isReadableStreamDisturbed(JSC::ExecState&, JSC::JSValue);
index 89d14d71c31b41ca7585a0940492f93c84d651ef..c46e1c69c1156e9bb74c423b46f909af3453f7b3 100644 (file)
@@ -498,7 +498,6 @@ enum EventThrottlingBehavior {
 
     DOMString resourceLoadStatisticsForOrigin(DOMString domain);
     void setResourceLoadStatisticsEnabled(boolean enable);
-    void setResourceLoadStatisticsShouldThrottleObserverNotifications(boolean enable);
 
     [MayThrowException] void setCanShowModalDialogOverride(boolean allow);
 
index b145209e60796262f72bcfa0b6f343d6ddb34844..ab2f7857f8e8f14b4423db53843da964a481ee27 100644 (file)
@@ -1,3 +1,17 @@
+2017-08-04  John Wilander  <wilander@apple.com>
+
+        Resource Load Statistics: Report user interaction immediately, but only when needed
+        https://bugs.webkit.org/show_bug.cgi?id=175090
+        <rdar://problem/33685546>
+
+        Reviewed by Chris Dumez.
+
+        * WebProcess/InjectedBundle/API/c/WKBundle.cpp:
+        (WKBundleClearResourceLoadStatistics):
+            Test infrastructure. Ends up calling
+            WebCore::ResourceLoadObserver::clearState().
+        * WebProcess/InjectedBundle/API/c/WKBundlePrivate.h:
+
 2017-08-04  Antti Koivisto  <antti@apple.com>
 
         Network cache should be usable as non-singleton
index d0826dd9c1018d027737cb40d4c0ea76975fd65d..34bce00338345c5635e6259916ac80b4af56a902 100644 (file)
@@ -39,6 +39,7 @@
 #include "WebPage.h"
 #include "WebPageGroupProxy.h"
 #include <WebCore/DatabaseTracker.h>
+#include <WebCore/ResourceLoadObserver.h>
 
 using namespace WebCore;
 using namespace WebKit;
@@ -276,3 +277,8 @@ void WKBundleSetTabKeyCyclesThroughElements(WKBundleRef bundleRef, WKBundlePageR
 {
     toImpl(bundleRef)->setTabKeyCyclesThroughElements(toImpl(pageRef), enabled);
 }
+
+void WKBundleClearResourceLoadStatistics(WKBundleRef)
+{
+    ResourceLoadObserver::shared().clearState();
+}
index ce8324373b3551d82aac7ee950df4fefc16a2000..9b58a9e82ea2852604455cd3e66c8eb79486348a 100644 (file)
@@ -91,6 +91,8 @@ WK_EXPORT bool WKBundleIsProcessingUserGesture(WKBundleRef bundle);
 
 WK_EXPORT void WKBundleSetTabKeyCyclesThroughElements(WKBundleRef bundle, WKBundlePageRef page, bool enabled);
 
+WK_EXPORT void WKBundleClearResourceLoadStatistics(WKBundleRef);
+
 #ifdef __cplusplus
 }
 #endif
index b45ae4161f39240975f4c537154ef63f04ec52e1..d95d8409e9c51b70551d8abbd76cca3a144682b3 100644 (file)
@@ -1,3 +1,15 @@
+2017-08-04  John Wilander  <wilander@apple.com>
+
+        Resource Load Statistics: Report user interaction immediately, but only when needed
+        https://bugs.webkit.org/show_bug.cgi?id=175090
+        <rdar://problem/33685546>
+
+        Reviewed by Chris Dumez.
+
+        * WebKitTestRunner/InjectedBundle/InjectedBundle.cpp:
+        (WTR::InjectedBundle::beginTesting):
+            Now calls WebCore::ResourceLoadObserver::clearState().
+
 2017-08-03  Brian Burg  <bburg@apple.com>
 
         Remove ENABLE(WEB_SOCKET) guards
index 7a58e23e1d4c5a71be75a78ecac0b72cfbfd2f18..96b0113fcef5ec18bd5624ae81a70553b3c11de1 100644 (file)
@@ -367,6 +367,7 @@ 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