Attachment 356212 Details for Bug 192236 – Patch

[patch] Patch

bug-192236-20181130115143.patch (text/plain), 12.37 KB, created by zalan on 2018-11-30 11:51:43 PST

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-11-30 11:51:43 PST

Size: 12.37 KB

patch

obsolete

>Subversion Revision: 238737
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index f858dd480f493797eb370f7901a2729601fdc83f..b42e4979d7dab71ec5174ccbf30b0937a9f9ab01 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 NOBODY (OOPS!).
>+
>+        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  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][BFC] Compute min/maxHeight margins only when they are needed.
>diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
>index ab3870e8d3496de59ab72fb0be86afbe68f2b4d5..1408a00a341c64f770886b843a71cbec9b48efff 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 99bbb14cfa354474e50980d30b728b20a0dc2b3e..dc9c604a136f60f249b43893b9e87bb58cde9c20 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 NOBODY (OOPS!).
>+
>+        * 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  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][BFC] Compute min/maxHeight margins only when they are needed.
>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..1ec284230159f3747993610850a420420493b20b
>--- /dev/null
>+++ b/LayoutTests/fast/events/touch/ios/hover-when-style-change-is-async-expected.txt
>@@ -0,0 +1,2 @@
>+PASS if NOT clicked.
>+
>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..176694ef469345561d6cc00c2a77a1ecd591ba8c
>--- /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 NOT clicked.</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, NOT PASS";
>+}, false);
>+
>+tapthis.addEventListener("click", function( event ) {   
>+    result.innerHTML = "clicked, NOT PASS";
>+}, false);
>+</script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 192236: 356212 | 356247