[ContentChangeObserver] Stop using the global _WKContentChange
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 27 Mar 2019 19:05:27 +0000 (19:05 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 27 Mar 2019 19:05:27 +0000 (19:05 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196288
<rdar://problem/49228081>

Reviewed by Simon Fraser.

Source/WebCore:

This patch ensures that activities on frames don't overwrite the observed state on other frames.
(Unfortunately the global variable is still used in WebKitLegacy (see webkit.org/b/196286)).

Tests: fast/events/touch/ios/content-observation/remove-subframe-while-observing.html
       fast/events/touch/ios/content-observation/subframe.html

* page/ios/ContentChangeObserver.cpp:
(WebCore::ContentChangeObserver::observedContentChange const): Deleted.
* page/ios/ContentChangeObserver.h:
(WebCore::ContentChangeObserver::observedContentChange const):
(WebCore::ContentChangeObserver::setHasNoChangeState):
(WebCore::ContentChangeObserver::setHasIndeterminateState):
(WebCore::ContentChangeObserver::setHasVisibleChangeState):
(WebCore::ContentChangeObserver::setObservedContentState):

LayoutTests:

* fast/events/touch/ios/content-observation/remove-subframe-while-observing-expected.txt: Added.
* fast/events/touch/ios/content-observation/remove-subframe-while-observing.html: Added.
* fast/events/touch/ios/content-observation/subframe.html: Added.
* platform/ios-device-wk1/TestExpectations:
* platform/ios-simulator-wk1/TestExpectations:

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

LayoutTests/ChangeLog
LayoutTests/fast/events/touch/ios/content-observation/remove-subframe-while-observing-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/touch/ios/content-observation/remove-subframe-while-observing.html [new file with mode: 0644]
LayoutTests/fast/events/touch/ios/content-observation/subframe.html [new file with mode: 0644]
LayoutTests/platform/ios-device-wk1/TestExpectations
LayoutTests/platform/ios-simulator-wk1/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/page/ios/ContentChangeObserver.cpp
Source/WebCore/page/ios/ContentChangeObserver.h

index 3271821..020cb4a 100644 (file)
@@ -1,3 +1,17 @@
+2019-03-27  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] Stop using the global _WKContentChange
+        https://bugs.webkit.org/show_bug.cgi?id=196288
+        <rdar://problem/49228081>
+
+        Reviewed by Simon Fraser.
+
+        * fast/events/touch/ios/content-observation/remove-subframe-while-observing-expected.txt: Added.
+        * fast/events/touch/ios/content-observation/remove-subframe-while-observing.html: Added.
+        * fast/events/touch/ios/content-observation/subframe.html: Added.
+        * platform/ios-device-wk1/TestExpectations:
+        * platform/ios-simulator-wk1/TestExpectations:
+
 2019-03-27  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         Remove the SVG tear off objects for SVGPathSeg, SVGPathSegList and SVGAnimatedPathSegList
diff --git a/LayoutTests/fast/events/touch/ios/content-observation/remove-subframe-while-observing-expected.txt b/LayoutTests/fast/events/touch/ios/content-observation/remove-subframe-while-observing-expected.txt
new file mode 100644 (file)
index 0000000..66db4c9
--- /dev/null
@@ -0,0 +1,2 @@
+PASS if 'clicked' text is not shown below.
+
diff --git a/LayoutTests/fast/events/touch/ios/content-observation/remove-subframe-while-observing.html b/LayoutTests/fast/events/touch/ios/content-observation/remove-subframe-while-observing.html
new file mode 100644 (file)
index 0000000..d2e5b6c
--- /dev/null
@@ -0,0 +1,63 @@
+<html>
+<head>
+<title>This tests the case when visible content change happens on the main frame while the subframe is removed</title>
+<script src="../../../../../resources/basic-gestures.js"></script>
+<style>
+#tapthis {
+    width: 400px;
+    height: 400px;
+    border: 1px solid green;
+}
+
+#becomesVisible {
+    display: none;
+    width: 100px;
+    height: 100px;
+    background-color: green;
+}
+</style>
+<script>
+async function test() {
+    if (!window.testRunner || !testRunner.runUIScript)
+        return;
+    if (window.internals)
+        internals.settings.setContentChangeObserverEnabled(true);
+
+    testRunner.waitUntilDone();
+    testRunner.dumpAsText();
+
+    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 'clicked' text is not shown below.</div>
+<div id=becomesVisible></div>
+<iframe id=removeThis src=subframe.html></iframe>
+<pre id=result></pre>
+<script>
+tapthis.addEventListener("mousemove", function( event ) {
+    becomesVisible.style.display = "block";
+    document.body.offsetHeight;
+
+    let iframeToRemove = document.getElementById("removeThis");
+    if (iframeToRemove)
+        iframeToRemove.remove();
+    if (window.testRunner)
+        setTimeout("testRunner.notifyDone()", 250);
+}, false);
+
+becomesVisible.addEventListener("click", function( event ) {   
+    result.innerHTML = "clicked hidden";
+}, false);
+
+tapthis.addEventListener("click", function( event ) {   
+    result.innerHTML = "clicked";
+}, false);
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/events/touch/ios/content-observation/subframe.html b/LayoutTests/fast/events/touch/ios/content-observation/subframe.html
new file mode 100644 (file)
index 0000000..db6cda7
--- /dev/null
@@ -0,0 +1 @@
+subframe content
\ No newline at end of file
index c269f6f..fd0ea63 100644 (file)
@@ -3,3 +3,4 @@
 # See http://trac.webkit.org/wiki/TestExpectations for more information on this file.
 #
 
+webkit.org/b/196286 fast/events/touch/ios/content-observation/remove-subframe-while-observing.html [ Failure ]
index 968bf26..58110fc 100644 (file)
@@ -14,3 +14,5 @@ editing/pasteboard/data-transfer-get-data-non-normalized-types.html [ Pass ]
 editing/pasteboard/data-transfer-item-list-add-file-on-copy.html [ Pass ]
 editing/pasteboard/data-transfer-item-list-add-file-multiple-times.html [ Pass ]
 editing/pasteboard/data-transfer-items-add-custom-data.html [ Pass ]
+
+webkit.org/b/196286 fast/events/touch/ios/content-observation/remove-subframe-while-observing.html [ Failure ]
\ No newline at end of file
index 0c7020c..da9e4bb 100644 (file)
@@ -1,3 +1,26 @@
+2019-03-27  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] Stop using the global _WKContentChange
+        https://bugs.webkit.org/show_bug.cgi?id=196288
+        <rdar://problem/49228081>
+
+        Reviewed by Simon Fraser.
+
+        This patch ensures that activities on frames don't overwrite the observed state on other frames.  
+        (Unfortunately the global variable is still used in WebKitLegacy (see webkit.org/b/196286)).
+
+        Tests: fast/events/touch/ios/content-observation/remove-subframe-while-observing.html
+               fast/events/touch/ios/content-observation/subframe.html
+
+        * page/ios/ContentChangeObserver.cpp:
+        (WebCore::ContentChangeObserver::observedContentChange const): Deleted.
+        * page/ios/ContentChangeObserver.h:
+        (WebCore::ContentChangeObserver::observedContentChange const):
+        (WebCore::ContentChangeObserver::setHasNoChangeState):
+        (WebCore::ContentChangeObserver::setHasIndeterminateState):
+        (WebCore::ContentChangeObserver::setHasVisibleChangeState):
+        (WebCore::ContentChangeObserver::setObservedContentState):
+
 2019-03-27  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         Remove the SVG tear off objects for SVGPathSeg, SVGPathSegList and SVGAnimatedPathSegList
index 1cddbcd..0304029 100644 (file)
@@ -334,11 +334,6 @@ void ContentChangeObserver::mouseMovedDidFinish()
     m_mouseMovedEventIsBeingDispatched = false;
 }
 
-WKContentChange ContentChangeObserver::observedContentChange() const
-{
-    return WKObservedContentChange();
-}
-
 void ContentChangeObserver::setShouldObserveNextStyleRecalc(bool shouldObserve)
 {
     if (shouldObserve)
index e87db7a..d6a6920 100644 (file)
@@ -45,7 +45,7 @@ public:
     ContentChangeObserver(Document&);
 
     WEBCORE_EXPORT void startContentObservationForDuration(Seconds duration);
-    WEBCORE_EXPORT WKContentChange observedContentChange() const;
+    WKContentChange observedContentChange() const { return m_observedContentState; }
 
     void didInstallDOMTimer(const DOMTimer&, Seconds timeout, bool singleShot);
     void didRemoveDOMTimer(const DOMTimer&);
@@ -142,9 +142,9 @@ private:
     void stopObservingPendingActivities();
     void reset();
 
-    void setHasIndeterminateState();
-    void setHasVisibleChangeState();
-    void setHasNoChangeState();
+    void setHasNoChangeState() { setObservedContentState(WKContentNoChange); }
+    void setHasIndeterminateState() { setObservedContentState(WKContentIndeterminateChange); }
+    void setHasVisibleChangeState() { setObservedContentState(WKContentVisibilityChange); } 
 
     bool hasVisibleChangeState() const { return observedContentChange() == WKContentVisibilityChange; }
     bool hasObservedDOMTimer() const { return !m_DOMTimerList.isEmpty(); }
@@ -158,6 +158,7 @@ private:
     bool isObservationTimeWindowActive() const { return m_contentObservationTimer.isActive(); }
 
     void completeDurationBasedContentObservation();
+    void setObservedContentState(WKContentChange);
 
     enum class Event {
         StartedTouchStartEventDispatching,
@@ -186,6 +187,7 @@ private:
     HashSet<const DOMTimer*> m_DOMTimerList;
     // FIXME: Move over to WeakHashSet when it starts supporting const.
     HashSet<const Element*> m_elementsWithTransition;
+    WKContentChange m_observedContentState { WKContentNoChange };
     bool m_touchEventIsBeingDispatched { false };
     bool m_isWaitingForStyleRecalc { false };
     bool m_isInObservedStyleRecalc { false };
@@ -196,20 +198,10 @@ private:
     bool m_isObservingTransitions { false };
 };
 
-inline void ContentChangeObserver::setHasNoChangeState()
+inline void ContentChangeObserver::setObservedContentState(WKContentChange observedContentChange)
 {
-    WKSetObservedContentChange(WKContentNoChange);
-}
-
-inline void ContentChangeObserver::setHasIndeterminateState()
-{
-    ASSERT(!hasVisibleChangeState());
-    WKSetObservedContentChange(WKContentIndeterminateChange);
-}
-
-inline void ContentChangeObserver::setHasVisibleChangeState()
-{
-    WKSetObservedContentChange(WKContentVisibilityChange);
+    m_observedContentState = observedContentChange;
+    WKSetObservedContentChange(observedContentChange);
 }
 
 inline bool ContentChangeObserver::isObservingContentChanges() const