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 22:34:57 +0000 (22:34 +0000)
committerwilander@apple.com <wilander@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Aug 2017 22:34:57 +0000 (22:34 +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@220302 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 64cf85225ba5d0152bdf81b9f30a5c8528f4161a..141d57a3d18a21f67ca99fd0731880a9f1f24162 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  Matt Baker  <mattbaker@apple.com>
 
         Web Inspector: capture async stack trace when workers/main context posts a message
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..abab6b1
--- /dev/null
@@ -0,0 +1,22 @@
+main frame - didStartProvisionalLoadForFrame
+main frame - didCommitLoadForFrame
+main frame - didFinishDocumentLoadForFrame
+main frame - didHandleOnloadEventsForFrame
+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
+This is the test element
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..414a8b3
--- /dev/null
@@ -0,0 +1,64 @@
+<!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, callback) {
+        var element = document.getElementById(elementId);
+        var centerX = element.offsetLeft + element.offsetWidth / 2;
+        var centerY = element.offsetTop + element.offsetHeight / 2;
+        UIHelper.activateAt(centerX, centerY).then(
+            function() {
+                callback();
+            },
+            function() {
+                testFailed("Promise rejected.");
+                finishJSTest();
+            }
+        );
+    }
+
+    function firstInteraction() {
+        shouldBeFalse("testRunner.isStatisticsHasHadUserInteraction(topFrameOrigin)");
+        shouldBeFalse("testRunner.isStatisticsHasHadUserInteraction(subFrameOrigin)");
+        activateElement("testElement", secondInteraction);
+    }
+
+    function secondInteraction() {
+        shouldBeTrue("testRunner.isStatisticsHasHadUserInteraction(topFrameOrigin)");
+        shouldBeFalse("testRunner.isStatisticsHasHadUserInteraction(subFrameOrigin)");
+
+        if (testRunner)
+            testRunner.setStatisticsHasHadUserInteraction(topFrameOrigin, false);
+
+        shouldBeFalse("testRunner.isStatisticsHasHadUserInteraction(topFrameOrigin)");
+
+        activateElement("testElement", finishTest);
+    }
+
+    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>
+<div id="testElement">This is the test element</div>
+</body>
+</html>
\ No newline at end of file
index a51c18350bc872a3d7b77aa2a799abee096a23ab..59f86b4152bc8568f542ffc19dc3c15775839f1a 100644 (file)
@@ -728,8 +728,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 90444364610c64107e8a8e3caa767067902e9b4b..a198e4f00a3216f0747066785127433c839a84a0 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  Matt Baker  <mattbaker@apple.com>
 
         Web Inspector: capture async stack trace when workers/main context posts a message
index 37b710e51efc3ba244e050edb8bcc4cb02b9abd7..5c228ebb429a93d00637076477cdd0dcdab4c4f6 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,21 @@ 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();
+    m_notificationTimer.stop();
+    notifyObserver();
 }
 
 ResourceLoadStatistics& ResourceLoadObserver::ensureResourceStatisticsForPrimaryDomain(const String& primaryDomain)
@@ -309,12 +301,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 +332,11 @@ Vector<ResourceLoadStatistics> ResourceLoadObserver::takeStatistics()
     return statistics;
 }
 
+void ResourceLoadObserver::clearState()
+{
+    m_notificationTimer.stop();
+    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 cf3b5389b3fffc6938a820f8a58e12796b111aa2..18f0715217f9d5fec4dc50a394b8330f14cbc4b3 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  Matt Lewis  <jlewis3@apple.com>
 
         Unreviewed, rolling out r220288.
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 6575c46ab5d9db51b456d5695938caf03c6a80fd..73803de5eb79cfbc0ee87d22562c6c25d539eebe 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-04  Tim Horton  <timothy_horton@apple.com>
 
         Add an API test for r220286
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