- Revision
- 242321
- Author
- [email protected]
- Date
- 2019-03-02 16:15:11 -0800 (Sat, 02 Mar 2019)
Log Message
[ContentChangeObserver] Introduce ContentChangeObserver::adjustObservedState
https://bugs.webkit.org/show_bug.cgi?id=195244
<rdar://problem/48536737>
Reviewed by Simon Fraser.
Source/WebCore:
Move state change handling code to adjustObservedState() and introduce signalContentChangeIfNeeded() to
let the client know about the state change (or lack of state change).
Test: fast/events/touch/ios/visibility-change-happens-at-the-second-timer.html
* page/ios/ContentChangeObserver.cpp:
(WebCore::ContentChangeObserver::didInstallDOMTimer):
(WebCore::ContentChangeObserver::didRemoveDOMTimer):
(WebCore::ContentChangeObserver::stopObservingDOMTimerExecute):
(WebCore::ContentChangeObserver::stopObservingStyleRecalc):
(WebCore::ContentChangeObserver::clearTimersAndReportContentChange):
(WebCore::ContentChangeObserver::didContentVisibilityChange):
(WebCore::ContentChangeObserver::addObservedDOMTimer):
(WebCore::ContentChangeObserver::removeObservedDOMTimer):
(WebCore::ContentChangeObserver::setShouldObserveStyleRecalc):
(WebCore::ContentChangeObserver::adjustObservedState):
(WebCore::ContentChangeObserver::signalContentChangeIfNeeded):
* page/ios/ContentChangeObserver.h:
(WebCore::ContentChangeObserver::isObservingDOMTimerScheduling const):
(WebCore::ContentChangeObserver::addObservedDOMTimer): Deleted.
(WebCore::ContentChangeObserver::setShouldObserveStyleRecalc): Deleted.
LayoutTests:
* fast/events/touch/ios/visibility-change-happens-at-the-second-timer-expected.txt: Added.
* fast/events/touch/ios/visibility-change-happens-at-the-second-timer.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (242320 => 242321)
--- trunk/LayoutTests/ChangeLog 2019-03-03 00:02:34 UTC (rev 242320)
+++ trunk/LayoutTests/ChangeLog 2019-03-03 00:15:11 UTC (rev 242321)
@@ -1,3 +1,14 @@
+2019-03-02 Zalan Bujtas <[email protected]>
+
+ [ContentChangeObserver] Introduce ContentChangeObserver::adjustObservedState
+ https://bugs.webkit.org/show_bug.cgi?id=195244
+ <rdar://problem/48536737>
+
+ Reviewed by Simon Fraser.
+
+ * fast/events/touch/ios/visibility-change-happens-at-the-second-timer-expected.txt: Added.
+ * fast/events/touch/ios/visibility-change-happens-at-the-second-timer.html: Added.
+
2019-03-02 Wenson Hsieh <[email protected]>
[iOS] Programmatic paste access should be granted when copying and pasting within the same origin
Added: trunk/LayoutTests/fast/events/touch/ios/visibility-change-happens-at-the-second-timer-expected.txt (0 => 242321)
--- trunk/LayoutTests/fast/events/touch/ios/visibility-change-happens-at-the-second-timer-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/visibility-change-happens-at-the-second-timer-expected.txt 2019-03-03 00:15:11 UTC (rev 242321)
@@ -0,0 +1,2 @@
+PASS if 'clicked' text is not shown below.
+
Added: trunk/LayoutTests/fast/events/touch/ios/visibility-change-happens-at-the-second-timer.html (0 => 242321)
--- trunk/LayoutTests/fast/events/touch/ios/visibility-change-happens-at-the-second-timer.html (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/visibility-change-happens-at-the-second-timer.html 2019-03-03 00:15:11 UTC (rev 242321)
@@ -0,0 +1,65 @@
+<html>
+<head>
+<title>This tests the case when the second timer triggers visible content change</title>
+<script src=""
+<style>
+#tapthis {
+ width: 400px;
+ height: 400px;
+ border: 1px solid green;
+}
+
+#becomesVisible {
+ visibility: hidden;
+ width: 100px;
+ height: 100px;
+ background-color: green;
+}
+</style>
+<script>
+async function test() {
+ if (!window.testRunner || !testRunner.runUIScript)
+ return;
+
+ 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>
+<pre id=result></pre>
+<script>
+tapthis.addEventListener("mouseover", function( event ) {
+ // 1. Install 2 timers on hover
+ setTimeout(function() {
+ // 2. Trigger some non-visibility style change with forcing offsetHeight.
+ becomesVisible.style.marginLeft = "5px";
+ document.body.offsetHeight;
+ }, 0);
+ // 3. Install a longer timer with visibility change.
+ setTimeout(function() {
+ becomesVisible.style.visibility = "visible";
+ document.body.offsetHeight;
+ if (window.testRunner)
+ setTimeout(testRunner.notifyDone(), 0);
+ }, 20);
+}, false);
+
+becomesVisible.addEventListener("click", function( event ) {
+ result.innerHTML = "clicked hidden";
+}, false);
+
+tapthis.addEventListener("click", function( event ) {
+ result.innerHTML = "clicked";
+}, false);
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (242320 => 242321)
--- trunk/Source/WebCore/ChangeLog 2019-03-03 00:02:34 UTC (rev 242320)
+++ trunk/Source/WebCore/ChangeLog 2019-03-03 00:15:11 UTC (rev 242321)
@@ -1,5 +1,35 @@
2019-03-02 Zalan Bujtas <[email protected]>
+ [ContentChangeObserver] Introduce ContentChangeObserver::adjustObservedState
+ https://bugs.webkit.org/show_bug.cgi?id=195244
+ <rdar://problem/48536737>
+
+ Reviewed by Simon Fraser.
+
+ Move state change handling code to adjustObservedState() and introduce signalContentChangeIfNeeded() to
+ let the client know about the state change (or lack of state change).
+
+ Test: fast/events/touch/ios/visibility-change-happens-at-the-second-timer.html
+
+ * page/ios/ContentChangeObserver.cpp:
+ (WebCore::ContentChangeObserver::didInstallDOMTimer):
+ (WebCore::ContentChangeObserver::didRemoveDOMTimer):
+ (WebCore::ContentChangeObserver::stopObservingDOMTimerExecute):
+ (WebCore::ContentChangeObserver::stopObservingStyleRecalc):
+ (WebCore::ContentChangeObserver::clearTimersAndReportContentChange):
+ (WebCore::ContentChangeObserver::didContentVisibilityChange):
+ (WebCore::ContentChangeObserver::addObservedDOMTimer):
+ (WebCore::ContentChangeObserver::removeObservedDOMTimer):
+ (WebCore::ContentChangeObserver::setShouldObserveStyleRecalc):
+ (WebCore::ContentChangeObserver::adjustObservedState):
+ (WebCore::ContentChangeObserver::signalContentChangeIfNeeded):
+ * page/ios/ContentChangeObserver.h:
+ (WebCore::ContentChangeObserver::isObservingDOMTimerScheduling const):
+ (WebCore::ContentChangeObserver::addObservedDOMTimer): Deleted.
+ (WebCore::ContentChangeObserver::setShouldObserveStyleRecalc): Deleted.
+
+2019-03-02 Zalan Bujtas <[email protected]>
+
[ContentChangeObserver] Move away from WKContentChange values
https://bugs.webkit.org/show_bug.cgi?id=195240
<rdar://problem/48532358>
Modified: trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp (242320 => 242321)
--- trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp 2019-03-03 00:02:34 UTC (rev 242320)
+++ trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp 2019-03-03 00:15:11 UTC (rev 242321)
@@ -65,8 +65,7 @@
return;
LOG_WITH_STREAM(ContentObservation, stream << "didInstallDOMTimer: register this timer: (" << &timer << ") and observe when it fires.");
- setHasIndeterminateState();
- addObservedDOMTimer(timer);
+ registerDOMTimer(timer);
}
void ContentChangeObserver::didRemoveDOMTimer(const DOMTimer& timer)
@@ -75,10 +74,8 @@
return;
LOG_WITH_STREAM(ContentObservation, stream << "removeDOMTimer: remove registered timer (" << &timer << ")");
- removeObservedDOMTimer(timer);
- // FIXME: Just because this is the last timer, it does not mean we are in a determined state.
- if (!hasObservedDOMTimer())
- m_page.chrome().client().observedContentChange(m_page.mainFrame());
+ unregisterDOMTimer(timer);
+ notifyContentChangeIfNeeded();
}
void ContentChangeObserver::startObservingDOMTimerExecute(const DOMTimer& timer)
@@ -96,19 +93,10 @@
return;
LOG_WITH_STREAM(ContentObservation, stream << "stopObservingDOMTimerExecute: stop observing (" << &timer << ") timer callback.");
- removeObservedDOMTimer(timer);
- stopObservingContentChanges();
- // Check if the timer callback triggered either a sync or async style update.
- if (hasDeterminateState()) {
- LOG_WITH_STREAM(ContentObservation, stream << "stopObservingDOMTimerExecute: (" << &timer << ") in determined state.");
- m_page.chrome().client().observedContentChange(m_page.mainFrame());
- return;
- }
- if (WebCore::hasPendingStyleRecalc(m_page)) {
- // An async style recalc has been scheduled. Let's observe it.
- LOG_WITH_STREAM(ContentObservation, stream << "stopObservingDOMTimerExecute: (" << &timer << ") wait until next style recalc fires.");
- setShouldObserveStyleRecalc(true);
- }
+ m_isObservingContentChanges = false;
+ unregisterDOMTimer(timer);
+ setShouldObserveStyleRecalc(WebCore::hasPendingStyleRecalc(m_page));
+ notifyContentChangeIfNeeded();
}
void ContentChangeObserver::startObservingStyleRecalc()
@@ -129,12 +117,8 @@
LOG(ContentObservation, "stopObservingStyleRecalc: stop observing style recalc");
setShouldObserveStyleRecalc(false);
- if (hasDeterminateState()) {
- LOG(ContentObservation, "stopObservingStyleRecalc: has determinate state, report content change.");
- m_page.chrome().client().observedContentChange(m_page.mainFrame());
- return;
- }
- LOG(ContentObservation, "stopObservingStyleRecalc: can't decide it yet.");
+ adjustObservedState(Event::StyleRecalcFinished);
+ notifyContentChangeIfNeeded();
}
void ContentChangeObserver::clearTimersAndReportContentChange()
@@ -141,7 +125,7 @@
{
if (!hasObservedDOMTimer())
return;
- LOG(ContentObservation, "clearTimersAndReportContentChange: remove registered timers and report content change.");
+ LOG_WITH_STREAM(ContentObservation, stream << "clearTimersAndReportContentChange: remove registered timers and report content change." << observedContentChange());
clearObservedDOMTimers();
m_page.chrome().client().observedContentChange(m_page.mainFrame());
@@ -157,18 +141,19 @@
clearTimersAndReportContentChange();
}
-void ContentChangeObserver::didContentVisibilityChange()
+void ContentChangeObserver::contentVisibilityDidChange()
{
- setHasVisibleChangeState();
+ LOG(ContentObservation, "contentVisibilityDidChange: visible content change did happen.");
+ adjustObservedState(Event::ContentVisibilityChanged);
}
void ContentChangeObserver::startObservingContentChanges()
{
ASSERT(!hasPendingStyleRecalc(m_page));
+ clearObservedDOMTimers();
startObservingDOMTimerScheduling();
- setHasNoChangeState();
- clearObservedDOMTimers();
m_isObservingContentChanges = true;
+ adjustObservedState(Event::ContentObservationStarted);
}
void ContentChangeObserver::stopObservingContentChanges()
@@ -182,14 +167,25 @@
return WKObservedContentChange();
}
-void ContentChangeObserver::removeObservedDOMTimer(const DOMTimer& timer)
+void ContentChangeObserver::registerDOMTimer(const DOMTimer& timer)
{
+ m_DOMTimerList.add(&timer);
+ adjustObservedState(Event::InstalledDOMTimer);
+}
+
+void ContentChangeObserver::unregisterDOMTimer(const DOMTimer& timer)
+{
m_DOMTimerList.remove(&timer);
- // Force reset the content change flag when the last observed content modifier is removed. We should not be in an indeterminate state anymore.
- if (!hasObservedDOMTimer() && observedContentChange() == WKContentIndeterminateChange)
- setHasNoChangeState();
+ adjustObservedState(Event::RemovedDOMTimer);
}
+void ContentChangeObserver::setShouldObserveStyleRecalc(bool shouldObserve)
+{
+ if (shouldObserve)
+ LOG(ContentObservation, "Wait until next style recalc fires.");
+ m_shouldObserveStyleRecalc = shouldObserve;
+}
+
bool ContentChangeObserver::hasDeterminateState() const
{
if (hasVisibleChangeState())
@@ -197,6 +193,40 @@
return observedContentChange() == WKContentNoChange && !hasObservedDOMTimer() && !hasPendingStyleRecalc(m_page);
}
+void ContentChangeObserver::adjustObservedState(Event event)
+{
+ switch (event) {
+ case Event::ContentObservationStarted:
+ setHasNoChangeState();
+ break;
+ case Event::InstalledDOMTimer:
+ // Expecting a timer fire. Promote to an indeterminate state.
+ ASSERT(!hasVisibleChangeState());
+ setHasIndeterminateState();
+ break;
+ case Event::RemovedDOMTimer:
+ case Event::StyleRecalcFinished:
+ // Demote to "no change" when there's no pending activity anymore.
+ if (observedContentChange() == WKContentIndeterminateChange && !hasObservedDOMTimer() && !hasPendingStyleRecalc(m_page))
+ setHasNoChangeState();
+ break;
+ case Event::ContentVisibilityChanged:
+ setHasVisibleChangeState();
+ break;
+ }
+}
+
+void ContentChangeObserver::notifyContentChangeIfNeeded()
+{
+ if (!hasDeterminateState()) {
+ LOG(ContentObservation, "notifyContentChangeIfNeeded: not in a determined state yet.");
+ return;
+ }
+ LOG_WITH_STREAM(ContentObservation, stream << "notifyContentChangeIfNeeded: sending observedContentChange ->" << observedContentChange());
+ m_page.chrome().client().observedContentChange(m_page.mainFrame());
+}
+
+
static Visibility elementImplicitVisibility(const Element& element)
{
auto* renderer = element.renderer();
@@ -254,7 +284,7 @@
if ((m_previousDisplay == DisplayType::None && style->display() != DisplayType::None)
|| (m_previousVisibility == Visibility::Hidden && style->visibility() != Visibility::Hidden)
|| (m_previousImplicitVisibility == Visibility::Hidden && elementImplicitVisibility(m_element) == Visibility::Visible))
- m_contentChangeObserver->didContentVisibilityChange();
+ m_contentChangeObserver->contentVisibilityDidChange();
}
ContentChangeObserver::StyleRecalcScope::StyleRecalcScope(Page* page)
Modified: trunk/Source/WebCore/page/ios/ContentChangeObserver.h (242320 => 242321)
--- trunk/Source/WebCore/page/ios/ContentChangeObserver.h 2019-03-03 00:02:34 UTC (rev 242320)
+++ trunk/Source/WebCore/page/ios/ContentChangeObserver.h 2019-03-03 00:15:11 UTC (rev 242321)
@@ -44,7 +44,7 @@
void didInstallDOMTimer(const DOMTimer&, Seconds timeout, bool singleShot);
void didRemoveDOMTimer(const DOMTimer&);
- void didContentVisibilityChange();
+ void contentVisibilityDidChange();
void didSuspendActiveDOMObjects();
void willDetachPage();
@@ -89,12 +89,12 @@
void startObservingStyleRecalc();
void stopObservingStyleRecalc();
- void addObservedDOMTimer(const DOMTimer& timer) { m_DOMTimerList.add(&timer); }
+ void registerDOMTimer(const DOMTimer&);
+ void unregisterDOMTimer(const DOMTimer&);
bool isObservingDOMTimerScheduling() const { return m_isObservingDOMTimerScheduling; }
- void removeObservedDOMTimer(const DOMTimer&);
bool containsObservedDOMTimer(const DOMTimer& timer) const { return m_DOMTimerList.contains(&timer); }
- void setShouldObserveStyleRecalc(bool shouldObserve) { m_shouldObserveStyleRecalc = shouldObserve; }
+ void setShouldObserveStyleRecalc(bool);
bool shouldObserveStyleRecalc() const { return m_shouldObserveStyleRecalc; }
bool isObservingContentChanges() const { return m_isObservingContentChanges; }
@@ -110,6 +110,17 @@
bool hasObservedDOMTimer() const { return !m_DOMTimerList.isEmpty(); }
bool hasDeterminateState() const;
+ void notifyContentChangeIfNeeded();
+
+ enum class Event {
+ ContentObservationStarted,
+ InstalledDOMTimer,
+ RemovedDOMTimer,
+ StyleRecalcFinished,
+ ContentVisibilityChanged
+ };
+ void adjustObservedState(Event);
+
Page& m_page;
HashSet<const DOMTimer*> m_DOMTimerList;
bool m_shouldObserveStyleRecalc { false };