[ContentChangeObserver] Decouple mouseMoved event and the "is observing content chang...
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 4 Mar 2019 19:33:04 +0000 (19:33 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 4 Mar 2019 19:33:04 +0000 (19:33 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195286

Reviewed by Simon Fraser.

Now isObservingContentChanges returns true only when we are actively observing content change during timer firing and/or style recalculating.
This patch also renames a couple of functions to follow the didStart/didFinish pattern.

* page/ios/ContentChangeObserver.cpp:
(WebCore::ContentChangeObserver::domTimerExecuteDidStart):
(WebCore::ContentChangeObserver::domTimerExecuteDidFinish):
(WebCore::ContentChangeObserver::styleRecalcDidStart):
(WebCore::ContentChangeObserver::styleRecalcDidFinish):
(WebCore::ContentChangeObserver::mouseMovedDidStart):
(WebCore::ContentChangeObserver::mouseMovedDidFinish):
(WebCore::ContentChangeObserver::setShouldObserveNextStyleRecalc):
(WebCore::ContentChangeObserver::StyleChangeScope::StyleChangeScope):
(WebCore::ContentChangeObserver::MouseMovedScope::MouseMovedScope):
(WebCore::ContentChangeObserver::MouseMovedScope::~MouseMovedScope):
(WebCore::ContentChangeObserver::StyleRecalcScope::StyleRecalcScope):
(WebCore::ContentChangeObserver::StyleRecalcScope::~StyleRecalcScope):
(WebCore::ContentChangeObserver::DOMTimerScope::DOMTimerScope):
(WebCore::ContentChangeObserver::DOMTimerScope::~DOMTimerScope):
(WebCore::ContentChangeObserver::startObservingDOMTimerExecute): Deleted.
(WebCore::ContentChangeObserver::stopObservingDOMTimerExecute): Deleted.
(WebCore::ContentChangeObserver::startObservingStyleRecalc): Deleted.
(WebCore::ContentChangeObserver::stopObservingStyleRecalc): Deleted.
(WebCore::ContentChangeObserver::startObservingMouseMoved): Deleted.
(WebCore::ContentChangeObserver::stopObservingMouseMoved): Deleted.
(WebCore::ContentChangeObserver::setShouldObserveStyleRecalc): Deleted.
* page/ios/ContentChangeObserver.h:
(WebCore::ContentChangeObserver::setShouldObserveDOMTimerScheduling):
(WebCore::ContentChangeObserver::isObservingDOMTimerScheduling const):
(WebCore::ContentChangeObserver::isObservingStyleRecalc const):
(WebCore::ContentChangeObserver::isObservingContentChanges const):
(WebCore::ContentChangeObserver::startObservingDOMTimerScheduling): Deleted.
(WebCore::ContentChangeObserver::stopObservingDOMTimerScheduling): Deleted.
(WebCore::ContentChangeObserver::shouldObserveStyleRecalc const): Deleted.

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

Source/WebCore/ChangeLog
Source/WebCore/page/ios/ContentChangeObserver.cpp
Source/WebCore/page/ios/ContentChangeObserver.h

index 01b33e6..312648a 100644 (file)
@@ -1,3 +1,44 @@
+2019-03-04  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] Decouple mouseMoved event and the "is observing content change" status.
+        https://bugs.webkit.org/show_bug.cgi?id=195286
+
+        Reviewed by Simon Fraser.
+
+        Now isObservingContentChanges returns true only when we are actively observing content change during timer firing and/or style recalculating.
+        This patch also renames a couple of functions to follow the didStart/didFinish pattern.
+
+        * page/ios/ContentChangeObserver.cpp:
+        (WebCore::ContentChangeObserver::domTimerExecuteDidStart):
+        (WebCore::ContentChangeObserver::domTimerExecuteDidFinish):
+        (WebCore::ContentChangeObserver::styleRecalcDidStart):
+        (WebCore::ContentChangeObserver::styleRecalcDidFinish):
+        (WebCore::ContentChangeObserver::mouseMovedDidStart):
+        (WebCore::ContentChangeObserver::mouseMovedDidFinish):
+        (WebCore::ContentChangeObserver::setShouldObserveNextStyleRecalc):
+        (WebCore::ContentChangeObserver::StyleChangeScope::StyleChangeScope):
+        (WebCore::ContentChangeObserver::MouseMovedScope::MouseMovedScope):
+        (WebCore::ContentChangeObserver::MouseMovedScope::~MouseMovedScope):
+        (WebCore::ContentChangeObserver::StyleRecalcScope::StyleRecalcScope):
+        (WebCore::ContentChangeObserver::StyleRecalcScope::~StyleRecalcScope):
+        (WebCore::ContentChangeObserver::DOMTimerScope::DOMTimerScope):
+        (WebCore::ContentChangeObserver::DOMTimerScope::~DOMTimerScope):
+        (WebCore::ContentChangeObserver::startObservingDOMTimerExecute): Deleted.
+        (WebCore::ContentChangeObserver::stopObservingDOMTimerExecute): Deleted.
+        (WebCore::ContentChangeObserver::startObservingStyleRecalc): Deleted.
+        (WebCore::ContentChangeObserver::stopObservingStyleRecalc): Deleted.
+        (WebCore::ContentChangeObserver::startObservingMouseMoved): Deleted.
+        (WebCore::ContentChangeObserver::stopObservingMouseMoved): Deleted.
+        (WebCore::ContentChangeObserver::setShouldObserveStyleRecalc): Deleted.
+        * page/ios/ContentChangeObserver.h:
+        (WebCore::ContentChangeObserver::setShouldObserveDOMTimerScheduling):
+        (WebCore::ContentChangeObserver::isObservingDOMTimerScheduling const):
+        (WebCore::ContentChangeObserver::isObservingStyleRecalc const):
+        (WebCore::ContentChangeObserver::isObservingContentChanges const):
+        (WebCore::ContentChangeObserver::startObservingDOMTimerScheduling): Deleted.
+        (WebCore::ContentChangeObserver::stopObservingDOMTimerScheduling): Deleted.
+        (WebCore::ContentChangeObserver::shouldObserveStyleRecalc const): Deleted.
+
 2019-03-04  Yusuke Suzuki  <ysuzuki@apple.com>
 
         [JSC] Offer @makeTypeError instead of exposing @TypeError
index 65ae858..8b1cfe2 100644 (file)
@@ -66,46 +66,46 @@ void ContentChangeObserver::didRemoveDOMTimer(const DOMTimer& timer)
     notifyContentChangeIfNeeded();
 }
 
-void ContentChangeObserver::startObservingDOMTimerExecute(const DOMTimer& timer)
+void ContentChangeObserver::domTimerExecuteDidStart(const DOMTimer& timer)
 {
     if (!containsObservedDOMTimer(timer))
         return;
     LOG_WITH_STREAM(ContentObservation, stream << "startObservingDOMTimerExecute: start observing (" << &timer << ") timer callback.");
 
-    m_isObservingContentChanges = true;
+    m_domTimerisBeingExecuted = true;
 }
 
-void ContentChangeObserver::stopObservingDOMTimerExecute(const DOMTimer& timer)
+void ContentChangeObserver::domTimerExecuteDidFinish(const DOMTimer& timer)
 {
     if (!containsObservedDOMTimer(timer))
         return;
     LOG_WITH_STREAM(ContentObservation, stream << "stopObservingDOMTimerExecute: stop observing (" << &timer << ") timer callback.");
 
-    m_isObservingContentChanges = false;
+    m_domTimerisBeingExecuted = false;
     unregisterDOMTimer(timer);
-    setShouldObserveStyleRecalc(m_document.hasPendingStyleRecalc());
+    setShouldObserveNextStyleRecalc(m_document.hasPendingStyleRecalc());
     notifyContentChangeIfNeeded();
 }
 
-void ContentChangeObserver::startObservingStyleRecalc()
+void ContentChangeObserver::styleRecalcDidStart()
 {
-    if (!shouldObserveStyleRecalc())
+    if (!isObservingStyleRecalc())
         return;
     if (hasVisibleChangeState())
         return;
     LOG(ContentObservation, "startObservingStyleRecalc: start observing style recalc.");
 
-    m_isObservingContentChanges = true;
+    m_styleRecalcIsBeingExecuted = true;
 }
 
-void ContentChangeObserver::stopObservingStyleRecalc()
+void ContentChangeObserver::styleRecalcDidFinish()
 {
-    if (!shouldObserveStyleRecalc())
+    if (!isObservingStyleRecalc())
         return;
     LOG(ContentObservation, "stopObservingStyleRecalc: stop observing style recalc");
 
-    setShouldObserveStyleRecalc(false);
-    m_isObservingContentChanges = false;
+    m_styleRecalcIsBeingExecuted = false;
+    setShouldObserveNextStyleRecalc(false);
     adjustObservedState(Event::StyleRecalcFinished);
     notifyContentChangeIfNeeded();
 }
@@ -138,19 +138,17 @@ void ContentChangeObserver::contentVisibilityDidChange()
     adjustObservedState(Event::ContentVisibilityChanged);
 }
 
-void ContentChangeObserver::startObservingMouseMoved()
+void ContentChangeObserver::mouseMovedDidStart()
 {
     ASSERT(!m_document.hasPendingStyleRecalc());
     clearObservedDOMTimers();
-    startObservingDOMTimerScheduling();
-    m_isObservingContentChanges = true;
+    setShouldObserveDOMTimerScheduling(true);
     adjustObservedState(Event::ContentObservationStarted);
 }
 
-void ContentChangeObserver::stopObservingMouseMoved()
+void ContentChangeObserver::mouseMovedDidFinish()
 {
-    stopObservingDOMTimerScheduling();
-    m_isObservingContentChanges = false;
+    setShouldObserveDOMTimerScheduling(false);
 }
 
 WKContentChange ContentChangeObserver::observedContentChange() const
@@ -170,11 +168,11 @@ void ContentChangeObserver::unregisterDOMTimer(const DOMTimer& timer)
     adjustObservedState(Event::RemovedDOMTimer);
 }
 
-void ContentChangeObserver::setShouldObserveStyleRecalc(bool shouldObserve)
+void ContentChangeObserver::setShouldObserveNextStyleRecalc(bool shouldObserve)
 {
     if (shouldObserve)
         LOG(ContentObservation, "Wait until next style recalc fires.");
-    m_shouldObserveStyleRecalc = shouldObserve;
+    m_isObservingStyleRecalc = shouldObserve;
 }
 
 bool ContentChangeObserver::hasDeterminateState() const
@@ -246,7 +244,7 @@ static Visibility elementImplicitVisibility(const Element& element)
 ContentChangeObserver::StyleChangeScope::StyleChangeScope(Document& document, const Element& element)
     : m_contentChangeObserver(document.contentChangeObserver())
     , m_element(element)
-    , m_needsObserving(m_contentChangeObserver.isObservingContentChanges() && m_contentChangeObserver.observedContentChange() != WKContentVisibilityChange)
+    , m_needsObserving(m_contentChangeObserver.isObservingContentChanges() && !m_contentChangeObserver.hasVisibleChangeState())
 {
     if (m_needsObserving) {
         m_previousDisplay = element.renderStyle() ? element.renderStyle()->display() : DisplayType::None;
@@ -283,23 +281,23 @@ ContentChangeObserver::StyleChangeScope::~StyleChangeScope()
 ContentChangeObserver::MouseMovedScope::MouseMovedScope(Document& document)
     : m_contentChangeObserver(document.contentChangeObserver())
 {
-    m_contentChangeObserver.startObservingMouseMoved();
+    m_contentChangeObserver.mouseMovedDidStart();
 }
 
 ContentChangeObserver::MouseMovedScope::~MouseMovedScope()
 {
-    m_contentChangeObserver.stopObservingMouseMoved();
+    m_contentChangeObserver.mouseMovedDidFinish();
 }
 
 ContentChangeObserver::StyleRecalcScope::StyleRecalcScope(Document& document)
     : m_contentChangeObserver(document.contentChangeObserver())
 {
-    m_contentChangeObserver.startObservingStyleRecalc();
+    m_contentChangeObserver.styleRecalcDidStart();
 }
 
 ContentChangeObserver::StyleRecalcScope::~StyleRecalcScope()
 {
-    m_contentChangeObserver.stopObservingStyleRecalc();
+    m_contentChangeObserver.styleRecalcDidFinish();
 }
 
 ContentChangeObserver::DOMTimerScope::DOMTimerScope(Document* document, const DOMTimer& domTimer)
@@ -307,13 +305,13 @@ ContentChangeObserver::DOMTimerScope::DOMTimerScope(Document* document, const DO
     , m_domTimer(domTimer)
 {
     if (m_contentChangeObserver)
-        m_contentChangeObserver->startObservingDOMTimerExecute(m_domTimer);
+        m_contentChangeObserver->domTimerExecuteDidStart(m_domTimer);
 }
 
 ContentChangeObserver::DOMTimerScope::~DOMTimerScope()
 {
     if (m_contentChangeObserver)
-        m_contentChangeObserver->stopObservingDOMTimerExecute(m_domTimer);
+        m_contentChangeObserver->domTimerExecuteDidFinish(m_domTimer);
 }
 
 }
index 07b5ec9..23d1250 100644 (file)
@@ -86,27 +86,23 @@ public:
     };
 
 private:
-    void startObservingMouseMoved();
-    void stopObservingMouseMoved();
-
-    void startObservingDOMTimerScheduling() { m_isObservingDOMTimerScheduling = true; }
-    void stopObservingDOMTimerScheduling() { m_isObservingDOMTimerScheduling = false; }
-
-    void startObservingDOMTimerExecute(const DOMTimer&);
-    void stopObservingDOMTimerExecute(const DOMTimer&);
-
-    void startObservingStyleRecalc();
-    void stopObservingStyleRecalc();
+    void mouseMovedDidStart();
+    void mouseMovedDidFinish();
 
+    void setShouldObserveDOMTimerScheduling(bool observe) { m_isObservingDOMTimerScheduling = observe; }
+    bool isObservingDOMTimerScheduling() const { return m_isObservingDOMTimerScheduling; }
+    void domTimerExecuteDidStart(const DOMTimer&);
+    void domTimerExecuteDidFinish(const DOMTimer&);
     void registerDOMTimer(const DOMTimer&);
     void unregisterDOMTimer(const DOMTimer&);
-    bool isObservingDOMTimerScheduling() const { return m_isObservingDOMTimerScheduling; }
     bool containsObservedDOMTimer(const DOMTimer& timer) const { return m_DOMTimerList.contains(&timer); }
 
-    void setShouldObserveStyleRecalc(bool);
-    bool shouldObserveStyleRecalc() const { return m_shouldObserveStyleRecalc; }
+    void styleRecalcDidStart();
+    void styleRecalcDidFinish();
+    void setShouldObserveNextStyleRecalc(bool);
+    bool isObservingStyleRecalc() const { return m_isObservingStyleRecalc; }
 
-    bool isObservingContentChanges() const { return m_isObservingContentChanges; }
+    bool isObservingContentChanges() const { return m_domTimerisBeingExecuted || m_styleRecalcIsBeingExecuted; }
 
     void clearObservedDOMTimers() { m_DOMTimerList.clear(); }
     void clearTimersAndReportContentChange();
@@ -132,9 +128,10 @@ private:
 
     Document& m_document;
     HashSet<const DOMTimer*> m_DOMTimerList;
-    bool m_shouldObserveStyleRecalc { false };
+    bool m_isObservingStyleRecalc { false };
+    bool m_styleRecalcIsBeingExecuted { false };
     bool m_isObservingDOMTimerScheduling { false };
-    bool m_isObservingContentChanges { false };
+    bool m_domTimerisBeingExecuted { false };
 };
 
 inline void ContentChangeObserver::setHasNoChangeState()