[webkit-reviews] review granted: [Bug 196483] [ContentChangeObserver] Ignore reconstructed renderers when checking for visibility change : [Attachment 366494] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted zalan
<zalan at apple.com>'s request for review:
Bug 196483: [ContentChangeObserver] Ignore reconstructed renderers when
checking for visibility change
https://bugs.webkit.org/show_bug.cgi?id=196483

Attachment 366494: Patch

https://bugs.webkit.org/attachment.cgi?id=366494&action=review




--- Comment #3 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 366494
  --> https://bugs.webkit.org/attachment.cgi?id=366494
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=366494&action=review

> Source/WebCore/ChangeLog:11
> +	   as "visible change" -> hover.

-> to "thereby triggering"

> Source/WebCore/page/ios/ContentChangeObserver.cpp:321
> +    if (!isConsideredHidden(element))
> +	   m_elementsWithDestroyedVisibleRenderer.add(&element);

Maybe add a test for this code path.

> Source/WebCore/page/ios/ContentChangeObserver.cpp:510
> +    auto needsObserving =
m_contentChangeObserver.isObservingContentChanges() 
> +	   && !m_contentChangeObserver.hasVisibleChangeState()
> +	   && !m_contentChangeObserver.visibleRendererWasDestroyed(m_element);

Maybe move into a private function

> Source/WebCore/page/ios/ContentChangeObserver.cpp:512
> +	   if (needsObserving)
> +	       m_wasHidden = isConsideredHidden(m_element);

Indentation!