- Revision
- 223307
- Author
- [email protected]
- Date
- 2017-10-13 16:28:01 -0700 (Fri, 13 Oct 2017)
Log Message
CMD+R / CMD+Q keyboard shortcuts are treated as user interaction with page
https://bugs.webkit.org/show_bug.cgi?id=178183
<rdar://problem/33327730>
Reviewed by Ryosuke Niwa.
Source/WebCore:
Key events are granted user interaction credit (in terms of updating the last time of user
interaction), even if the key event was not handled. Instead, we should defer granting
access until the key event has been handled.
Add a new default constructor argument to UserGestureIndicator to be used when handling key
events, so we can delay a decision about whether to grant ResourceLoadStatistics
'hasHadUserInteraction' until we confirm that the event was handled by the page.
This change does not affect other aspects of user interaction.
Tests: fast/events
http/tests/resourceLoadStatistics/prevalent-resource-handled-keydown.html
http/tests/resourceLoadStatistics/prevalent-resource-unhandled-keydown.html
* dom/UserGestureIndicator.cpp:
(WebCore::UserGestureIndicator::UserGestureIndicator): Add check based on constructor argument.
Also: Drive by fix to avoid calling 'currentToken' when not on the main thread.
* dom/UserGestureIndicator.h:
* page/EventHandler.cpp:
(WebCore::EventHandler::keyEvent): If the key event was handled, grant user interaction credit
for ResourceLoadStatistics processing.
(WebCore::EventHandler::internalKeyEvent): Use the new UserGestureIndicator constructor argument.
LayoutTests:
* http/tests/resourceLoadStatistics/prevalent-resource-handled-keydown-expected.txt: Added.
* http/tests/resourceLoadStatistics/prevalent-resource-handled-keydown.html: Added.
* http/tests/resourceLoadStatistics/prevalent-resource-unhandled-keydown-expected.txt: Added.
* http/tests/resourceLoadStatistics/prevalent-resource-unhandled-keydown.html: Added.
* http/tests/resourceLoadStatistics/resources: Added.
* http/tests/resourceLoadStatistics/resources/onclick.html: Added.
* platform/ios/TestExpectations: Skip tests that require 'keyDown' support, since this is not
available on iOS.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (223306 => 223307)
--- trunk/LayoutTests/ChangeLog 2017-10-13 22:36:28 UTC (rev 223306)
+++ trunk/LayoutTests/ChangeLog 2017-10-13 23:28:01 UTC (rev 223307)
@@ -1,3 +1,20 @@
+2017-10-13 Brent Fulgham <[email protected]>
+
+ CMD+R / CMD+Q keyboard shortcuts are treated as user interaction with page
+ https://bugs.webkit.org/show_bug.cgi?id=178183
+ <rdar://problem/33327730>
+
+ Reviewed by Ryosuke Niwa.
+
+ * http/tests/resourceLoadStatistics/prevalent-resource-handled-keydown-expected.txt: Added.
+ * http/tests/resourceLoadStatistics/prevalent-resource-handled-keydown.html: Added.
+ * http/tests/resourceLoadStatistics/prevalent-resource-unhandled-keydown-expected.txt: Added.
+ * http/tests/resourceLoadStatistics/prevalent-resource-unhandled-keydown.html: Added.
+ * http/tests/resourceLoadStatistics/resources: Added.
+ * http/tests/resourceLoadStatistics/resources/onclick.html: Added.
+ * platform/ios/TestExpectations: Skip tests that require 'keyDown' support, since this is not
+ available on iOS.
+
2017-10-13 Matt Lewis <[email protected]>
Marked http/tests/inspector/network/resource-sizes-memory-cache.html as flaky.
Added: trunk/LayoutTests/http/tests/resourceLoadStatistics/prevalent-resource-handled-keydown-expected.txt (0 => 223307)
--- trunk/LayoutTests/http/tests/resourceLoadStatistics/prevalent-resource-handled-keydown-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/resourceLoadStatistics/prevalent-resource-handled-keydown-expected.txt 2017-10-13 23:28:01 UTC (rev 223307)
@@ -0,0 +1,12 @@
+Tests that we grant User Interaction credit for handled keypresses.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Simulate user typing letter 'a' into the field.
+PASS testInput.value is "a"
+PASS Origin was granted user interaction.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/http/tests/resourceLoadStatistics/prevalent-resource-handled-keydown.html (0 => 223307)
--- trunk/LayoutTests/http/tests/resourceLoadStatistics/prevalent-resource-handled-keydown.html (rev 0)
+++ trunk/LayoutTests/http/tests/resourceLoadStatistics/prevalent-resource-handled-keydown.html 2017-10-13 23:28:01 UTC (rev 223307)
@@ -0,0 +1,56 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+description("Tests that we grant User Interaction credit for handled keypresses.");
+jsTestIsAsync = true;
+
+const statisticsUrl = "http://127.0.0.1:8000/temp";
+
+_onload_ = function() {
+ const testFrame = document.getElementById("testFrame");
+
+ if (window.testRunner && window.internals) {
+ internals.setResourceLoadStatisticsEnabled(true);
+ testRunner.setStatisticsNotifyPagesWhenDataRecordsWereScanned(true);
+ }
+
+ testRunner.setStatisticsPrevalentResource(statisticsUrl, true);
+ if (!testRunner.isStatisticsPrevalentResource(statisticsUrl))
+ testFailed("Host did not get set as prevalent resource.");
+
+ testRunner.setStatisticsHasHadUserInteraction(statisticsUrl, false);
+ if (testRunner.isStatisticsHasHadUserInteraction(statisticsUrl))
+ testFailed("Host did not get cleared of user interaction.");
+
+ testInput = document.getElementById("testInput");
+
+ testRunner.installStatisticsDidModifyDataRecordsCallback(function() {
+ shouldBeEqualToString("testInput.value", "a");
+
+ if (!testRunner.isStatisticsHasHadUserInteraction(statisticsUrl))
+ testFailed("Origin did not get user interaction credit.");
+ else
+ testPassed("Origin was granted user interaction.");
+
+ setTimeout(function() {
+ testFrame.src = ""
+ setTimeout(finishJSTest, 0);
+ }, 0);
+ });
+ testRunner.setStatisticsShouldClassifyResourcesBeforeDataRecordsRemoval(false);
+ testRunner.setStatisticsMinimumTimeBetweenDataRecordsRemoval(0);
+ testRunner.statisticsProcessStatisticsAndDataRecords();
+
+ debug("Simulate user typing letter 'a' into the field.");
+ testInput.focus();
+ if (window.eventSender)
+ eventSender.keyDown("a");
+}
+</script>
+<iframe id="testFrame" src=""
+<input id="testInput" type="text"></input>
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/http/tests/resourceLoadStatistics/prevalent-resource-unhandled-keydown-expected.txt (0 => 223307)
--- trunk/LayoutTests/http/tests/resourceLoadStatistics/prevalent-resource-unhandled-keydown-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/resourceLoadStatistics/prevalent-resource-unhandled-keydown-expected.txt 2017-10-13 23:28:01 UTC (rev 223307)
@@ -0,0 +1,11 @@
+Tests that we do not grant User Interaction credit for unhandled keypress.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Simulate an unhandled user key press.
+PASS Origin did not get user interaction credit.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/http/tests/resourceLoadStatistics/prevalent-resource-unhandled-keydown.html (0 => 223307)
--- trunk/LayoutTests/http/tests/resourceLoadStatistics/prevalent-resource-unhandled-keydown.html (rev 0)
+++ trunk/LayoutTests/http/tests/resourceLoadStatistics/prevalent-resource-unhandled-keydown.html 2017-10-13 23:28:01 UTC (rev 223307)
@@ -0,0 +1,49 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+description("Tests that we do not grant User Interaction credit for unhandled keypress.");
+jsTestIsAsync = true;
+
+const statisticsUrl = "http://127.0.0.1:8000/temp";
+
+_onload_ = function() {
+ const testFrame = document.getElementById("testFrame");
+
+ if (window.testRunner && window.internals) {
+ internals.setResourceLoadStatisticsEnabled(true);
+ testRunner.setStatisticsNotifyPagesWhenDataRecordsWereScanned(true);
+ }
+
+ testRunner.setStatisticsPrevalentResource(statisticsUrl, true);
+ if (!testRunner.isStatisticsPrevalentResource(statisticsUrl))
+ testFailed("Host did not get set as prevalent resource.");
+
+ testRunner.setStatisticsHasHadUserInteraction(statisticsUrl, false);
+ if (testRunner.isStatisticsHasHadUserInteraction(statisticsUrl))
+ testFailed("Host did not get cleared of user interaction.");
+
+ testRunner.installStatisticsDidModifyDataRecordsCallback(function() {
+
+ if (!testRunner.isStatisticsHasHadUserInteraction(statisticsUrl))
+ testPassed("Origin did not get user interaction credit.");
+
+ setTimeout(function() {
+ testFrame.src = ""
+ setTimeout(finishJSTest, 0);
+ }, 0);
+ });
+ testRunner.setStatisticsShouldClassifyResourcesBeforeDataRecordsRemoval(false);
+ testRunner.setStatisticsMinimumTimeBetweenDataRecordsRemoval(0);
+ testRunner.statisticsProcessStatisticsAndDataRecords();
+
+ debug("Simulate an unhandled user key press.");
+ if (window.eventSender)
+ eventSender.keyDown("a", ["metaKey"]);
+}
+</script>
+<iframe id="testFrame" src=""
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/http/tests/resourceLoadStatistics/resources/onclick.html (0 => 223307)
--- trunk/LayoutTests/http/tests/resourceLoadStatistics/resources/onclick.html (rev 0)
+++ trunk/LayoutTests/http/tests/resourceLoadStatistics/resources/onclick.html 2017-10-13 23:28:01 UTC (rev 223307)
@@ -0,0 +1,9 @@
+<html>
+<body>
+<table><tr>
+<td id='foo'>
+<a _onclick_="return 'bar'" href=""
+</td>
+</tr></table>
+</body>
+</html>
Modified: trunk/LayoutTests/platform/ios/TestExpectations (223306 => 223307)
--- trunk/LayoutTests/platform/ios/TestExpectations 2017-10-13 22:36:28 UTC (rev 223306)
+++ trunk/LayoutTests/platform/ios/TestExpectations 2017-10-13 23:28:01 UTC (rev 223307)
@@ -378,6 +378,8 @@
fast/events/keyboardevent-code.html [ Skip ]
http/tests/navigation/keyboard-events-during-provisional-navigation.html [ Skip ]
http/tests/navigation/keyboard-events-during-provisional-subframe-navigation.html [ Skip ]
+http/tests/resourceLoadStatistics/prevalent-resource-handled-keydown.html [ Skip ]
+http/tests/resourceLoadStatistics/prevalent-resource-unhandled-keydown.html [ Skip ]
media/audio-dealloc-crash.html [ Skip ]
media/restricted-audio-playback-with-document-gesture.html [ Skip ]
media/restricted-audio-playback-with-multiple-settimeouts.html [ Skip ]
Modified: trunk/Source/WebCore/ChangeLog (223306 => 223307)
--- trunk/Source/WebCore/ChangeLog 2017-10-13 22:36:28 UTC (rev 223306)
+++ trunk/Source/WebCore/ChangeLog 2017-10-13 23:28:01 UTC (rev 223307)
@@ -1,3 +1,34 @@
+2017-10-13 Brent Fulgham <[email protected]>
+
+ CMD+R / CMD+Q keyboard shortcuts are treated as user interaction with page
+ https://bugs.webkit.org/show_bug.cgi?id=178183
+ <rdar://problem/33327730>
+
+ Reviewed by Ryosuke Niwa.
+
+ Key events are granted user interaction credit (in terms of updating the last time of user
+ interaction), even if the key event was not handled. Instead, we should defer granting
+ access until the key event has been handled.
+
+ Add a new default constructor argument to UserGestureIndicator to be used when handling key
+ events, so we can delay a decision about whether to grant ResourceLoadStatistics
+ 'hasHadUserInteraction' until we confirm that the event was handled by the page.
+
+ This change does not affect other aspects of user interaction.
+
+ Tests: fast/events
+ http/tests/resourceLoadStatistics/prevalent-resource-handled-keydown.html
+ http/tests/resourceLoadStatistics/prevalent-resource-unhandled-keydown.html
+
+ * dom/UserGestureIndicator.cpp:
+ (WebCore::UserGestureIndicator::UserGestureIndicator): Add check based on constructor argument.
+ Also: Drive by fix to avoid calling 'currentToken' when not on the main thread.
+ * dom/UserGestureIndicator.h:
+ * page/EventHandler.cpp:
+ (WebCore::EventHandler::keyEvent): If the key event was handled, grant user interaction credit
+ for ResourceLoadStatistics processing.
+ (WebCore::EventHandler::internalKeyEvent): Use the new UserGestureIndicator constructor argument.
+
2017-10-13 Chris Dumez <[email protected]>
DOMTokenList shouldn't add empty attributes
Modified: trunk/Source/WebCore/dom/UserGestureIndicator.cpp (223306 => 223307)
--- trunk/Source/WebCore/dom/UserGestureIndicator.cpp 2017-10-13 22:36:28 UTC (rev 223306)
+++ trunk/Source/WebCore/dom/UserGestureIndicator.cpp 2017-10-13 23:28:01 UTC (rev 223307)
@@ -46,19 +46,22 @@
observer(*this);
}
-UserGestureIndicator::UserGestureIndicator(std::optional<ProcessingUserGestureState> state, Document* document, UserGestureType gestureType)
- : m_previousToken(currentToken())
+UserGestureIndicator::UserGestureIndicator(std::optional<ProcessingUserGestureState> state, Document* document, UserGestureType gestureType, ProcessInteractionStyle processInteractionStyle)
{
// Silently ignore UserGestureIndicators on non main threads.
if (!isMainThread())
return;
+ // It is only safe to use currentToken() on the main thread.
+ m_previousToken = currentToken();
+
if (state)
currentToken() = UserGestureToken::create(state.value(), gestureType);
if (document && currentToken()->processingUserGesture()) {
document->updateLastHandledUserGestureTimestamp(MonotonicTime::now());
- ResourceLoadObserver::shared().logUserInteractionWithReducedTimeResolution(document->topDocument());
+ if (processInteractionStyle == ProcessInteractionStyle::Immediate)
+ ResourceLoadObserver::shared().logUserInteractionWithReducedTimeResolution(document->topDocument());
document->topDocument().setUserDidInteractWithPage(true);
}
}
Modified: trunk/Source/WebCore/dom/UserGestureIndicator.h (223306 => 223307)
--- trunk/Source/WebCore/dom/UserGestureIndicator.h 2017-10-13 22:36:28 UTC (rev 223306)
+++ trunk/Source/WebCore/dom/UserGestureIndicator.h 2017-10-13 23:28:01 UTC (rev 223307)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2010-2017 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -84,7 +84,8 @@
WEBCORE_EXPORT static bool processingUserGestureForMedia();
// If a document is provided, its last known user gesture timestamp is updated.
- WEBCORE_EXPORT explicit UserGestureIndicator(std::optional<ProcessingUserGestureState>, Document* = nullptr, UserGestureType = UserGestureType::Other);
+ enum class ProcessInteractionStyle { Immediate, Delayed };
+ WEBCORE_EXPORT explicit UserGestureIndicator(std::optional<ProcessingUserGestureState>, Document* = nullptr, UserGestureType = UserGestureType::Other, ProcessInteractionStyle = ProcessInteractionStyle::Immediate);
WEBCORE_EXPORT explicit UserGestureIndicator(RefPtr<UserGestureToken>);
WEBCORE_EXPORT ~UserGestureIndicator();
Modified: trunk/Source/WebCore/page/EventHandler.cpp (223306 => 223307)
--- trunk/Source/WebCore/page/EventHandler.cpp 2017-10-13 22:36:28 UTC (rev 223306)
+++ trunk/Source/WebCore/page/EventHandler.cpp 2017-10-13 23:28:01 UTC (rev 223307)
@@ -78,6 +78,7 @@
#include "RenderTextControlSingleLine.h"
#include "RenderView.h"
#include "RenderWidget.h"
+#include "ResourceLoadObserver.h"
#include "RuntimeApplicationChecks.h"
#include "SVGDocument.h"
#include "SVGNames.h"
@@ -3125,8 +3126,12 @@
bool wasHandled = internalKeyEvent(keyEvent);
// If the key event was not handled, do not treat it as user interaction with the page.
- if (topDocument && !wasHandled)
- topDocument->setUserDidInteractWithPage(savedUserDidInteractWithPage);
+ if (topDocument) {
+ if (!wasHandled)
+ topDocument->setUserDidInteractWithPage(savedUserDidInteractWithPage);
+ else
+ ResourceLoadObserver::shared().logUserInteractionWithReducedTimeResolution(topDocument->topDocument());
+ }
return wasHandled;
}
@@ -3191,7 +3196,7 @@
if (initialKeyEvent.windowsVirtualKeyCode() == VK_ESCAPE)
gestureType = UserGestureType::EscapeKey;
- UserGestureIndicator gestureIndicator(ProcessingUserGesture, m_frame.document(), gestureType);
+ UserGestureIndicator gestureIndicator(ProcessingUserGesture, m_frame.document(), gestureType, UserGestureIndicator::ProcessInteractionStyle::Delayed);
UserTypingGestureIndicator typingGestureIndicator(m_frame);
if (FrameView* view = m_frame.view())
