1619858 - Crash in [@ mozilla::dom::DOMIntersectionObserver::Unobserve]

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Gabriele Svelto [:gsvelto]]

This bug is for crash report bp-6b95d4a1-c49e-4235-8525-98fd40200228.

Top 10 frames of crashing thread:

0 XUL mozilla::dom::DOMIntersectionObserver::Unobserve xpcom/ds/nsTArray.h:485
1 XUL mozilla::dom::HTMLImageElement::StopLazyLoadingAndStartLoadIfNeeded dom/html/HTMLImageElement.cpp:1298
2 XUL mozilla::dom::LazyLoadCallback dom/base/DOMIntersectionObserver.cpp:131
3 XUL mozilla::dom::DOMIntersectionObserver::Notify dom/base/DOMIntersectionObserver.cpp:659
4 XUL mozilla::dom::Document::NotifyIntersectionObservers dom/base/Document.cpp:14671
5 XUL mozilla::detail::RunnableMethodImpl<mozilla::dom::Document*, void  xpcom/threads/nsThreadUtils.h:1210
6 XUL nsThread::ProcessNextEvent xpcom/threads/SchedulerGroup.cpp:282
7 XUL <name omitted> xpcom/threads/nsThreadUtils.cpp:481
8 XUL mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:87
9 XUL MessageLoop::Run ipc/chromium/src/base/message_loop.cc:290

This is rather odd because it seems like a NULL-pointer dereference on the lazyLoadObserver object and we're MOZ_ASSERT() it but apparently we never hit it when running debug tests (or we never noticed).

The oldest build id for this crash is 20200218213359. Note that there's older crashes that are unrelated, this is happening on 75 only.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Testcase.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1619858 - Use an intersection observer per document for lazyload. r=hiro

We can't observe when the sub-document gets detached from the root document to
drop the observation, so this is the sound thing to do.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1619858 - Minimal cleanup of the image lazy loading code. r=hiro

We don't really need to wait till we have an inner window, really.

The mOwner is only used to create a reflector, and we don't even want to expose
this observer to script, so we could leave it null if we wanted.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9580f250e2fc
Use an intersection observer per document for lazyload. r=hiro

Comment 5

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/22094 for changes under testing/web-platform/tests

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aafbae13087b
Minimal cleanup of the image lazy loading code. r=hiro

Comment 7

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/web-platform-tests/wpt/pull/22094
* Community-TC (pull_request) (https://community-tc.services.mozilla.com/tasks/groups/B_JeSywzQgS_WTCck4axMg)

Comment 8

[Emilio Cobos Álvarez (:emilio)]

So the task failed as expected on Firefox because it crashed... James, this is landed a crashtest, should I ask it to get merged manually after it gets to central?

Comment 9

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/9580f250e2fc
https://hg.mozilla.org/mozilla-central/rev/aafbae13087b

Comment 10

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 11

[James Graham [:jgraham]]

The initial problem was not the crash per-se it's that we didn't release mozlog when we made a breaking API change, so we wre throwing an error trying to record the crash. Now that's fixed it seems like the crash is intermittent (or intermittently detected), so I still need to admin merge.

Comment 12

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by jgraham

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.