Closed Bug 1619858 Opened 3 months ago Closed 3 months ago

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

Categories

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

Unspecified
All
defect

Tracking

()

RESOLVED FIXED
mozilla75
Tracking Status
firefox-esr68 --- unaffected
firefox73 --- unaffected
firefox74 --- unaffected
firefox75 --- fixed

People

(Reporter: gsvelto, Assigned: emilio)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(3 files)

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.

Flags: needinfo?(emilio)
Flags: needinfo?(dholbert)
Flags: needinfo?(hikezoe.birchill)
Blocks: lazyload
Priority: -- → P2
Attached file Testcase.
Flags: needinfo?(emilio)
Assignee: nobody → emilio
Flags: needinfo?(hikezoe.birchill)
Flags: needinfo?(dholbert)

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.

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.

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9580f250e2fc
Use an intersection observer per document for lazyload. r=hiro
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/22094 for changes under testing/web-platform/tests
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/aafbae13087b
Minimal cleanup of the image lazy loading code. r=hiro

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?

Flags: needinfo?(emilio) → needinfo?(james)
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla75
Upstream PR was closed without merging

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.

Flags: needinfo?(james)
Upstream PR merged by jgraham

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

Keywords: regression
You need to log in before you can comment on or make changes to this bug.