⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 243752 in webkit


Ignore:
Timestamp:
Apr 2, 2019, 12:43:51 PM (7 years ago)
Author:
Alan Bujtas
Message:

[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.

Source/WebCore:

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):

LayoutTests:

  • 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.
Location:
trunk
Files:
6 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r243750 r243752  
     12019-04-02  Zalan Bujtas  <zalan@apple.com>
     2
     3        [ContentChangeObserver] Ignore reconstructed renderers when checking for visibility change
     4        https://bugs.webkit.org/show_bug.cgi?id=196483
     5        <rdar://problem/49288174>
     6
     7        Reviewed by Simon Fraser.
     8
     9        * fast/events/touch/ios/content-observation/visible-content-gains-new-renderer-expected.txt: Added.
     10        * fast/events/touch/ios/content-observation/visible-content-gains-new-renderer.html: Added.
     11
    1122019-04-02  Shawn Roberts  <sroberts@apple.com>
    213

  • trunk/Source/WebCore/ChangeLog

    r243746 r243752  
     12019-04-02  Zalan Bujtas  <zalan@apple.com>
     2
     3        [ContentChangeObserver] Ignore reconstructed renderers when checking for visibility change
     4        https://bugs.webkit.org/show_bug.cgi?id=196483
     5        <rdar://problem/49288174>
     6
     7        Reviewed by Simon Fraser.
     8
     9        This patch fixes the cases when the content gets reconstructed in a way that existing and visible elements gain
     10        new renderers within one style recalc. We failed to recognize such cases and ended up detecting the newly constructed renderers
     11        as "visible change" thereby triggering hover.
     12
     13        Test: fast/events/touch/ios/content-observation/visible-content-gains-new-renderer.html
     14
     15        * page/ios/ContentChangeObserver.cpp:
     16        (WebCore::ContentChangeObserver::renderTreeUpdateDidStart):
     17        (WebCore::ContentChangeObserver::renderTreeUpdateDidFinish):
     18        (WebCore::ContentChangeObserver::reset):
     19        (WebCore::ContentChangeObserver::willDestroyRenderer):
     20        (WebCore::ContentChangeObserver::StyleChangeScope::StyleChangeScope):
     21        (WebCore::ContentChangeObserver::RenderTreeUpdateScope::RenderTreeUpdateScope):
     22        (WebCore::ContentChangeObserver::RenderTreeUpdateScope::~RenderTreeUpdateScope):
     23        * page/ios/ContentChangeObserver.h:
     24        (WebCore::ContentChangeObserver::visibleRendererWasDestroyed const):
     25        * rendering/updating/RenderTreeUpdater.cpp:
     26        (WebCore::RenderTreeUpdater::updateRenderTree):
     27        (WebCore::RenderTreeUpdater::tearDownRenderers):
     28
    1292019-04-02  Fujii Hironori  <Hironori.Fujii@sony.com>
    230

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

    r243678 r243752  
    249249}
    250250
     251void ContentChangeObserver::renderTreeUpdateDidStart()
     252{
     253    if (!m_document.settings().contentChangeObserverEnabled())
     254        return;
     255    if (!isObservingContentChanges())
     256        return;
     257
     258    LOG(ContentObservation, "renderTreeUpdateDidStart: RenderTree update started");
     259    m_isInObservedRenderTreeUpdate = true;
     260    m_elementsWithDestroyedVisibleRenderer.clear();
     261}
     262
     263void ContentChangeObserver::renderTreeUpdateDidFinish()
     264{
     265    if (!m_isInObservedRenderTreeUpdate)
     266        return;
     267
     268    LOG(ContentObservation, "renderTreeUpdateDidStart: RenderTree update finished");
     269    m_isInObservedRenderTreeUpdate = false;
     270    m_elementsWithDestroyedVisibleRenderer.clear();
     271}
     272
    251273void ContentChangeObserver::stopObservingPendingActivities()
    252274{
    … …  
    266288    m_touchEventIsBeingDispatched = false;
    267289    m_isInObservedStyleRecalc = false;
     290    m_isInObservedRenderTreeUpdate = false;
    268291    m_observedDomTimerIsBeingExecuted = false;
    269292    m_mouseMovedEventIsBeingDispatched = false;
    270293
    271294    m_contentObservationTimer.stop();
     295    m_elementsWithDestroyedVisibleRenderer.clear();
    272296}
    273297
    … …  
    282306    LOG(ContentObservation, "willDetachPage");
    283307    reset();
     308}
     309
     310void ContentChangeObserver::willDestroyRenderer(const Element& element)
     311{
     312    if (!m_document.settings().contentChangeObserverEnabled())
     313        return;
     314    if (!m_isInObservedRenderTreeUpdate)
     315        return;
     316    if (hasVisibleChangeState())
     317        return;
     318    LOG_WITH_STREAM(ContentObservation, stream << "willDestroyRenderer element: " << &element);
     319
     320    if (!isConsideredHidden(element))
     321        m_elementsWithDestroyedVisibleRenderer.add(&element);
    284322}
    285323
    … …  
    463501}
    464502
     503bool ContentChangeObserver::shouldObserveVisibilityChangeForElement(const Element& element)
     504{
     505    return isObservingContentChanges() && !hasVisibleChangeState() && !visibleRendererWasDestroyed(element);
     506}
     507
    465508ContentChangeObserver::StyleChangeScope::StyleChangeScope(Document& document, const Element& element)
    466509    : m_contentChangeObserver(document.contentChangeObserver())
    … …  
    468511    , m_hadRenderer(element.renderer())
    469512{
    470     if (m_contentChangeObserver.isObservingContentChanges() && !m_contentChangeObserver.hasVisibleChangeState())
     513    if (m_contentChangeObserver.shouldObserveVisibilityChangeForElement(element))
    471514        m_wasHidden = isConsideredHidden(m_element);
    472515}
    … …  
    559602}
    560603
     604ContentChangeObserver::RenderTreeUpdateScope::RenderTreeUpdateScope(Document& document)
     605    : m_contentChangeObserver(document.contentChangeObserver())
     606{
     607    m_contentChangeObserver.renderTreeUpdateDidStart();
     608}
     609
     610ContentChangeObserver::RenderTreeUpdateScope::~RenderTreeUpdateScope()
     611{
     612    m_contentChangeObserver.renderTreeUpdateDidFinish();
     613}
     614
    561615}
    562616

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

    r243556 r243752  
    6262    void willDetachPage();
    6363
     64    void willDestroyRenderer(const Element&);
     65
    6466    class StyleChangeScope {
    6567    public:
    … …  
    8890        WEBCORE_EXPORT MouseMovedScope(Document&);
    8991        WEBCORE_EXPORT ~MouseMovedScope();
     92    private:
     93        ContentChangeObserver& m_contentChangeObserver;
     94    };
     95
     96    class RenderTreeUpdateScope {
     97    public:
     98        RenderTreeUpdateScope(Document&);
     99        ~RenderTreeUpdateScope();
    90100    private:
    91101        ContentChangeObserver& m_contentChangeObserver;
    … …  
    160170    void completeDurationBasedContentObservation();
    161171    void setObservedContentState(WKContentChange);
     172
     173    void renderTreeUpdateDidStart();
     174    void renderTreeUpdateDidFinish();
     175    bool visibleRendererWasDestroyed(const Element& element) const { return m_elementsWithDestroyedVisibleRenderer.contains(&element); }
     176    bool shouldObserveVisibilityChangeForElement(const Element&);
    162177
    163178    enum class Event {
    … …  
    188203    // FIXME: Move over to WeakHashSet when it starts supporting const.
    189204    HashSet<const Element*> m_elementsWithTransition;
     205    HashSet<const Element*> m_elementsWithDestroyedVisibleRenderer;
    190206    WKContentChange m_observedContentState { WKContentNoChange };
    191207    bool m_touchEventIsBeingDispatched { false };
    … …  
    197213    bool m_isBetweenTouchEndAndMouseMoved { false };
    198214    bool m_isObservingTransitions { false };
     215    bool m_isInObservedRenderTreeUpdate { false };
    199216};
    200217

  • trunk/Source/WebCore/rendering/updating/RenderTreeUpdater.cpp

    r242340 r243752  
    137137void RenderTreeUpdater::updateRenderTree(ContainerNode& root)
    138138{
     139#if PLATFORM(IOS_FAMILY)
     140    ContentChangeObserver::RenderTreeUpdateScope observingScope(m_document);
     141#endif
     142
    139143    ASSERT(root.renderer());
    140144    ASSERT(m_parentStack.isEmpty());
    … …  
    557561
    558562            if (auto* renderer = element.renderer()) {
     563#if PLATFORM(IOS_FAMILY)
     564                document.contentChangeObserver().willDestroyRenderer(element);
     565#endif
    559566                builder.destroyAndCleanUpAnonymousWrappers(*renderer);
    560567                element.setRenderer(nullptr);
Note: See TracChangeset for help on using the changeset viewer.