ASSERT(m_callback->hasCallback()) under IntersectionObserver::notify()

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Chris Dumez]

ASSERTION FAILED: m_callback->hasCallback()
./page/IntersectionObserver.cpp(278) : void WebCore::IntersectionObserver::notify()
1   0x4744dd069 WTFCrash
2   0x48611b24b WTFCrashWithInfo(int, char const*, char const*, int)
3   0x489e238fb WebCore::IntersectionObserver::notify()
4   0x489046a39 WebCore::Document::updateIntersectionObservations()
5   0x489e80f59 WebCore::Page::updateRendering()::$_27::operator()(WebCore::Document&) const
6   0x489e80ef1 WTF::Detail::CallableWrapper<WebCore::Page::updateRendering()::$_27, void, WebCore::Document&>::call(WebCore::Document&)
7   0x488f9b647 WTF::Function<void (WebCore::Document&)>::operator()(WebCore::Document&) const
8   0x489e3c286 WebCore::Page::forEachDocument(WTF::Function<void (WebCore::Document&)> const&) const
9   0x489e433b9 WebCore::Page::updateRendering()::$_21::operator()(WebCore::RenderingUpdateStep, WTF::Function<void (WebCore::Document&)> const&) const
10  0x489e42ffa WebCore::Page::updateRendering()
11  0x45c6e2b61 WebKit::WebPage::updateRendering()
12  0x45bef992d WebKit::TiledCoreAnimationDrawingArea::updateRendering(WebKit::TiledCoreAnimationDrawingArea::UpdateRenderingType)
13  0x45bef9898 WebKit::TiledCoreAnimationDrawingArea::forceRepaint()
14  0x45c6e1502 WebKit::WebPage::forceRepaintWithoutCallback()
15  0x45c2d8d3d WKBundlePageForceRepaint
16  0x454c2a62e WTR::InjectedBundlePage::dump()

[Chris Dumez]

<rdar://80837616>

[Chris Dumez]

Created attachment 440227 [details]
Patch

[Chris Dumez]

Created attachment 440240 [details]
Patch

[Simon Fraser (smfr)]

Does Resize Observer need the same fix?

[Chris Dumez]

(In reply to Simon Fraser (smfr) from comment #4)
> Does Resize Observer need the same fix?

I'll check but I doubt we'd expect a resize observation to ever be sent for a detached element?

[Ryosuke Niwa]

(In reply to Chris Dumez from comment #5)
> (In reply to Simon Fraser (smfr) from comment #4)
> > Does Resize Observer need the same fix?
> 
> I'll check but I doubt we'd expect a resize observation to ever be sent for
> a detached element?

I think we do for the initial observation. Also, right after the element got detached from the document because then the element becomes (0, 0) instead of whatever size it used to have. Come to think of it, wouldn't intersection observer do the same thing?

If a node was in the document and intersection observer notified of its current position, wouldn't removing the node result in another notification?

[Chris Dumez]

(In reply to Ryosuke Niwa from comment #6)
> (In reply to Chris Dumez from comment #5)
> > (In reply to Simon Fraser (smfr) from comment #4)
> > > Does Resize Observer need the same fix?
> > 
> > I'll check but I doubt we'd expect a resize observation to ever be sent for
> > a detached element?
> 
> I think we do for the initial observation. Also, right after the element got
> detached from the document because then the element becomes (0, 0) instead
> of whatever size it used to have. Come to think of it, wouldn't intersection
> observer do the same thing?
> 
> If a node was in the document and intersection observer notified of its
> current position, wouldn't removing the node result in another notification?

I am adding a test to cover the case where the target is initially visible and then we remove it from the document (it does trigger an observation notification in the intersection handler case).

I will need to test the resize observer too. It may well have the exact same bug based on your comments.

[Chris Dumez]

(In reply to Ryosuke Niwa from comment #6)
> (In reply to Chris Dumez from comment #5)
> > (In reply to Simon Fraser (smfr) from comment #4)
> > > Does Resize Observer need the same fix?
> > 
> > I'll check but I doubt we'd expect a resize observation to ever be sent for
> > a detached element?
> 
> I think we do for the initial observation. Also, right after the element got
> detached from the document because then the element becomes (0, 0) instead
> of whatever size it used to have. Come to think of it, wouldn't intersection
> observer do the same thing?

Based on my local testing, the same test does not crash for the resize observer. We don't dispatch an initial observation at all it seems when the element is initially disconnected.

[Chris Dumez]

Created attachment 440270 [details]
Patch

[Chris Dumez]

(In reply to Chris Dumez from comment #8)
> (In reply to Ryosuke Niwa from comment #6)
> > (In reply to Chris Dumez from comment #5)
> > > (In reply to Simon Fraser (smfr) from comment #4)
> > > > Does Resize Observer need the same fix?
> > > 
> > > I'll check but I doubt we'd expect a resize observation to ever be sent for
> > > a detached element?
> > 
> > I think we do for the initial observation. Also, right after the element got
> > detached from the document because then the element becomes (0, 0) instead
> > of whatever size it used to have. Come to think of it, wouldn't intersection
> > observer do the same thing?
> 
> Based on my local testing, the same test does not crash for the resize
> observer. We don't dispatch an initial observation at all it seems when the
> element is initially disconnected.

I'll add some corresponding resize observer tests in a separate patch. I could not get them to hit the assertion though.

[Simon Fraser (smfr)]

(In reply to Chris Dumez from comment #8)

> Based on my local testing, the same test does not crash for the resize
> observer. We don't dispatch an initial observation at all it seems when the
> element is initially disconnected.

That would make a nice layout/wpt test.

[EWS]

Committed r283590 (?): <https://commits.webkit.org/r283590>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 440270 [details].