Changeset 248750 in webkit


Ignore:
Timestamp:
Aug 15, 2019 3:18:56 PM (5 years ago)
Author:
Alan Bujtas
Message:

[ContentChangeObserver] Dispatch synthetic click when the visibility candidate element becomes hidden again.
​https://bugs.webkit.org/show_bug.cgi?id=200773
<rdar://problem/54351728>

Reviewed by Simon Fraser.

Source/WebCore:

This patch fixes the case when the candidate element (going from hidden to visible) becomes hidden by the end of the observation window. It essentially means that no visible change has happened
and we should proceed with dispatching the synthetic click event.
We now keep track of the candidate element and reset the visiblity state when it loses its renderer.

Test: fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden.html

  • page/ios/ContentChangeObserver.cpp:

(WebCore::ContentChangeObserver::didAddTransition):
(WebCore::ContentChangeObserver::didFinishTransition):
(WebCore::ContentChangeObserver::didInstallDOMTimer):
(WebCore::ContentChangeObserver::reset):
(WebCore::ContentChangeObserver::rendererWillBeDestroyed):
(WebCore::ContentChangeObserver::contentVisibilityDidChange):
(WebCore::ContentChangeObserver::touchEventDidStart):
(WebCore::ContentChangeObserver::touchEventDidFinish):
(WebCore::ContentChangeObserver::mouseMovedDidStart):
(WebCore::ContentChangeObserver::mouseMovedDidFinish):
(WebCore::ContentChangeObserver::adjustObservedState):
(WebCore::ContentChangeObserver::StyleChangeScope::~StyleChangeScope):
(WebCore::ContentChangeObserver::hasDeterminateState const): Deleted.

  • page/ios/ContentChangeObserver.h:

(WebCore::ContentChangeObserver::hasObservedTransition const):
(WebCore::ContentChangeObserver::setTouchEventIsBeingDispatched):
(WebCore::ContentChangeObserver::isTouchEventBeingDispatched const):
(WebCore::ContentChangeObserver::setMouseMovedEventIsBeingDispatched):
(WebCore::ContentChangeObserver::isMouseMovedEventBeingDispatched const):
(WebCore::ContentChangeObserver::isObservingContentChanges const):

LayoutTests:

  • fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden-expected.html: Added.
  • fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden.html: Added.
Location:
trunk
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r248746 r248750  
     12019-08-15  Zalan Bujtas  <zalan@apple.com>
     2
     3        [ContentChangeObserver] Dispatch synthetic click when the visibility candidate element becomes hidden again.
     4        https://bugs.webkit.org/show_bug.cgi?id=200773
     5        <rdar://problem/54351728>
     6
     7        Reviewed by Simon Fraser.
     8
     9        * fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden-expected.html: Added.
     10        * fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden.html: Added.
     11
    1122019-08-15  Robin Morisset  <rmorisset@apple.com>
    213

  • trunk/Source/WebCore/ChangeLog

    r248749 r248750  
     12019-08-15  Zalan Bujtas  <zalan@apple.com>
     2
     3        [ContentChangeObserver] Dispatch synthetic click when the visibility candidate element becomes hidden again.
     4        https://bugs.webkit.org/show_bug.cgi?id=200773
     5        <rdar://problem/54351728>
     6
     7        Reviewed by Simon Fraser.
     8
     9        This patch fixes the case when the candidate element (going from hidden to visible) becomes hidden by the end of the observation window. It essentially means that no visible change has happened
     10        and we should proceed with dispatching the synthetic click event.
     11        We now keep track of the candidate element and reset the visiblity state when it loses its renderer.
     12
     13        Test: fast/events/touch/ios/content-observation/going-from-hidden-to-visible-and-to-hidden.html
     14
     15        * page/ios/ContentChangeObserver.cpp:
     16        (WebCore::ContentChangeObserver::didAddTransition):
     17        (WebCore::ContentChangeObserver::didFinishTransition):
     18        (WebCore::ContentChangeObserver::didInstallDOMTimer):
     19        (WebCore::ContentChangeObserver::reset):
     20        (WebCore::ContentChangeObserver::rendererWillBeDestroyed):
     21        (WebCore::ContentChangeObserver::contentVisibilityDidChange):
     22        (WebCore::ContentChangeObserver::touchEventDidStart):
     23        (WebCore::ContentChangeObserver::touchEventDidFinish):
     24        (WebCore::ContentChangeObserver::mouseMovedDidStart):
     25        (WebCore::ContentChangeObserver::mouseMovedDidFinish):
     26        (WebCore::ContentChangeObserver::adjustObservedState):
     27        (WebCore::ContentChangeObserver::StyleChangeScope::~StyleChangeScope):
     28        (WebCore::ContentChangeObserver::hasDeterminateState const): Deleted.
     29        * page/ios/ContentChangeObserver.h:
     30        (WebCore::ContentChangeObserver::hasObservedTransition const):
     31        (WebCore::ContentChangeObserver::setTouchEventIsBeingDispatched):
     32        (WebCore::ContentChangeObserver::isTouchEventBeingDispatched const):
     33        (WebCore::ContentChangeObserver::setMouseMovedEventIsBeingDispatched):
     34        (WebCore::ContentChangeObserver::isMouseMovedEventBeingDispatched const):
     35        (WebCore::ContentChangeObserver::isObservingContentChanges const):
     36
    1372019-08-15  Justin Fan  <justin_fan@apple.com>
    238

  • trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp

    r248695 r248750  
    215215    if (hasVisibleChangeState())
    216216        return;
     217    if (!isObservingContentChanges())
     218        return;
    217219    if (!isObservingTransitions())
    218220        return;
    … …  
    252254        }
    253255        if (isConsideredClickable(*targetElement, ElementHadRenderer::Yes))
    254             weakThis->contentVisibilityDidChange();
     256            weakThis->contentVisibilityDidChange(*targetElement);
    255257        weakThis->adjustObservedState(Event::CompletedTransition);
    256258    });
    … …  
    272274    if (!m_document.settings().contentChangeObserverEnabled())
    273275        return;
     276    if (!isObservingContentChanges())
     277        return;
     278    if (!isObservingDOMTimerScheduling())
     279        return;
     280    if (hasVisibleChangeState())
     281        return;
    274282    if (m_document.activeDOMObjectsAreSuspended())
    275283        return;
    276284    if (timeout > maximumDelayForTimers || !singleShot)
    277         return;
    278     if (!isObservingDOMTimerScheduling())
    279         return;
    280     if (hasVisibleChangeState())
    281285        return;
    282286    LOG_WITH_STREAM(ContentObservation, stream << "didInstallDOMTimer: register this timer: (" << &timer << ") and observe when it fires.");
    … …  
    360364    stopObservingPendingActivities();
    361365    setHasNoChangeState();
     366
     367    setTouchEventIsBeingDispatched(false);
    362368    setIsBetweenTouchEndAndMouseMoved(false);
    363 
    364     m_touchEventIsBeingDispatched = false;
     369    setMouseMovedEventIsBeingDispatched(false);
     370
    365371    m_isInObservedStyleRecalc = false;
    366372    m_observedDomTimerIsBeingExecuted = false;
    367     m_mouseMovedEventIsBeingDispatched = false;
     373
     374    m_visibilityCandidateElement = { };
    368375
    369376    m_contentObservationTimer.stop();
    … …  
    390397    if (!isObservingContentChanges())
    391398        return;
    392     if (hasVisibleChangeState())
    393         return;
    394399    LOG_WITH_STREAM(ContentObservation, stream << "rendererWillBeDestroyed element: " << &element);
    395400
    396401    if (!isVisuallyHidden(element))
    397402        m_elementsWithDestroyedVisibleRenderer.add(&element);
    398 }
    399 
    400 void ContentChangeObserver::contentVisibilityDidChange()
     403    // Candidate element is no longer visible.
     404    if (m_visibilityCandidateElement == &element) {
     405        // FIXME: We should also check for other type of visiblity changes.
     406        ASSERT(hasVisibleChangeState());
     407        m_visibilityCandidateElement = { };
     408        setHasIndeterminateState();
     409    }
     410}
     411
     412void ContentChangeObserver::contentVisibilityDidChange(const Element& element)
    401413{
    402414    LOG(ContentObservation, "contentVisibilityDidChange: visible content change did happen.");
     415    // FIXME: This should evolve into a list of candidate elements.
     416    m_visibilityCandidateElement = makeWeakPtr(element);
    403417    adjustObservedState(Event::ContentVisibilityChanged);
    404418}
    … …  
    412426        return;
    413427    LOG(ContentObservation, "touchEventDidStart: touch start event started.");
    414     m_touchEventIsBeingDispatched = true;
     428    setTouchEventIsBeingDispatched(true);
    415429    adjustObservedState(Event::StartedTouchStartEventDispatching);
    416430#else
    … …  
    422436{
    423437#if ENABLE(TOUCH_EVENTS)
    424     if (!m_touchEventIsBeingDispatched)
     438    if (!isTouchEventBeingDispatched())
    425439        return;
    426440    ASSERT(m_document.settings().contentChangeObserverEnabled());
    427441    LOG(ContentObservation, "touchEventDidFinish: touch start event finished.");
    428     m_touchEventIsBeingDispatched = false;
     442    setTouchEventIsBeingDispatched(false);
    429443    adjustObservedState(Event::EndedTouchStartEventDispatching);
    430444#endif
    … …  
    436450        return;
    437451    LOG(ContentObservation, "mouseMovedDidStart: mouseMoved started.");
    438     m_mouseMovedEventIsBeingDispatched = true;
     452    setMouseMovedEventIsBeingDispatched(true);
    439453    adjustObservedState(Event::StartedMouseMovedEventDispatching);
    440454}
    … …  
    442456void ContentChangeObserver::mouseMovedDidFinish()
    443457{
    444     if (!m_mouseMovedEventIsBeingDispatched)
     458    if (!isMouseMovedEventBeingDispatched())
    445459        return;
    446460    ASSERT(m_document.settings().contentChangeObserverEnabled());
    447461    LOG(ContentObservation, "mouseMovedDidFinish: mouseMoved finished.");
    448462    adjustObservedState(Event::EndedMouseMovedEventDispatching);
    449     m_mouseMovedEventIsBeingDispatched = false;
     463    setMouseMovedEventIsBeingDispatched(false);
    450464}
    451465
    … …  
    455469        LOG(ContentObservation, "Wait until next style recalc fires.");
    456470    m_isWaitingForStyleRecalc = shouldObserve;
    457 }
    458 
    459 bool ContentChangeObserver::hasDeterminateState() const
    460 {
    461     if (hasVisibleChangeState())
    462         return true;
    463     return observedContentChange() == WKContentNoChange && !hasPendingActivity();
    464471}
    465472
    … …  
    478485
    479486    auto notifyClientIfNeeded = [&] {
    480         // First demote to "no change" if there's no pending activity anymore.
    481         if (observedContentChange() == WKContentIndeterminateChange && !hasPendingActivity())
     487        if (isTouchEventBeingDispatched()) {
     488            LOG(ContentObservation, "notifyClientIfNeeded: Touch event is being dispatched. No need to notify the client.");
     489            return;
     490        }
     491        if (isBetweenTouchEndAndMouseMoved()) {
     492            LOG(ContentObservation, "notifyClientIfNeeded: Not reached mouseMoved yet. No need to notify the client.");
     493            return;
     494        }
     495        if (isMouseMovedEventBeingDispatched()) {
     496            LOG(ContentObservation, "notifyClientIfNeeded: in mouseMoved call. No need to notify the client.");
     497            return;
     498        }
     499        if (isObservationTimeWindowActive()) {
     500            LOG(ContentObservation, "notifyClientIfNeeded: Inside the fixed window observation. No need to notify the client.");
     501            return;
     502        }
     503
     504        // The fixed observation window (which is the final step in content observation) is closed and now we check if are still waiting for timers or animations to finish.
     505        if (hasPendingActivity()) {
     506            LOG(ContentObservation, "notifyClientIfNeeded: We are still waiting on some events.");
     507            return;
     508        }
     509
     510        // First demote to "no change" because we've got no pending activity anymore.
     511        if (observedContentChange() == WKContentIndeterminateChange)
    482512            setHasNoChangeState();
    483513
    484         if (isBetweenTouchEndAndMouseMoved()) {
    485             LOG(ContentObservation, "adjustStateAndNotifyContentChangeIfNeeded: Not reached mouseMoved yet. No need to notify the client.");
    486             return;
    487         }
    488         if (m_mouseMovedEventIsBeingDispatched) {
    489             LOG(ContentObservation, "adjustStateAndNotifyContentChangeIfNeeded: in mouseMoved call. No need to notify the client.");
    490             return;
    491         }
    492         if (isObservationTimeWindowActive()) {
    493             LOG(ContentObservation, "adjustStateAndNotifyContentChangeIfNeeded: Inside the fixed window observation. No need to notify the client.");
    494             return;
    495         }
    496         if (!hasDeterminateState()) {
    497             LOG(ContentObservation, "adjustStateAndNotifyContentChangeIfNeeded: not in a determined state yet.");
    498             return;
    499         }
    500         LOG_WITH_STREAM(ContentObservation, stream << "adjustStateAndNotifyContentChangeIfNeeded: sending observedContentChange ->" << observedContentChange());
     514        LOG_WITH_STREAM(ContentObservation, stream << "notifyClientIfNeeded: sending observedContentChange ->" << observedContentChange());
    501515        ASSERT(m_document.page());
    502516        ASSERT(m_document.frame());
    … …  
    596610    // Either the page decided to call preventDefault on the touch action or the tap gesture evolved to some other gesture (long press, double tap).
    597611    if (event == Event::WillNotProceedWithClick) {
    598         reset();
     612        stopContentObservation();
    599613        return;
    600614    }
    … …  
    629643
    630644    if (changedFromHiddenToVisible() && isConsideredClickable(m_element, m_hadRenderer ? ElementHadRenderer::Yes : ElementHadRenderer::No))
    631         m_contentChangeObserver.contentVisibilityDidChange();
     645        m_contentChangeObserver.contentVisibilityDidChange(m_element);
    632646}
    633647

  • trunk/Source/WebCore/page/ios/ContentChangeObserver.h

    r248695 r248750  
    129129    void didRecognizeLongPress();
    130130
    131     void contentVisibilityDidChange();
     131    void contentVisibilityDidChange(const Element&);
    132132
    133133    void setShouldObserveDOMTimerSchedulingAndTransitions(bool);
    … …  
    160160    bool hasObservedDOMTimer() const { return !m_DOMTimerList.isEmpty(); }
    161161    bool hasObservedTransition() const { return !m_elementsWithTransition.isEmpty(); }
    162     bool hasDeterminateState() const;
    163162
    164163    void setIsBetweenTouchEndAndMouseMoved(bool isBetween) { m_isBetweenTouchEndAndMouseMoved = isBetween; }
    165164    bool isBetweenTouchEndAndMouseMoved() const { return m_isBetweenTouchEndAndMouseMoved; }
     165
     166    void setTouchEventIsBeingDispatched(bool dispatching) { m_touchEventIsBeingDispatched = dispatching; }
     167    bool isTouchEventBeingDispatched() const { return m_touchEventIsBeingDispatched; }
     168
     169    void setMouseMovedEventIsBeingDispatched(bool dispatching) { m_mouseMovedEventIsBeingDispatched = dispatching; }
     170    bool isMouseMovedEventBeingDispatched() const { return m_mouseMovedEventIsBeingDispatched; }
    166171
    167172    bool hasPendingActivity() const { return hasObservedDOMTimer() || hasObservedTransition() || m_isWaitingForStyleRecalc || isObservationTimeWindowActive(); }
    … …  
    203208    WKContentChange m_observedContentState { WKContentNoChange };
    204209    WeakPtr<Element> m_hiddenTouchTargetElement;
     210    WeakPtr<Element> m_visibilityCandidateElement;
    205211    bool m_touchEventIsBeingDispatched { false };
    206212    bool m_isWaitingForStyleRecalc { false };
    … …  
    215221inline bool ContentChangeObserver::isObservingContentChanges() const
    216222{
    217     return m_touchEventIsBeingDispatched
    218         || m_isBetweenTouchEndAndMouseMoved
    219         || m_mouseMovedEventIsBeingDispatched
     223    return isTouchEventBeingDispatched()
     224        || isBetweenTouchEndAndMouseMoved()
     225        || isMouseMovedEventBeingDispatched()
    220226        || m_observedDomTimerIsBeingExecuted
    221227        || m_isInObservedStyleRecalc
Note: See TracChangeset for help on using the changeset viewer.