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 fa8860c..e201387 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 abdd07e..c3ad86c 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 0191cff..78b9028 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 644749f..9ef9c0c 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 37b710e..4d1479f 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 32684bd..dcc2946 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 ee2f91f..4b25a91 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 ec2f22b..0964184 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 89d14d7..c46e1c6 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 b145209..ab2f785 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 d0826dd..34bce00 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 ce83243..9b58a9e 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 b45ae41..d95d840 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 7a58e23..96b0113 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