[ContentChangeObserver] Any previously destroyed renderer should not be considered...
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Aug 2019 23:53:14 +0000 (23:53 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 14 Aug 2019 23:53:14 +0000 (23:53 +0000)
https://bugs.webkit.org/show_bug.cgi?id=200732
<rdar://problem/54319654>

Reviewed by Simon Fraser.

Source/WebCore:

A visible element should not be considered a candidate to content change observation when it loses the visiblity status momentarily.
This patch extends the check of re-constructed renderers for the duration of the content change observation (as opposed to just a single render tree update cycle)

Test: fast/events/touch/ios/content-observation/visible-content-gains-new-renderer2.html

* page/ios/ContentChangeObserver.cpp:
(WebCore::ContentChangeObserver::reset):
(WebCore::ContentChangeObserver::rendererWillBeDestroyed):
(WebCore::ContentChangeObserver::adjustObservedState):
(WebCore::ContentChangeObserver::renderTreeUpdateDidStart): Deleted.
(WebCore::ContentChangeObserver::renderTreeUpdateDidFinish): Deleted.
(WebCore::ContentChangeObserver::stopContentObservation): Deleted.
(WebCore::ContentChangeObserver::willDestroyRenderer): Deleted.
(WebCore::ContentChangeObserver::RenderTreeUpdateScope::RenderTreeUpdateScope): Deleted.
(WebCore::ContentChangeObserver::RenderTreeUpdateScope::~RenderTreeUpdateScope): Deleted.
* page/ios/ContentChangeObserver.h:
(WebCore::ContentChangeObserver::isObservingContentChanges const):
* rendering/RenderElement.cpp:
(WebCore::RenderElement::willBeDestroyed):
* rendering/updating/RenderTreeUpdater.cpp:
(WebCore::RenderTreeUpdater::updateRenderTree):
(WebCore::RenderTreeUpdater::tearDownRenderers):

LayoutTests:

* fast/events/touch/ios/content-observation/visible-content-gains-new-renderer2-expected.txt: Added.
* fast/events/touch/ios/content-observation/visible-content-gains-new-renderer2.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/events/touch/ios/content-observation/visible-content-gains-new-renderer2-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/touch/ios/content-observation/visible-content-gains-new-renderer2.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/ios/ContentChangeObserver.cpp
Source/WebCore/page/ios/ContentChangeObserver.h
Source/WebCore/rendering/RenderElement.cpp
Source/WebCore/rendering/updating/RenderTreeUpdater.cpp

index 10a5422..b99dcdd 100644 (file)
@@ -1,3 +1,14 @@
+2019-08-14  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] Any previously destroyed renderer should not be considered a candidate for content observation.
+        https://bugs.webkit.org/show_bug.cgi?id=200732
+        <rdar://problem/54319654>
+
+        Reviewed by Simon Fraser.
+
+        * fast/events/touch/ios/content-observation/visible-content-gains-new-renderer2-expected.txt: Added.
+        * fast/events/touch/ios/content-observation/visible-content-gains-new-renderer2.html: Added.
+
 2019-08-14  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r248638.
diff --git a/LayoutTests/fast/events/touch/ios/content-observation/visible-content-gains-new-renderer2-expected.txt b/LayoutTests/fast/events/touch/ios/content-observation/visible-content-gains-new-renderer2-expected.txt
new file mode 100644 (file)
index 0000000..677e3a3
--- /dev/null
@@ -0,0 +1,2 @@
+PASS if 'clicked' text is shown below.
+clicked
diff --git a/LayoutTests/fast/events/touch/ios/content-observation/visible-content-gains-new-renderer2.html b/LayoutTests/fast/events/touch/ios/content-observation/visible-content-gains-new-renderer2.html
new file mode 100644 (file)
index 0000000..5efd2ca
--- /dev/null
@@ -0,0 +1,58 @@
+<!DOCTYPE html><!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<title>This tests the case when visible content gets removed and later added back and should not be considered as new content</title>
+<script src="../../../../../resources/ui-helper.js"></script>
+<style>
+#tapThis {
+    width: 400px;
+    height: 400px;
+    border: 1px solid green;
+}
+
+#alreadyVisible {
+    display: block;
+    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();
+
+    await UIHelper.activateElement(tapThis);
+}
+</script>
+</head>
+<body onload="test()">
+<div id=tapThis>PASS if 'clicked' text is shown below.</div>
+<div id=alreadyVisible></div>
+<pre id=result></pre>
+<script>
+tapThis.addEventListener("touchstart", function( event ) {
+    alreadyVisible.style.display = "none";
+}, false);
+
+tapThis.addEventListener("mouseover", function( event ) {
+    alreadyVisible.style.display = "block";
+}, false);
+
+alreadyVisible.addEventListener("click", function( event ) {
+    result.innerHTML = "clicked alreadyVisible";
+}, false);
+
+tapThis.addEventListener("click", function( event ) {   
+    result.innerHTML = "clicked";
+    if (window.testRunner)
+        testRunner.notifyDone();
+}, false);
+</script>
+</body>
+</html>
index 86c8936..6b259f6 100644 (file)
@@ -1,3 +1,34 @@
+2019-08-14  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] Any previously destroyed renderer should not be considered a candidate for content observation.
+        https://bugs.webkit.org/show_bug.cgi?id=200732
+        <rdar://problem/54319654>
+
+        Reviewed by Simon Fraser.
+
+        A visible element should not be considered a candidate to content change observation when it loses the visiblity status momentarily.
+        This patch extends the check of re-constructed renderers for the duration of the content change observation (as opposed to just a single render tree update cycle)
+
+        Test: fast/events/touch/ios/content-observation/visible-content-gains-new-renderer2.html
+
+        * page/ios/ContentChangeObserver.cpp:
+        (WebCore::ContentChangeObserver::reset):
+        (WebCore::ContentChangeObserver::rendererWillBeDestroyed):
+        (WebCore::ContentChangeObserver::adjustObservedState):
+        (WebCore::ContentChangeObserver::renderTreeUpdateDidStart): Deleted.
+        (WebCore::ContentChangeObserver::renderTreeUpdateDidFinish): Deleted.
+        (WebCore::ContentChangeObserver::stopContentObservation): Deleted.
+        (WebCore::ContentChangeObserver::willDestroyRenderer): Deleted.
+        (WebCore::ContentChangeObserver::RenderTreeUpdateScope::RenderTreeUpdateScope): Deleted.
+        (WebCore::ContentChangeObserver::RenderTreeUpdateScope::~RenderTreeUpdateScope): Deleted.
+        * page/ios/ContentChangeObserver.h:
+        (WebCore::ContentChangeObserver::isObservingContentChanges const):
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::willBeDestroyed):
+        * rendering/updating/RenderTreeUpdater.cpp:
+        (WebCore::RenderTreeUpdater::updateRenderTree):
+        (WebCore::RenderTreeUpdater::tearDownRenderers):
+
 2019-08-14  Andy Estes  <aestes@apple.com>
 
         [Cocoa] Add some WKA extension points
index 8146289..a70510f 100644 (file)
@@ -342,28 +342,6 @@ void ContentChangeObserver::styleRecalcDidFinish()
     adjustObservedState(Event::EndedStyleRecalc);
 }
 
-void ContentChangeObserver::renderTreeUpdateDidStart()
-{
-    if (!m_document.settings().contentChangeObserverEnabled())
-        return;
-    if (!isObservingContentChanges())
-        return;
-
-    LOG(ContentObservation, "renderTreeUpdateDidStart: RenderTree update started");
-    m_isInObservedRenderTreeUpdate = true;
-    m_elementsWithDestroyedVisibleRenderer.clear();
-}
-
-void ContentChangeObserver::renderTreeUpdateDidFinish()
-{
-    if (!m_isInObservedRenderTreeUpdate)
-        return;
-
-    LOG(ContentObservation, "renderTreeUpdateDidStart: RenderTree update finished");
-    m_isInObservedRenderTreeUpdate = false;
-    m_elementsWithDestroyedVisibleRenderer.clear();
-}
-
 void ContentChangeObserver::stopObservingPendingActivities()
 {
     setShouldObserveNextStyleRecalc(false);
@@ -385,7 +363,6 @@ void ContentChangeObserver::reset()
 
     m_touchEventIsBeingDispatched = false;
     m_isInObservedStyleRecalc = false;
-    m_isInObservedRenderTreeUpdate = false;
     m_observedDomTimerIsBeingExecuted = false;
     m_mouseMovedEventIsBeingDispatched = false;
 
@@ -406,15 +383,15 @@ void ContentChangeObserver::willDetachPage()
     reset();
 }
 
-void ContentChangeObserver::willDestroyRenderer(const Element& element)
+void ContentChangeObserver::rendererWillBeDestroyed(const Element& element)
 { 
     if (!m_document.settings().contentChangeObserverEnabled())
         return;
-    if (!m_isInObservedRenderTreeUpdate)
+    if (!isObservingContentChanges())
         return;
     if (hasVisibleChangeState())
         return;
-    LOG_WITH_STREAM(ContentObservation, stream << "willDestroyRenderer element: " << &element);
+    LOG_WITH_STREAM(ContentObservation, stream << "rendererWillBeDestroyed element: " << &element);
 
     if (!isVisuallyHidden(element))
         m_elementsWithDestroyedVisibleRenderer.add(&element);
@@ -704,17 +681,6 @@ ContentChangeObserver::DOMTimerScope::~DOMTimerScope()
         m_contentChangeObserver->domTimerExecuteDidFinish(m_domTimer);
 }
 
-ContentChangeObserver::RenderTreeUpdateScope::RenderTreeUpdateScope(Document& document)
-    : m_contentChangeObserver(document.contentChangeObserver())
-{
-    m_contentChangeObserver.renderTreeUpdateDidStart();
-}
-
-ContentChangeObserver::RenderTreeUpdateScope::~RenderTreeUpdateScope()
-{
-    m_contentChangeObserver.renderTreeUpdateDidFinish();
-}
-
 }
 
 #endif // PLATFORM(IOS_FAMILY)
index af17a3d..1c2b4d0 100644 (file)
@@ -67,7 +67,7 @@ public:
     void didSuspendActiveDOMObjects();
     void willDetachPage();
 
-    void willDestroyRenderer(const Element&);
+    void rendererWillBeDestroyed(const Element&);
     void willNotProceedWithClick();
 
     void setHiddenTouchTarget(Element& targetElement) { m_hiddenTouchTargetElement = makeWeakPtr(targetElement); }
@@ -102,14 +102,6 @@ public:
         ContentChangeObserver& m_contentChangeObserver;
     };
 
-    class RenderTreeUpdateScope {
-    public:
-        RenderTreeUpdateScope(Document&);
-        ~RenderTreeUpdateScope();
-    private:
-        ContentChangeObserver& m_contentChangeObserver;
-    };
-
     class StyleRecalcScope {
     public:
         StyleRecalcScope(Document&);
@@ -177,8 +169,6 @@ private:
 
     void completeDurationBasedContentObservation();
 
-    void renderTreeUpdateDidStart();
-    void renderTreeUpdateDidFinish();
     bool visibleRendererWasDestroyed(const Element& element) const { return m_elementsWithDestroyedVisibleRenderer.contains(&element); }
     bool shouldObserveVisibilityChangeForElement(const Element&);
 
@@ -220,7 +210,6 @@ private:
     bool m_mouseMovedEventIsBeingDispatched { false };
     bool m_isBetweenTouchEndAndMouseMoved { false };
     bool m_isObservingTransitions { false };
-    bool m_isInObservedRenderTreeUpdate { false };
 };
 
 inline bool ContentChangeObserver::isObservingContentChanges() const
index 393033c..1b11e2e 100644 (file)
@@ -26,6 +26,9 @@
 #include "RenderElement.h"
 
 #include "AXObjectCache.h"
+#if PLATFORM(IOS_FAMILY)
+#include "ContentChangeObserver.h"
+#endif
 #include "ContentData.h"
 #include "CursorList.h"
 #include "ElementChildIterator.h"
@@ -919,6 +922,10 @@ inline void RenderElement::clearSubtreeLayoutRootIfNeeded() const
 
 void RenderElement::willBeDestroyed()
 {
+#if PLATFORM(IOS_FAMILY)
+    if (!renderTreeBeingDestroyed() && element())
+        document().contentChangeObserver().rendererWillBeDestroyed(*element());
+#endif
     if (m_style.hasFixedBackgroundImage() && !settings().fixedBackgroundsPaintRelativeToDocument())
         view().frameView().removeSlowRepaintObject(*this);
 
index f42bca3..9a98d25 100644 (file)
@@ -137,10 +137,6 @@ static bool shouldCreateRenderer(const Element& element, const RenderElement& pa
 
 void RenderTreeUpdater::updateRenderTree(ContainerNode& root)
 {
-#if PLATFORM(IOS_FAMILY)
-    ContentChangeObserver::RenderTreeUpdateScope observingScope(m_document);
-#endif
-
     ASSERT(root.renderer());
     ASSERT(m_parentStack.isEmpty());
 
@@ -561,9 +557,6 @@ void RenderTreeUpdater::tearDownRenderers(Element& root, TeardownType teardownTy
             GeneratedContent::removeAfterPseudoElement(element, builder);
 
             if (auto* renderer = element.renderer()) {
-#if PLATFORM(IOS_FAMILY)
-                document.contentChangeObserver().willDestroyRenderer(element);
-#endif
                 builder.destroyAndCleanUpAnonymousWrappers(*renderer);
                 element.setRenderer(nullptr);
             }