Typing into a cell in a Google Sheet lags behind by one character
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Jul 2019 20:06:04 +0000 (20:06 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 17 Jul 2019 20:06:04 +0000 (20:06 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199587
<rdar://problem/51616845>

Reviewed by Brent Fulgham.

Source/WebCore:

Add a Google Sheets quirk. Put all DOM timers scheduled from keydown and keypress event listeners
into a holding tank. The timers continue to tick, but are barred from executing their action until
the next text insertion or deletion or 32 ms (on device) have elapsed, whichever is sooner. We only
allocate a holding tank once per document, only if the quirk is active, and this allocation is done
when the document schedules a timer on keydown or keypress. The holding tank lives for the lifetime
of the document.

The story behind the quirk:

On keypress Google Sheets schedules timers and expects that a DOM update will occur (i.e. text
will be inserted or deleted) within the same event loop iteration as the dispatched keypress. The
UI Events spec. [1] makes no such guarantee of when a DOM update must occur in relation to the keypress
event. It could happen in the same event loop iteration as the key press (as Google expects), the
next iteration, 500ms later, 2 minutes later, etc. What the spec does guarantee is that by the time
a DOM input event is dispatched that the DOM will be updated. And this is the solution to the problem
Google Sheets is trying to solve, but is doing so using pre-IE 9 technology (though similar
functionality was available via onpropertychange in IE < 9).

See also <https://github.com/w3c/uievents/issues/238>, which is tracking a spec. text update for
this quirk.

Test: fast/events/ios/dom-update-on-keydown-quirk.html

[1] <https://w3c.github.io/uievents/> (Editor's Draft, 14 October 2018)

* SourcesCocoa.txt:
* WebCore.xcodeproj/project.pbxproj:
Add some files to the project.

* dom/Document.cpp:
(WebCore::Document::domTimerHoldingTank): Added.
* dom/Document.h:
(WebCore::Document::domTimerHoldingTankIfExists): Added.

* page/DOMTimer.cpp:
(WebCore::DOMTimer::install): Put the newly instantiated timer into the holding tank.
(WebCore::DOMTimer::removeById): Remove the timer from the holding tank.
(WebCore::DOMTimer::fired): Check if the timer is in the holding tank. If it is and it is a one-
shot timer then schedule it for the next event loop iteration. If it's a repeating timer just
let it continue ticking. Otherwise, do what we no now and execute the timer's action. The reason
we do not suspend timers in the holding tank is because:
    1. Far out timers (Google Sheets registers timers as far out as 5 minutes!) are not penalized.
    Though smart supension logic could avoid this. See (3).

    2. Empirical observations indicate that the keyboard will perform the insertion or deletion
    reasonably quickly (not the same event loop iteration as the keydown, but within two iterations out).
    So, the timers in the holding tank are short-lived.

    3. Simplifies the code. There is no need to keep additional bookkeeping to track multiple timer
    suspension reasons (timers currently can only have one suspension reason) or alternatively defer
    scheduling a timer until a later time and computing a new "fair" firing time when scheduled.
* page/EventHandler.cpp:
(WebCore::EventHandler::internalKeyEvent): Place a token on the stack to put all DOM timers
scheduled on keydown and keypress into the holding tank if the quirk is enabled.
* page/Quirks.cpp:
(WebCore::Quirks::needsDeferKeyDownAndKeyPressTimersUntilNextEditingCommand const): Added.
* page/Quirks.h:
* page/Settings.yaml: Added setting so that this quirk can be enabled from a layout test. This setting
also lets us enable the quirk for all sites or for certain third-party apps if desired.
* page/ios/DOMTimerHoldingTank.cpp: Added.
(WebCore::DOMTimerHoldingTank::DOMTimerHoldingTank):
(WebCore::DOMTimerHoldingTank::add):
(WebCore::DOMTimerHoldingTank::remove):
(WebCore::DOMTimerHoldingTank::contains):
(WebCore::DOMTimerHoldingTank::removeAll):
(WebCore::DOMTimerHoldingTank::stopExceededMaximumHoldTimer):
* page/ios/DOMTimerHoldingTank.h: Added.
(WebCore::DeferDOMTimersForScope::DeferDOMTimersForScope):
(WebCore::DeferDOMTimersForScope::~DeferDOMTimersForScope):
(WebCore::DeferDOMTimersForScope::isDeferring):

Source/WebKit:

Remove all timers from the holding tank on text insertion or deletion (represented as an
editing command). Timers that were in the holding tank never stopped ticking and will now
be able to execute their action.

* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::executeEditingCommand):
(WebKit::WebPage::insertTextAsync):
(WebKit::WebPage::setCompositionAsync):
(WebKit::WebPage::confirmCompositionAsync):
Call platformWillPerformEditingCommand().

* WebProcess/WebPage/WebPage.h:
(WebKit::WebPage::platformWillPerformEditingCommand): Added.
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::platformWillPerformEditingCommand): Remove all the timers from the holding
tank if we have a holding tank.

LayoutTests:

Add a test that enables the quirk and ensures that the DOM is up-to-date on expiration of a
zero timer scheduled from keydown, keypress, keyup, and input.

* fast/events/ios/dom-update-on-keydown-quirk-expected.txt: Added.
* fast/events/ios/dom-update-on-keydown-quirk.html: Added.

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

19 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/events/ios/dom-update-on-keydown-quirk-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/ios/dom-update-on-keydown-quirk.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/SourcesCocoa.txt
Source/WebCore/WebCore.xcodeproj/project.pbxproj
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/page/DOMTimer.cpp
Source/WebCore/page/EventHandler.cpp
Source/WebCore/page/Quirks.cpp
Source/WebCore/page/Quirks.h
Source/WebCore/page/Settings.yaml
Source/WebCore/page/ios/DOMTimerHoldingTank.cpp [new file with mode: 0644]
Source/WebCore/page/ios/DOMTimerHoldingTank.h [new file with mode: 0644]
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebPage/WebPage.cpp
Source/WebKit/WebProcess/WebPage/WebPage.h
Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

index a679453..34602c0 100644 (file)
@@ -1,3 +1,17 @@
+2019-07-17  Daniel Bates  <dabates@apple.com>
+
+        Typing into a cell in a Google Sheet lags behind by one character
+        https://bugs.webkit.org/show_bug.cgi?id=199587
+        <rdar://problem/51616845>
+
+        Reviewed by Brent Fulgham.
+
+        Add a test that enables the quirk and ensures that the DOM is up-to-date on expiration of a
+        zero timer scheduled from keydown, keypress, keyup, and input.
+
+        * fast/events/ios/dom-update-on-keydown-quirk-expected.txt: Added.
+        * fast/events/ios/dom-update-on-keydown-quirk.html: Added.
+
 2019-07-17  Myles C. Maxfield  <mmaxfield@apple.com>
 
         [WHLSL] Move WHLSL tests to their own folder
diff --git a/LayoutTests/fast/events/ios/dom-update-on-keydown-quirk-expected.txt b/LayoutTests/fast/events/ios/dom-update-on-keydown-quirk-expected.txt
new file mode 100644 (file)
index 0000000..fad26c2
--- /dev/null
@@ -0,0 +1,21 @@
+This tests that the value of the field is updated by the time any timer scheduled on keydown, keypress, or keyup fires. To run this test manually, focus the text field and press [.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+
+For keydown:
+PASS document.getElementById("input").value is "["
+
+For keypress:
+PASS document.getElementById("input").value is "["
+
+For input:
+PASS document.getElementById("input").value is "["
+
+For keyup:
+PASS document.getElementById("input").value is "["
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/events/ios/dom-update-on-keydown-quirk.html b/LayoutTests/fast/events/ios/dom-update-on-keydown-quirk.html
new file mode 100644 (file)
index 0000000..38e22f7
--- /dev/null
@@ -0,0 +1,53 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../../resources/js-test.js"></script>
+<script src="../../../resources/ui-helper.js"></script>
+</head>
+<body>
+<input id="input">
+<script>
+const expectedValue = "["; // [ chosen to avoid having to handle or disable iOS auto capitalization. 
+
+let input = document.getElementById("input");
+
+function done()
+{
+    document.body.removeChild(input);
+    finishJSTest();
+}
+
+async function runTest()
+{
+    if (window.internals.settings)
+        internals.settings.setNeedsDeferKeyDownAndKeyPressTimersUntilNextEditingCommandQuirk(true);
+
+    if (window.testRunner)
+        await UIHelper.callFunctionAndWaitForEvent(() => UIHelper.activateElement(input), input, "focus");
+
+    function checkResult(event) {
+        // Note that scheduling a timer from an Input event handler is for aesthetics only: to make the
+        // logged Input event be ordered like the spec'ed DOM dispatch event order. By the time the Input
+        // event fires the DOM is guaranteed to have been updated. So, no timer is needed.
+        window.setTimeout(() => {
+            debug(`<br>For ${event.type}:`);
+            shouldBeEqualToString('document.getElementById("input").value', expectedValue);
+            if (event.type === "keyup")
+                done();
+        }, 0);
+    }
+
+    for (let eventName of ["keydown", "keypress", "keyup", "input"])
+        input.addEventListener(eventName, checkResult, { once: true });
+
+    if (window.testRunner)
+        UIHelper.keyDown(expectedValue);
+}
+
+window.jsTestIsAsync = true;
+description("This tests that the value of the field is updated by the time any timer scheduled on keydown, keypress, or keyup fires. To run this test manually, focus the text field and press <kbd>[</kbd>.");
+
+runTest();
+</script>
+</body>
+</html>
\ No newline at end of file
index 10a5ff5..da0e43d 100644 (file)
@@ -1,3 +1,82 @@
+2019-07-17  Daniel Bates  <dabates@apple.com>
+
+        Typing into a cell in a Google Sheet lags behind by one character
+        https://bugs.webkit.org/show_bug.cgi?id=199587
+        <rdar://problem/51616845>
+
+        Reviewed by Brent Fulgham.
+
+        Add a Google Sheets quirk. Put all DOM timers scheduled from keydown and keypress event listeners
+        into a holding tank. The timers continue to tick, but are barred from executing their action until
+        the next text insertion or deletion or 32 ms (on device) have elapsed, whichever is sooner. We only
+        allocate a holding tank once per document, only if the quirk is active, and this allocation is done
+        when the document schedules a timer on keydown or keypress. The holding tank lives for the lifetime
+        of the document.
+
+        The story behind the quirk:
+
+        On keypress Google Sheets schedules timers and expects that a DOM update will occur (i.e. text
+        will be inserted or deleted) within the same event loop iteration as the dispatched keypress. The
+        UI Events spec. [1] makes no such guarantee of when a DOM update must occur in relation to the keypress
+        event. It could happen in the same event loop iteration as the key press (as Google expects), the
+        next iteration, 500ms later, 2 minutes later, etc. What the spec does guarantee is that by the time
+        a DOM input event is dispatched that the DOM will be updated. And this is the solution to the problem
+        Google Sheets is trying to solve, but is doing so using pre-IE 9 technology (though similar
+        functionality was available via onpropertychange in IE < 9).
+
+        See also <https://github.com/w3c/uievents/issues/238>, which is tracking a spec. text update for
+        this quirk.
+
+        Test: fast/events/ios/dom-update-on-keydown-quirk.html
+
+        [1] <https://w3c.github.io/uievents/> (Editor's Draft, 14 October 2018)
+
+        * SourcesCocoa.txt:
+        * WebCore.xcodeproj/project.pbxproj:
+        Add some files to the project.
+
+        * dom/Document.cpp:
+        (WebCore::Document::domTimerHoldingTank): Added.
+        * dom/Document.h:
+        (WebCore::Document::domTimerHoldingTankIfExists): Added.
+
+        * page/DOMTimer.cpp:
+        (WebCore::DOMTimer::install): Put the newly instantiated timer into the holding tank.
+        (WebCore::DOMTimer::removeById): Remove the timer from the holding tank.
+        (WebCore::DOMTimer::fired): Check if the timer is in the holding tank. If it is and it is a one-
+        shot timer then schedule it for the next event loop iteration. If it's a repeating timer just
+        let it continue ticking. Otherwise, do what we no now and execute the timer's action. The reason
+        we do not suspend timers in the holding tank is because:
+            1. Far out timers (Google Sheets registers timers as far out as 5 minutes!) are not penalized.
+            Though smart supension logic could avoid this. See (3).
+
+            2. Empirical observations indicate that the keyboard will perform the insertion or deletion
+            reasonably quickly (not the same event loop iteration as the keydown, but within two iterations out).
+            So, the timers in the holding tank are short-lived.
+
+            3. Simplifies the code. There is no need to keep additional bookkeeping to track multiple timer
+            suspension reasons (timers currently can only have one suspension reason) or alternatively defer
+            scheduling a timer until a later time and computing a new "fair" firing time when scheduled.
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::internalKeyEvent): Place a token on the stack to put all DOM timers
+        scheduled on keydown and keypress into the holding tank if the quirk is enabled.
+        * page/Quirks.cpp:
+        (WebCore::Quirks::needsDeferKeyDownAndKeyPressTimersUntilNextEditingCommand const): Added.
+        * page/Quirks.h:
+        * page/Settings.yaml: Added setting so that this quirk can be enabled from a layout test. This setting
+        also lets us enable the quirk for all sites or for certain third-party apps if desired.
+        * page/ios/DOMTimerHoldingTank.cpp: Added.
+        (WebCore::DOMTimerHoldingTank::DOMTimerHoldingTank):
+        (WebCore::DOMTimerHoldingTank::add):
+        (WebCore::DOMTimerHoldingTank::remove):
+        (WebCore::DOMTimerHoldingTank::contains):
+        (WebCore::DOMTimerHoldingTank::removeAll):
+        (WebCore::DOMTimerHoldingTank::stopExceededMaximumHoldTimer):
+        * page/ios/DOMTimerHoldingTank.h: Added.
+        (WebCore::DeferDOMTimersForScope::DeferDOMTimersForScope):
+        (WebCore::DeferDOMTimersForScope::~DeferDOMTimersForScope):
+        (WebCore::DeferDOMTimersForScope::isDeferring):
+
 2019-07-17  Darin Adler  <darin@apple.com>
 
         No need for isURLAllowed function in Frame
index db8aa2e..e83348a 100644 (file)
@@ -131,6 +131,7 @@ page/cocoa/ResourceUsageThreadCocoa.mm
 page/cocoa/SettingsBaseCocoa.mm
 
 page/ios/ContentChangeObserver.cpp
+page/ios/DOMTimerHoldingTank.cpp
 page/ios/EventHandlerIOS.mm
 page/ios/FrameIOS.mm
 page/ios/WebEventRegion.mm
index bbb7cf7..81993cf 100644 (file)
                CE057FA61220731100A476D5 /* DocumentMarkerController.h in Headers */ = {isa = PBXBuildFile; fileRef = CE057FA41220731100A476D5 /* DocumentMarkerController.h */; settings = {ATTRIBUTES = (Private, ); }; };
                CE08C3D2152B599A0021B8C2 /* AlternativeTextController.h in Headers */ = {isa = PBXBuildFile; fileRef = CE08C3D0152B599A0021B8C2 /* AlternativeTextController.h */; settings = {ATTRIBUTES = (); }; };
                CE1866451F72E5B400A0CAB6 /* MarkedText.h in Headers */ = {isa = PBXBuildFile; fileRef = CE1866431F72E5B400A0CAB6 /* MarkedText.h */; settings = {ATTRIBUTES = (Private, ); }; };
+               CE1A501F22D5350900CBC927 /* DOMTimerHoldingTank.h in Headers */ = {isa = PBXBuildFile; fileRef = CE1A501D22D5350900CBC927 /* DOMTimerHoldingTank.h */; settings = {ATTRIBUTES = (Private, ); }; };
                CE2849871CA360DF00B4A57F /* ContentSecurityPolicyDirectiveNames.h in Headers */ = {isa = PBXBuildFile; fileRef = CE2849861CA360DF00B4A57F /* ContentSecurityPolicyDirectiveNames.h */; };
                CE5169E721F1B84700EA4F78 /* ColorIOS.h in Headers */ = {isa = PBXBuildFile; fileRef = CE5169E521F1B84700EA4F78 /* ColorIOS.h */; settings = {ATTRIBUTES = (Private, ); }; };
                CE5FA255209E48C50051D700 /* ContentSecurityPolicyClient.h in Headers */ = {isa = PBXBuildFile; fileRef = CE5FA253209E48C50051D700 /* ContentSecurityPolicyClient.h */; settings = {ATTRIBUTES = (Private, ); }; };
                CE08C3D0152B599A0021B8C2 /* AlternativeTextController.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = AlternativeTextController.h; sourceTree = "<group>"; };
                CE1866421F72E5B400A0CAB6 /* MarkedText.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = MarkedText.cpp; sourceTree = "<group>"; };
                CE1866431F72E5B400A0CAB6 /* MarkedText.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MarkedText.h; sourceTree = "<group>"; };
+               CE1A501D22D5350900CBC927 /* DOMTimerHoldingTank.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = DOMTimerHoldingTank.h; sourceTree = "<group>"; };
+               CE1A501E22D5350900CBC927 /* DOMTimerHoldingTank.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = DOMTimerHoldingTank.cpp; sourceTree = "<group>"; };
                CE2849861CA360DF00B4A57F /* ContentSecurityPolicyDirectiveNames.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = ContentSecurityPolicyDirectiveNames.h; path = csp/ContentSecurityPolicyDirectiveNames.h; sourceTree = "<group>"; };
                CE2849881CA3614600B4A57F /* ContentSecurityPolicyDirectiveNames.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = ContentSecurityPolicyDirectiveNames.cpp; path = csp/ContentSecurityPolicyDirectiveNames.cpp; sourceTree = "<group>"; };
                CE5169E521F1B84700EA4F78 /* ColorIOS.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ColorIOS.h; sourceTree = "<group>"; };
                        children = (
                                6FE9F09222211035004C5082 /* ContentChangeObserver.cpp */,
                                6FB5E212221F2447003989CF /* ContentChangeObserver.h */,
+                               CE1A501E22D5350900CBC927 /* DOMTimerHoldingTank.cpp */,
+                               CE1A501D22D5350900CBC927 /* DOMTimerHoldingTank.h */,
                                FE6938B51045D67E008EABB6 /* EventHandlerIOS.mm */,
                                FED13D3B0CEA936A00D89466 /* FrameIOS.mm */,
                                225A16B30D5C11E900090295 /* WebEventRegion.h */,
                                C544274B11A57E7A0063A749 /* DOMStringList.h in Headers */,
                                BC64640A11D7F304006455B0 /* DOMStringMap.h in Headers */,
                                188604B40F2E654A000B6443 /* DOMTimer.h in Headers */,
+                               CE1A501F22D5350900CBC927 /* DOMTimerHoldingTank.h in Headers */,
                                05FD69E012845D4300B2BEB3 /* DOMTimeStamp.h in Headers */,
                                76FC2B0C12370DA0006A991A /* DOMTokenList.h in Headers */,
                                2E37DFDB12DBAFB800A6B233 /* DOMURL.h in Headers */,
index b0d9d7f..f6a422e 100644 (file)
 #if PLATFORM(IOS_FAMILY)
 #include "ContentChangeObserver.h"
 #include "CSSFontSelector.h"
+#include "DOMTimerHoldingTank.h"
 #include "DeviceMotionClientIOS.h"
 #include "DeviceMotionController.h"
 #include "DeviceOrientationClientIOS.h"
@@ -8182,12 +8183,22 @@ void Document::setPaintWorkletGlobalScopeForName(const String& name, Ref<PaintWo
 #endif
 
 #if PLATFORM(IOS_FAMILY)
+
 ContentChangeObserver& Document::contentChangeObserver()
 {
     if (!m_contentChangeObserver)
         m_contentChangeObserver = std::make_unique<ContentChangeObserver>(*this);
     return *m_contentChangeObserver; 
 }
+
+DOMTimerHoldingTank& Document::domTimerHoldingTank()
+{
+    if (m_domTimerHoldingTank)
+        return *m_domTimerHoldingTank;
+    m_domTimerHoldingTank = std::make_unique<DOMTimerHoldingTank>();
+    return *m_domTimerHoldingTank;
+}
+
 #endif
 
 bool Document::hasEvaluatedUserAgentScripts() const
index a022efd..34913c5 100644 (file)
@@ -106,6 +106,7 @@ class ConstantPropertyMap;
 class ContentChangeObserver;
 class DOMImplementation;
 class DOMSelection;
+class DOMTimerHoldingTank;
 class DOMWindow;
 class DOMWrapperWorld;
 class Database;
@@ -883,6 +884,9 @@ public:
     void processWebAppOrientations();
 
     WEBCORE_EXPORT ContentChangeObserver& contentChangeObserver();
+
+    DOMTimerHoldingTank* domTimerHoldingTankIfExists() { return m_domTimerHoldingTank.get(); }
+    DOMTimerHoldingTank& domTimerHoldingTank();
 #endif
     
     void processViewport(const String& features, ViewportArguments::Type origin);
@@ -2044,6 +2048,7 @@ private:
     Ref<UndoManager> m_undoManager;
 #if PLATFORM(IOS_FAMILY)
     std::unique_ptr<ContentChangeObserver> m_contentChangeObserver;
+    std::unique_ptr<DOMTimerHoldingTank> m_domTimerHoldingTank;
 #endif
 
     HashMap<Element*, ElementIdentifier> m_identifiedElementsMap;
index 6a22dbe..7f57537 100644 (file)
@@ -43,6 +43,7 @@
 
 #if PLATFORM(IOS_FAMILY)
 #include "ContentChangeObserver.h"
+#include "DOMTimerHoldingTank.h"
 #endif
 
 namespace WebCore {
@@ -196,8 +197,12 @@ int DOMTimer::install(ScriptExecutionContext& context, std::unique_ptr<Scheduled
     if (NestedTimersMap* nestedTimers = NestedTimersMap::instanceForContext(context))
         nestedTimers->add(timer->m_timeoutId, *timer);
 #if PLATFORM(IOS_FAMILY)
-    if (is<Document>(context))
-        downcast<Document>(context).contentChangeObserver().didInstallDOMTimer(*timer, timeout, singleShot);
+    if (is<Document>(context)) {
+        auto& document = downcast<Document>(context);
+        document.contentChangeObserver().didInstallDOMTimer(*timer, timeout, singleShot);
+        if (DeferDOMTimersForScope::isDeferring())
+            document.domTimerHoldingTank().add(*timer);
+    }
 #endif
     return timer->m_timeoutId;
 }
@@ -213,8 +218,11 @@ void DOMTimer::removeById(ScriptExecutionContext& context, int timeoutId)
 #if PLATFORM(IOS_FAMILY)
     if (is<Document>(context)) {
         auto& document = downcast<Document>(context);
-        if (auto* timer = document.findTimeout(timeoutId))
+        if (auto* timer = document.findTimeout(timeoutId)) {
             document.contentChangeObserver().didRemoveDOMTimer(*timer);
+            if (auto* holdingTank = document.domTimerHoldingTankIfExists())
+                holdingTank->remove(*timer);
+        }
     }
 #endif
 
@@ -284,6 +292,17 @@ void DOMTimer::fired()
     ASSERT(scriptExecutionContext());
     ScriptExecutionContext& context = *scriptExecutionContext();
 
+#if PLATFORM(IOS_FAMILY)
+    if (is<Document>(context)) {
+        auto& document = downcast<Document>(context);
+        if (auto* holdingTank = document.domTimerHoldingTankIfExists(); holdingTank && holdingTank->contains(*this)) {
+            if (!repeatInterval())
+                startOneShot(0_s);
+            return;
+        }
+    }
+#endif
+
     DOMTimerFireState fireState(context, std::min(m_nestingLevel + 1, maxTimerNestingLevel));
 
     if (m_userGestureTokenToForward && m_userGestureTokenToForward->hasExpired(maxIntervalForUserGestureForwarding))
index ef05ff1..b97a8d5 100644 (file)
 #include "RuntimeEnabledFeatures.h"
 #endif
 
+#if PLATFORM(IOS_FAMILY)
+#include "DOMTimerHoldingTank.h"
+#endif
+
 namespace WebCore {
 
 using namespace HTMLNames;
@@ -3341,6 +3345,10 @@ bool EventHandler::internalKeyEvent(const PlatformKeyboardEvent& initialKeyEvent
     if (accessibilityPreventsEventPropagation(keydown))
         keydown->stopPropagation();
 
+#if PLATFORM(IOS_FAMILY)
+    DeferDOMTimersForScope deferralScope { m_frame.document()->quirks().needsDeferKeyDownAndKeyPressTimersUntilNextEditingCommand() };
+#endif
+
     element->dispatchEvent(keydown);
     if (handledByInputMethod)
         return true;
index b4f0ffd..1c7ef8b 100644 (file)
@@ -317,6 +317,22 @@ bool Quirks::shouldDisablePointerEventsQuirk() const
     return false;
 }
 
+bool Quirks::needsDeferKeyDownAndKeyPressTimersUntilNextEditingCommand() const
+{
+#if PLATFORM(IOS_FAMILY)
+    if (m_document->settings().needsDeferKeyDownAndKeyPressTimersUntilNextEditingCommandQuirk())
+        return true;
+
+    if (!needsQuirks())
+        return false;
+
+    auto& url = m_document->topDocument().url();
+    return equalLettersIgnoringASCIICase(url.host(), "docs.google.com") && url.path().startsWithIgnoringASCIICase("/spreadsheets/");
+#else
+    return false;
+#endif
+}
+
 // FIXME(<rdar://problem/50394969>): Remove after desmos.com adopts inputmode="none".
 bool Quirks::needsInputModeNoneImplicitly(const HTMLElement& element) const
 {
index 30d1066..28d2e54 100644 (file)
@@ -54,6 +54,7 @@ public:
 #endif
     bool shouldDisablePointerEventsQuirk() const;
     bool needsInputModeNoneImplicitly(const HTMLElement&) const;
+    bool needsDeferKeyDownAndKeyPressTimersUntilNextEditingCommand() const;
 
     WEBCORE_EXPORT bool shouldDispatchSyntheticMouseEventsWhenModifyingSelection() const;
     WEBCORE_EXPORT bool shouldSuppressAutocorrectionAndAutocaptializationInHiddenEditableAreas() const;
index 9f07553..45625e1 100644 (file)
@@ -162,6 +162,15 @@ googleAntiFlickerOptimizationQuirkEnabled:
 needsKeyboardEventDisambiguationQuirks:
   initial: false
 
+# This is an iOS-specific quirk. Unlike Mac, keyboard operations are asynchronous and hence a DOM update as
+# a result of text insertion or deletion does not occur within the same event loop iteration as a dispatched
+# DOM keydown event. Some sites, notably Google Sheets, schedule timers on keypress and expect on a DOM update
+# to have occurred on expiration. When enabled, this quirk puts all such scheduled timers in a holding tank
+# until the keyboard performs the insertion or deletion. This gives Google Sheets the illusion that the DOM
+# update happened within the same event loop iteration that the keypress event was dispatched in.
+needsDeferKeyDownAndKeyPressTimersUntilNextEditingCommandQuirk:
+  initial: false
+
 treatsAnyTextCSSLinkAsStylesheet:
   initial: false
 shrinksStandaloneImagesToFit:
diff --git a/Source/WebCore/page/ios/DOMTimerHoldingTank.cpp b/Source/WebCore/page/ios/DOMTimerHoldingTank.cpp
new file mode 100644 (file)
index 0000000..3883c3b
--- /dev/null
@@ -0,0 +1,82 @@
+/*
+ * Copyright (C) 2019 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "DOMTimerHoldingTank.h"
+
+#if PLATFORM(IOS_FAMILY)
+
+#include <wtf/HashSet.h>
+
+namespace WebCore {
+
+#if PLATFORM(IOS_SIMULATOR)
+constexpr Seconds maximumHoldTimeLimit { 50_ms };
+#else
+constexpr Seconds maximumHoldTimeLimit { 32_ms };
+#endif
+
+bool DeferDOMTimersForScope::s_isDeferring { false };
+
+DOMTimerHoldingTank::DOMTimerHoldingTank()
+    : m_exceededMaximumHoldTimer { *this, &DOMTimerHoldingTank::removeAll }
+{
+}
+
+DOMTimerHoldingTank::~DOMTimerHoldingTank() = default;
+
+void DOMTimerHoldingTank::add(const DOMTimer& timer)
+{
+    m_timers.add(&timer);
+    if (!m_exceededMaximumHoldTimer.isActive())
+        m_exceededMaximumHoldTimer.startOneShot(maximumHoldTimeLimit);
+}
+
+void DOMTimerHoldingTank::remove(const DOMTimer& timer)
+{
+    stopExceededMaximumHoldTimer();
+    m_timers.remove(&timer);
+}
+
+bool DOMTimerHoldingTank::contains(const DOMTimer& timer)
+{
+    return m_timers.contains(&timer);
+}
+
+void DOMTimerHoldingTank::removeAll()
+{
+    stopExceededMaximumHoldTimer();
+    m_timers.clear();
+}
+
+inline void DOMTimerHoldingTank::stopExceededMaximumHoldTimer()
+{
+    if (m_exceededMaximumHoldTimer.isActive())
+        m_exceededMaximumHoldTimer.stop();
+}
+
+} // namespace WebCore
+
+#endif // PLATFORM(IOS_FAMILY)
diff --git a/Source/WebCore/page/ios/DOMTimerHoldingTank.h b/Source/WebCore/page/ios/DOMTimerHoldingTank.h
new file mode 100644 (file)
index 0000000..60277c7
--- /dev/null
@@ -0,0 +1,74 @@
+/*
+ * Copyright (C) 2019 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#pragma once
+
+#if PLATFORM(IOS_FAMILY)
+
+#include "Timer.h"
+#include <wtf/Forward.h>
+
+namespace WebCore {
+
+class DOMTimer;
+
+class DOMTimerHoldingTank {
+public:
+    DOMTimerHoldingTank();
+    ~DOMTimerHoldingTank();
+
+    void add(const DOMTimer&);
+    void remove(const DOMTimer&);
+    bool contains(const DOMTimer&);
+    WEBCORE_EXPORT void removeAll();
+
+private:
+    void stopExceededMaximumHoldTimer();
+
+    HashSet<const DOMTimer*> m_timers;
+    Timer m_exceededMaximumHoldTimer;
+};
+
+class DeferDOMTimersForScope {
+public:
+    DeferDOMTimersForScope(bool enable)
+        : m_previousIsDeferring { s_isDeferring }
+    {
+        if (enable)
+            s_isDeferring = true;
+    }
+
+    ~DeferDOMTimersForScope() { s_isDeferring = m_previousIsDeferring; }
+
+    static bool isDeferring() { return s_isDeferring; }
+
+private:
+    bool m_previousIsDeferring;
+    static bool s_isDeferring;
+};
+
+} // namespace WebCore
+
+#endif // PLATFORM(IOS_FAMILY)
index d665feb..327365b 100644 (file)
@@ -1,3 +1,28 @@
+2019-07-17  Daniel Bates  <dabates@apple.com>
+
+        Typing into a cell in a Google Sheet lags behind by one character
+        https://bugs.webkit.org/show_bug.cgi?id=199587
+        <rdar://problem/51616845>
+
+        Reviewed by Brent Fulgham.
+
+        Remove all timers from the holding tank on text insertion or deletion (represented as an
+        editing command). Timers that were in the holding tank never stopped ticking and will now
+        be able to execute their action.
+
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::executeEditingCommand):
+        (WebKit::WebPage::insertTextAsync):
+        (WebKit::WebPage::setCompositionAsync):
+        (WebKit::WebPage::confirmCompositionAsync):
+        Call platformWillPerformEditingCommand().
+
+        * WebProcess/WebPage/WebPage.h:
+        (WebKit::WebPage::platformWillPerformEditingCommand): Added.
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::platformWillPerformEditingCommand): Remove all the timers from the holding
+        tank if we have a holding tank.
+
 2019-07-17  Darin Adler  <darin@apple.com>
 
         No need for isURLAllowed function in Frame
index 1005db5..10944f7 100644 (file)
@@ -1260,7 +1260,9 @@ PluginView* WebPage::pluginViewForFrame(Frame* frame)
 
 void WebPage::executeEditingCommand(const String& commandName, const String& argument)
 {
-    Frame& frame = m_page->focusController().focusedOrMainFrame();
+    platformWillPerformEditingCommand();
+
+    auto& frame = m_page->focusController().focusedOrMainFrame();
 
     if (PluginView* pluginView = focusedPluginViewForFrame(frame)) {
         pluginView->handleEditingCommand(commandName, argument);
@@ -5175,7 +5177,9 @@ void WebPage::setTextAsync(const String& text)
 
 void WebPage::insertTextAsync(const String& text, const EditingRange& replacementEditingRange, InsertTextOptions&& options)
 {
-    Frame& frame = m_page->focusController().focusedOrMainFrame();
+    platformWillPerformEditingCommand();
+
+    auto& frame = m_page->focusController().focusedOrMainFrame();
 
     Ref<Frame> protector(frame);
 
@@ -5249,7 +5253,9 @@ void WebPage::firstRectForCharacterRangeAsync(const EditingRange& editingRange,
 
 void WebPage::setCompositionAsync(const String& text, const Vector<CompositionUnderline>& underlines, const EditingRange& selection, const EditingRange& replacementEditingRange)
 {
-    Frame& frame = m_page->focusController().focusedOrMainFrame();
+    platformWillPerformEditingCommand();
+
+    auto& frame = m_page->focusController().focusedOrMainFrame();
 
     if (frame.selection().selection().isContentEditable()) {
         RefPtr<Range> replacementRange;
@@ -5265,6 +5271,8 @@ void WebPage::setCompositionAsync(const String& text, const Vector<CompositionUn
 
 void WebPage::confirmCompositionAsync()
 {
+    platformWillPerformEditingCommand();
+
     Frame& frame = m_page->focusController().focusedOrMainFrame();
     frame.editor().confirmComposition();
 }
index eb9728c..b15d6b8 100644 (file)
@@ -1225,6 +1225,7 @@ private:
     void platformReinitialize();
     void platformDetach();
     void platformEditorState(WebCore::Frame&, EditorState& result, IncludePostLayoutDataHint) const;
+    void platformWillPerformEditingCommand();
     void sendEditorStateUpdate();
 
 #if PLATFORM(COCOA)
@@ -1957,5 +1958,9 @@ private:
 #endif
 };
 
+#if !PLATFORM(IOS_FAMILY)
+inline void WebPage::platformWillPerformEditingCommand() { }
+#endif
+
 } // namespace WebKit
 
index 52ae1f7..c59a37b 100644 (file)
@@ -63,6 +63,7 @@
 #import <WebCore/AutofillElements.h>
 #import <WebCore/Chrome.h>
 #import <WebCore/ContentChangeObserver.h>
+#import <WebCore/DOMTimerHoldingTank.h>
 #import <WebCore/DataDetection.h>
 #import <WebCore/DiagnosticLoggingClient.h>
 #import <WebCore/DiagnosticLoggingKeys.h>
@@ -275,6 +276,15 @@ void WebPage::platformEditorState(Frame& frame, EditorState& result, IncludePost
     }
 }
 
+void WebPage::platformWillPerformEditingCommand()
+{
+    auto& frame = m_page->focusController().focusedOrMainFrame();
+    if (auto* document = frame.document()) {
+        if (auto* holdingTank = document->domTimerHoldingTankIfExists())
+            holdingTank->removeAll();
+    }
+}
+
 FloatSize WebPage::screenSize() const
 {
     return m_screenSize;