Attachment 356247 Details for Bug 192236 – Patch

[patch] Patch

bug-192236-20181130153015.patch (text/plain), 12.44 KB, created by zalan on 2018-11-30 15:30:16 PST

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-11-30 15:30:16 PST

Size: 12.44 KB

patch

obsolete

>Subversion Revision: 238755
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 36d909e828360789af1d7a15fb272d57224c25ef..a7a6dcb0dc278403d116aeb93c837a2a38fe5100 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,35 @@
>+2018-11-30  Zalan Bujtas  <zalan@apple.com>
>+
>+        Canât use RalphLauren.com on iPad because hover menus donât stay up
>+        https://bugs.webkit.org/show_bug.cgi?id=192236
>+        <rdar://problem/45792118>
>+
>+        Reviewed by Geoffrey Garen.
>+
>+        This patch introduces asynchronous content change observation.
>+        1. Start observing synchronous content change and timer install as the result of dispatching mouseMoved event.
>+        2. Start observing synchronous content change and style recalc schedule as the result of a timer callback (installed at #1).
>+        3. Start observing synchronous content change as the result of a style recalc (scheduled at #2).
>+
>+        This patch also extends the timeout value from 100ms to 250ms. Certain content prefer longer timeouts (see http://briancherne.github.io/jquery-hoverIntent/ for details).  
>+
>+        Test: fast/events/touch/ios/hover-when-style-change-is-async.html
>+
>+        * dom/Document.cpp:
>+        (WebCore::Document::scheduleStyleRecalc):
>+        (WebCore::Document::updateStyleIfNeeded):
>+        * page/DOMTimer.cpp:
>+        (WebCore::DOMTimer::install):
>+        (WebCore::DOMTimer::fired):
>+        * platform/ios/wak/WKContentObservation.cpp:
>+        (WKStartObservingStyleRecalcScheduling):
>+        (WKStopObservingStyleRecalcScheduling):
>+        (WKIsObservingStyleRecalcScheduling):
>+        (WKSetShouldObserveNextStyleRecalc):
>+        (WKShouldObserveNextStyleRecalc):
>+        (WKSetObservedContentChange):
>+        * platform/ios/wak/WKContentObservation.h:
>+
> 2018-11-30  Chris Dumez  <cdumez@apple.com>
> 
>         [PSON] We are sometimes swapping processes even though there is an opened window with an opener link to us
>diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
>index 92877e410f1bab3e56bda536998038da27714f3a..1548696a682bce9ebc6573d6f6d435f9eea571fe 100644
>--- a/Source/WebCore/dom/Document.cpp
>+++ b/Source/WebCore/dom/Document.cpp
>@@ -264,6 +264,7 @@
> #include "Navigator.h"
> #include "NavigatorGeolocation.h"
> #include "WKContentObservation.h"
>+#include "WKContentObservationInternal.h"
> #endif
> 
> #if ENABLE(IOS_GESTURE_EVENTS)
>@@ -1794,6 +1795,11 @@ void Document::scheduleStyleRecalc()
> 
>     ASSERT(childNeedsStyleRecalc() || m_pendingStyleRecalcShouldForce);
> 
>+#if PLATFORM(IOS_FAMILY)
>+    if (WKIsObservingStyleRecalcScheduling())
>+        WKSetObservedContentChange(WKContentIndeterminateChange);
>+#endif
>+
>     // FIXME: Why on earth is this here? This is clearly misplaced.
>     invalidateAccessKeyMap();
> 
>@@ -2028,10 +2034,29 @@ bool Document::updateStyleIfNeeded()
>             return false;
>     }
> 
>+#if PLATFORM(IOS_FAMILY)
>+    auto observingContentChange = WKShouldObserveNextStyleRecalc();
>+    if (observingContentChange) {
>+        WKSetShouldObserveNextStyleRecalc(false);
>+        WKStartObservingContentChanges();
>+    }
>+#endif
>     // The early exit above for !needsStyleRecalc() is needed when updateWidgetPositions() is called in runOrScheduleAsynchronousTasks().
>     RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(isSafeToUpdateStyleOrLayout(*this));
> 
>     resolveStyle();
>+
>+#if PLATFORM(IOS_FAMILY)
>+    if (observingContentChange) {
>+        WKStopObservingContentChanges();
>+
>+        auto inDeterminedState = WKObservedContentChange() == WKContentVisibilityChange || !WebThreadCountOfObservedDOMTimers();  
>+        if (inDeterminedState) {
>+            if (auto* page = this->page())
>+                page->chrome().client().observedContentChange(*frame());
>+        }
>+    }
>+#endif
>     return true;
> }
> 
>diff --git a/Source/WebCore/page/DOMTimer.cpp b/Source/WebCore/page/DOMTimer.cpp
>index b5d6775e5f062c3b246d9bdc00cd7e624670efeb..73ae60ada960e530721673dd9e6f71b67ff87baf 100644
>--- a/Source/WebCore/page/DOMTimer.cpp
>+++ b/Source/WebCore/page/DOMTimer.cpp
>@@ -222,7 +222,7 @@ int DOMTimer::install(ScriptExecutionContext& context, std::unique_ptr<Scheduled
> #if PLATFORM(IOS_FAMILY)
>     if (WKIsObservingDOMTimerScheduling() && is<Document>(context)) {
>         bool didDeferTimeout = context.activeDOMObjectsAreSuspended();
>-        if (!didDeferTimeout && timeout <= 100_ms && singleShot) {
>+        if (!didDeferTimeout && timeout <= 250_ms && singleShot) {
>             WKSetObservedContentChange(WKContentIndeterminateChange);
>             WebThreadAddObservedDOMTimer(timer);
>         }
>@@ -341,18 +341,16 @@ void DOMTimer::fired()
>     context.removeTimeout(m_timeoutId);
> 
> #if PLATFORM(IOS_FAMILY)
>-    bool shouldReportLackOfChanges;
>-    bool shouldBeginObservingChanges;
>+    auto isObversingLastTimer = false;
>+    auto shouldBeginObservingChanges = false;
>     if (is<Document>(context)) {
>-        shouldReportLackOfChanges = WebThreadCountOfObservedDOMTimers() == 1;
>+        isObversingLastTimer = WebThreadCountOfObservedDOMTimers() == 1;
>         shouldBeginObservingChanges = WebThreadContainsObservedDOMTimer(this);
>-    } else {
>-        shouldReportLackOfChanges = false;
>-        shouldBeginObservingChanges = false;
>     }
> 
>     if (shouldBeginObservingChanges) {
>         WKStartObservingContentChanges();
>+        WKStartObservingStyleRecalcScheduling();
>         WebThreadRemoveObservedDOMTimer(this);
>     }
> #endif
>@@ -366,12 +364,19 @@ void DOMTimer::fired()
> 
> #if PLATFORM(IOS_FAMILY)
>     if (shouldBeginObservingChanges) {
>+        WKStopObservingStyleRecalcScheduling();
>         WKStopObservingContentChanges();
> 
>-        if (WKObservedContentChange() == WKContentVisibilityChange || shouldReportLackOfChanges) {
>-            Document& document = downcast<Document>(context);
>-            if (Page* page = document.page())
>+        auto observedContentChange = WKObservedContentChange();
>+        // Check if the timer callback triggered either a sync or async style update.
>+        auto inDeterminedState = observedContentChange == WKContentVisibilityChange || (isObversingLastTimer && observedContentChange == WKContentNoChange);  
>+        if (inDeterminedState) {
>+            auto& document = downcast<Document>(context);
>+            if (auto* page = document.page())
>                 page->chrome().client().observedContentChange(*document.frame());
>+        } else if (observedContentChange == WKContentIndeterminateChange) {
>+            // An async style recalc has been scheduled. Let's observe it.
>+            WKSetShouldObserveNextStyleRecalc(true);
>         }
>     }
> #endif
>diff --git a/Source/WebCore/platform/ios/wak/WKContentObservation.cpp b/Source/WebCore/platform/ios/wak/WKContentObservation.cpp
>index 8cfa0ec87ccabae1db08b8ba67b9bf292d5a51b5..d78f91b7a1388b01c3b3e27d1b86db72059cfdc7 100644
>--- a/Source/WebCore/platform/ios/wak/WKContentObservation.cpp
>+++ b/Source/WebCore/platform/ios/wak/WKContentObservation.cpp
>@@ -37,7 +37,8 @@
> WKContentChange _WKContentChange                    = WKContentNoChange;
> bool            _WKObservingContentChanges          = false;
> bool            _WKObservingDOMTimerScheduling      = false;
>-
>+bool            _WKObservingStyleRecalScheduling    = false;
>+bool            _WKObservingNextStyleRecalc         = false;
> 
> bool WKObservingContentChanges(void)
> {
>@@ -71,6 +72,31 @@ bool WKIsObservingDOMTimerScheduling(void)
>     return _WKObservingDOMTimerScheduling;
> }
> 
>+void WKStartObservingStyleRecalcScheduling(void)
>+{
>+    _WKObservingStyleRecalScheduling = true;
>+}
>+
>+void WKStopObservingStyleRecalcScheduling(void)
>+{
>+    _WKObservingStyleRecalScheduling = false;
>+}
>+
>+bool WKIsObservingStyleRecalcScheduling(void)
>+{
>+    return _WKObservingStyleRecalScheduling;
>+}
>+
>+void WKSetShouldObserveNextStyleRecalc(bool observe)
>+{
>+    _WKObservingNextStyleRecalc = observe;
>+}
>+
>+bool WKShouldObserveNextStyleRecalc(void)
>+{
>+    return _WKObservingNextStyleRecalc;
>+}
>+
> WKContentChange WKObservedContentChange(void)
> {
>     return _WKContentChange;
>@@ -84,8 +110,9 @@ void WKSetObservedContentChange(WKContentChange change)
> 
>     if (change == WKContentVisibilityChange) {
>         _WKContentChange = change;
>-        // Don't need to listen to DOM timers anymore.
>+        // Don't need to listen to DOM timers/style recalcs anymore.
>         WebThreadClearObservedDOMTimers();
>+        _WKObservingNextStyleRecalc = false;
>         return;
>     }
> 
>diff --git a/Source/WebCore/platform/ios/wak/WKContentObservation.h b/Source/WebCore/platform/ios/wak/WKContentObservation.h
>index 1f36347d323717197a733838afe809d85f26e3e9..af5a3bc662c2e1b4777d451b9c5d53adc31b7c03 100644
>--- a/Source/WebCore/platform/ios/wak/WKContentObservation.h
>+++ b/Source/WebCore/platform/ios/wak/WKContentObservation.h
>@@ -46,6 +46,13 @@ WEBCORE_EXPORT void WKStartObservingDOMTimerScheduling(void);
> WEBCORE_EXPORT void WKStopObservingDOMTimerScheduling(void);
> WEBCORE_EXPORT bool WKIsObservingDOMTimerScheduling(void);
> 
>+WEBCORE_EXPORT void WKStartObservingStyleRecalcScheduling(void);
>+WEBCORE_EXPORT void WKStopObservingStyleRecalcScheduling(void);
>+WEBCORE_EXPORT bool WKIsObservingStyleRecalcScheduling(void);
>+
>+WEBCORE_EXPORT void WKSetShouldObserveNextStyleRecalc(bool);
>+WEBCORE_EXPORT bool WKShouldObserveNextStyleRecalc(void);
>+
> WEBCORE_EXPORT WKContentChange WKObservedContentChange(void);
> 
> WEBCORE_EXPORT int WebThreadCountOfObservedDOMTimers(void);
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index a34df2f0716fbf4735ed917cc9c639a88b1ce4af..fafae4f59370d858d9c432ee4e753ac6fb77246a 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-11-30  Zalan Bujtas  <zalan@apple.com>
>+
>+        Canât use RalphLauren.com on iPad because hover menus donât stay up
>+        https://bugs.webkit.org/show_bug.cgi?id=192236
>+        <rdar://problem/45792118>
>+
>+        Reviewed by Geoffrey Garen.
>+
>+        * fast/events/touch/ios/hover-when-style-change-is-async-expected.txt: Added.
>+        * fast/events/touch/ios/hover-when-style-change-is-async.html: Added.
>+
> 2018-11-30  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         Replace "auto fill" with "AutoFill" in some localizable strings
>diff --git a/LayoutTests/fast/events/touch/ios/hover-when-style-change-is-async-expected.txt b/LayoutTests/fast/events/touch/ios/hover-when-style-change-is-async-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..6ce685360feb975be40d70bcc7686ea10609de9a
>--- /dev/null
>+++ b/LayoutTests/fast/events/touch/ios/hover-when-style-change-is-async-expected.txt
>@@ -0,0 +1,2 @@
>+PASS if no 'clicked' text is shown below.
>+
>diff --git a/LayoutTests/fast/events/touch/ios/hover-when-style-change-is-async.html b/LayoutTests/fast/events/touch/ios/hover-when-style-change-is-async.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..5d06861f6994c83cd029fd28469052213403ec36
>--- /dev/null
>+++ b/LayoutTests/fast/events/touch/ios/hover-when-style-change-is-async.html
>@@ -0,0 +1,63 @@
>+<html>
>+<head>
>+<title>This test that we trigger hover when the content change is async.</title>
>+<script src="../../../../resources/basic-gestures.js"></script>
>+<style>
>+#tapthis {
>+    width: 400px;
>+    height: 400px;
>+    border: 1px solid green;
>+}
>+
>+#hiddenFirst {
>+    visibility: hidden;
>+    width: 100px;
>+    height: 100px;
>+    background-color: green;
>+}
>+</style>
>+<script>
>+async function test() {
>+    if (!window.testRunner || !testRunner.runUIScript)
>+        return;
>+
>+    testRunner.dumpAsText();
>+    testRunner.waitUntilDone();
>+
>+    let rect = tapthis.getBoundingClientRect();
>+    let x = rect.left + rect.width / 2;
>+    let y = rect.top + rect.height / 2;
>+
>+    await tapAtPoint(x, y);
>+}
>+</script>
>+</head>
>+<body onload="test()">
>+<div id=tapthis>PASS if no 'clicked' text is shown below.</div>
>+<div id=hiddenFirst></div>
>+<pre id=result></pre>
>+<script>
>+tapthis.addEventListener("mouseover", function( event ) {
>+    // 1. Install a timer on hover
>+    setTimeout(function() {
>+        // 2. Trigger a non-forcing style change
>+        hiddenFirst.style.visibility = "visible";
>+        // 3. Install a timer for style recalc
>+        setTimeout(function() {
>+            document.body.offsetHeight;
>+            if (window.testRunner)
>+                testRunner.notifyDone();
>+        }, 10);
>+    }, 0);
>+}, false);
>+
>+hiddenFirst.addEventListener("click", function( event ) {   
>+    result.innerHTML = "clicked";
>+}, false);
>+
>+tapthis.addEventListener("click", function( event ) {   
>+    result.innerHTML = "clicked";
>+}, false);
>+</script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 192236: 356212 | 356247