Diff
Modified: trunk/LayoutTests/ChangeLog (243751 => 243752)
--- trunk/LayoutTests/ChangeLog 2019-04-02 19:41:02 UTC (rev 243751)
+++ trunk/LayoutTests/ChangeLog 2019-04-02 19:43:51 UTC (rev 243752)
@@ -1,3 +1,14 @@
+2019-04-02 Zalan Bujtas <[email protected]>
+
+ [ContentChangeObserver] Ignore reconstructed renderers when checking for visibility change
+ https://bugs.webkit.org/show_bug.cgi?id=196483
+ <rdar://problem/49288174>
+
+ Reviewed by Simon Fraser.
+
+ * fast/events/touch/ios/content-observation/visible-content-gains-new-renderer-expected.txt: Added.
+ * fast/events/touch/ios/content-observation/visible-content-gains-new-renderer.html: Added.
+
2019-04-02 Shawn Roberts <[email protected]>
accessibility/mac/press-not-work-for-disabled-menu-list.html is a flaky failure
Added: trunk/LayoutTests/fast/events/touch/ios/content-observation/non-visible-becomes-visible-and-gains-new-renderer-expected.txt (0 => 243752)
--- trunk/LayoutTests/fast/events/touch/ios/content-observation/non-visible-becomes-visible-and-gains-new-renderer-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/content-observation/non-visible-becomes-visible-and-gains-new-renderer-expected.txt 2019-04-02 19:43:51 UTC (rev 243752)
@@ -0,0 +1,3 @@
+PASS if 'clicked' text is not shown below.
+content
+
Added: trunk/LayoutTests/fast/events/touch/ios/content-observation/non-visible-becomes-visible-and-gains-new-renderer.html (0 => 243752)
--- trunk/LayoutTests/fast/events/touch/ios/content-observation/non-visible-becomes-visible-and-gains-new-renderer.html (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/content-observation/non-visible-becomes-visible-and-gains-new-renderer.html 2019-04-02 19:43:51 UTC (rev 243752)
@@ -0,0 +1,60 @@
+<html>
+<head>
+<title>This tests the case when non-visible content gets rebuilt and visible and should be considered as new content</title>
+<script src=""
+<style>
+#tapthis {
+ width: 400px;
+ height: 400px;
+ border: 1px solid green;
+}
+
+#becomesVisible {
+ visibility: hidden;
+ 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();
+
+ 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=visibleParent><div id=becomesVisible>content</div></div>
+<pre id=result></pre>
+<script>
+tapthis.addEventListener("mouseover", function( event ) {
+ visibleParent.style.display = "inline-block";
+ becomesVisible.style.visibility = "visible";
+ document.body.offsetHeight;
+ if (window.testRunner)
+ setTimeout(testRunner.notifyDone(), 50);
+}, false);
+
+becomesVisible.addEventListener("click", function( event ) {
+ result.innerHTML = "clicked hidden";
+}, false);
+
+tapthis.addEventListener("click", function( event ) {
+ result.innerHTML = "clicked";
+}, false);
+</script>
+</body>
+</html>
Added: trunk/LayoutTests/fast/events/touch/ios/content-observation/non-visible-content-gains-new-renderer-expected.txt (0 => 243752)
--- trunk/LayoutTests/fast/events/touch/ios/content-observation/non-visible-content-gains-new-renderer-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/content-observation/non-visible-content-gains-new-renderer-expected.txt 2019-04-02 19:43:51 UTC (rev 243752)
@@ -0,0 +1,2 @@
+PASS if 'clicked' text is shown below.
+ clicked
Added: trunk/LayoutTests/fast/events/touch/ios/content-observation/non-visible-content-gains-new-renderer.html (0 => 243752)
--- trunk/LayoutTests/fast/events/touch/ios/content-observation/non-visible-content-gains-new-renderer.html (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/content-observation/non-visible-content-gains-new-renderer.html 2019-04-02 19:43:51 UTC (rev 243752)
@@ -0,0 +1,59 @@
+<html>
+<head>
+<title>This tests the case when non-visible content gets rebuilt and should not be considered as new content</title>
+<script src=""
+<style>
+#tapthis {
+ width: 400px;
+ height: 400px;
+ border: 1px solid green;
+}
+
+#notVisible {
+ visibility: hidden;
+ 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();
+
+ 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 shown below.</div>
+<div id=visibleParent><div id=notVisible>content</div></div>
+<pre id=result></pre>
+<script>
+tapthis.addEventListener("mouseover", function( event ) {
+ visibleParent.style.display = "inline-block";
+ document.body.offsetHeight;
+}, false);
+
+notVisible.addEventListener("click", function( event ) {
+ result.innerHTML = "clicked hidden";
+}, false);
+
+tapthis.addEventListener("click", function( event ) {
+ result.innerHTML = "clicked";
+ if (window.testRunner)
+ testRunner.notifyDone();
+}, false);
+</script>
+</body>
+</html>
Added: trunk/LayoutTests/fast/events/touch/ios/content-observation/visible-content-gains-new-renderer-expected.txt (0 => 243752)
--- trunk/LayoutTests/fast/events/touch/ios/content-observation/visible-content-gains-new-renderer-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/content-observation/visible-content-gains-new-renderer-expected.txt 2019-04-02 19:43:51 UTC (rev 243752)
@@ -0,0 +1,3 @@
+PASS if 'clicked' text is shown below.
+content
+ clicked
Added: trunk/LayoutTests/fast/events/touch/ios/content-observation/visible-content-gains-new-renderer.html (0 => 243752)
--- trunk/LayoutTests/fast/events/touch/ios/content-observation/visible-content-gains-new-renderer.html (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/content-observation/visible-content-gains-new-renderer.html 2019-04-02 19:43:51 UTC (rev 243752)
@@ -0,0 +1,58 @@
+<html>
+<head>
+<title>This tests the case when visible content gets rebuilt and should not be considered as new content</title>
+<script src=""
+<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();
+
+ 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 shown below.</div>
+<div id=visibleParent><div id=alreadyVisible>content</div></div>
+<pre id=result></pre>
+<script>
+tapthis.addEventListener("mouseover", function( event ) {
+ visibleParent.style.display = "inline-block";
+ document.body.offsetHeight;
+}, false);
+
+alreadyVisible.addEventListener("click", function( event ) {
+ result.innerHTML = "clicked hidden";
+}, false);
+
+tapthis.addEventListener("click", function( event ) {
+ result.innerHTML = "clicked";
+ if (window.testRunner)
+ testRunner.notifyDone();
+}, false);
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (243751 => 243752)
--- trunk/Source/WebCore/ChangeLog 2019-04-02 19:41:02 UTC (rev 243751)
+++ trunk/Source/WebCore/ChangeLog 2019-04-02 19:43:51 UTC (rev 243752)
@@ -1,3 +1,31 @@
+2019-04-02 Zalan Bujtas <[email protected]>
+
+ [ContentChangeObserver] Ignore reconstructed renderers when checking for visibility change
+ https://bugs.webkit.org/show_bug.cgi?id=196483
+ <rdar://problem/49288174>
+
+ Reviewed by Simon Fraser.
+
+ This patch fixes the cases when the content gets reconstructed in a way that existing and visible elements gain
+ new renderers within one style recalc. We failed to recognize such cases and ended up detecting the newly constructed renderers
+ as "visible change" thereby triggering hover.
+
+ Test: fast/events/touch/ios/content-observation/visible-content-gains-new-renderer.html
+
+ * page/ios/ContentChangeObserver.cpp:
+ (WebCore::ContentChangeObserver::renderTreeUpdateDidStart):
+ (WebCore::ContentChangeObserver::renderTreeUpdateDidFinish):
+ (WebCore::ContentChangeObserver::reset):
+ (WebCore::ContentChangeObserver::willDestroyRenderer):
+ (WebCore::ContentChangeObserver::StyleChangeScope::StyleChangeScope):
+ (WebCore::ContentChangeObserver::RenderTreeUpdateScope::RenderTreeUpdateScope):
+ (WebCore::ContentChangeObserver::RenderTreeUpdateScope::~RenderTreeUpdateScope):
+ * page/ios/ContentChangeObserver.h:
+ (WebCore::ContentChangeObserver::visibleRendererWasDestroyed const):
+ * rendering/updating/RenderTreeUpdater.cpp:
+ (WebCore::RenderTreeUpdater::updateRenderTree):
+ (WebCore::RenderTreeUpdater::tearDownRenderers):
+
2019-04-02 Fujii Hironori <[email protected]>
[CMake] WEBKIT_MAKE_FORWARDING_HEADERS shouldn't use POST_BUILD to copy generated headers
Modified: trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp (243751 => 243752)
--- trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp 2019-04-02 19:41:02 UTC (rev 243751)
+++ trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp 2019-04-02 19:43:51 UTC (rev 243752)
@@ -248,6 +248,28 @@
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);
@@ -265,10 +287,12 @@
m_touchEventIsBeingDispatched = false;
m_isInObservedStyleRecalc = false;
+ m_isInObservedRenderTreeUpdate = false;
m_observedDomTimerIsBeingExecuted = false;
m_mouseMovedEventIsBeingDispatched = false;
m_contentObservationTimer.stop();
+ m_elementsWithDestroyedVisibleRenderer.clear();
}
void ContentChangeObserver::didSuspendActiveDOMObjects()
@@ -283,6 +307,20 @@
reset();
}
+void ContentChangeObserver::willDestroyRenderer(const Element& element)
+{
+ if (!m_document.settings().contentChangeObserverEnabled())
+ return;
+ if (!m_isInObservedRenderTreeUpdate)
+ return;
+ if (hasVisibleChangeState())
+ return;
+ LOG_WITH_STREAM(ContentObservation, stream << "willDestroyRenderer element: " << &element);
+
+ if (!isConsideredHidden(element))
+ m_elementsWithDestroyedVisibleRenderer.add(&element);
+}
+
void ContentChangeObserver::contentVisibilityDidChange()
{
LOG(ContentObservation, "contentVisibilityDidChange: visible content change did happen.");
@@ -462,12 +500,17 @@
}
}
+bool ContentChangeObserver::shouldObserveVisibilityChangeForElement(const Element& element)
+{
+ return isObservingContentChanges() && !hasVisibleChangeState() && !visibleRendererWasDestroyed(element);
+}
+
ContentChangeObserver::StyleChangeScope::StyleChangeScope(Document& document, const Element& element)
: m_contentChangeObserver(document.contentChangeObserver())
, m_element(element)
, m_hadRenderer(element.renderer())
{
- if (m_contentChangeObserver.isObservingContentChanges() && !m_contentChangeObserver.hasVisibleChangeState())
+ if (m_contentChangeObserver.shouldObserveVisibilityChangeForElement(element))
m_wasHidden = isConsideredHidden(m_element);
}
@@ -558,6 +601,17 @@
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)
Modified: trunk/Source/WebCore/page/ios/ContentChangeObserver.h (243751 => 243752)
--- trunk/Source/WebCore/page/ios/ContentChangeObserver.h 2019-04-02 19:41:02 UTC (rev 243751)
+++ trunk/Source/WebCore/page/ios/ContentChangeObserver.h 2019-04-02 19:43:51 UTC (rev 243752)
@@ -61,6 +61,8 @@
void didSuspendActiveDOMObjects();
void willDetachPage();
+ void willDestroyRenderer(const Element&);
+
class StyleChangeScope {
public:
StyleChangeScope(Document&, const Element&);
@@ -91,6 +93,14 @@
ContentChangeObserver& m_contentChangeObserver;
};
+ class RenderTreeUpdateScope {
+ public:
+ RenderTreeUpdateScope(Document&);
+ ~RenderTreeUpdateScope();
+ private:
+ ContentChangeObserver& m_contentChangeObserver;
+ };
+
class StyleRecalcScope {
public:
StyleRecalcScope(Document&);
@@ -160,6 +170,11 @@
void completeDurationBasedContentObservation();
void setObservedContentState(WKContentChange);
+ void renderTreeUpdateDidStart();
+ void renderTreeUpdateDidFinish();
+ bool visibleRendererWasDestroyed(const Element& element) const { return m_elementsWithDestroyedVisibleRenderer.contains(&element); }
+ bool shouldObserveVisibilityChangeForElement(const Element&);
+
enum class Event {
StartedTouchStartEventDispatching,
EndedTouchStartEventDispatching,
@@ -187,6 +202,7 @@
HashSet<const DOMTimer*> m_DOMTimerList;
// FIXME: Move over to WeakHashSet when it starts supporting const.
HashSet<const Element*> m_elementsWithTransition;
+ HashSet<const Element*> m_elementsWithDestroyedVisibleRenderer;
WKContentChange m_observedContentState { WKContentNoChange };
bool m_touchEventIsBeingDispatched { false };
bool m_isWaitingForStyleRecalc { false };
@@ -196,6 +212,7 @@
bool m_mouseMovedEventIsBeingDispatched { false };
bool m_isBetweenTouchEndAndMouseMoved { false };
bool m_isObservingTransitions { false };
+ bool m_isInObservedRenderTreeUpdate { false };
};
inline void ContentChangeObserver::setObservedContentState(WKContentChange observedContentChange)
Modified: trunk/Source/WebCore/rendering/updating/RenderTreeUpdater.cpp (243751 => 243752)
--- trunk/Source/WebCore/rendering/updating/RenderTreeUpdater.cpp 2019-04-02 19:41:02 UTC (rev 243751)
+++ trunk/Source/WebCore/rendering/updating/RenderTreeUpdater.cpp 2019-04-02 19:43:51 UTC (rev 243752)
@@ -136,6 +136,10 @@
void RenderTreeUpdater::updateRenderTree(ContainerNode& root)
{
+#if PLATFORM(IOS_FAMILY)
+ ContentChangeObserver::RenderTreeUpdateScope observingScope(m_document);
+#endif
+
ASSERT(root.renderer());
ASSERT(m_parentStack.isEmpty());
@@ -556,6 +560,9 @@
GeneratedContent::removeAfterPseudoElement(element, builder);
if (auto* renderer = element.renderer()) {
+#if PLATFORM(IOS_FAMILY)
+ document.contentChangeObserver().willDestroyRenderer(element);
+#endif
builder.destroyAndCleanUpAnonymousWrappers(*renderer);
element.setRenderer(nullptr);
}
