1633019 - Crash in [@ mozilla::PresShell::EventHandler::MaybeFlushThrottledStyles]

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P3)

Description

[Hiroyuki Ikezoe (:hiro)]

This bug is for crash report bp-a2a6ff81-8426-4a32-ac3e-294a70200424.

Top 10 frames of crashing thread:

0 XUL mozilla::PresShell::EventHandler::MaybeFlushThrottledStyles layout/base/PresShell.cpp:7283
1 XUL mozilla::PresShell::EventHandler::HandleEventUsingCoordinates layout/base/PresShell.cpp:6577
2 XUL mozilla::WidgetEvent::IsUsingCoordinates const widget/WidgetEventImpl.cpp:449
3 XUL mozilla::PresShell::EventHandler::HandleEvent layout/base/PresShell.cpp:6517
4 XUL mozilla::PresShell::HandleEvent layout/base/PresShell.cpp:6443
5 XUL mozilla::PresShell::EventHandler::MaybeHandleEventWithAnotherPresShell layout/base/PresShell.cpp:7172
6 XUL mozilla::PresShell::EventHandler::HandleEvent layout/base/PresShell.cpp:6505
7 XUL mozilla::PresShell::HandleEvent layout/base/PresShell.cpp:6443
8 XUL nsViewManager::DispatchEvent view/nsViewManager.cpp:751
9 XUL nsView::HandleEvent view/nsView.cpp:1135

Copying from bug 1555645 comment 9.

I haven't audited all crash reports, but as far as I can tell the call stack is including MaybeHandleEventWithAnotherPresShell which means the crash happened inside a nested document. And the crash happened at the code around we tried to get the root pres shell, it may be possible that the root pres shell is an ancestor of the initial target pres shell, I am quite unsure though.

Anyways, my question is "do we really need to flush throttled styles for the root pres shell?". My answer is "no". Given that we've already called MaybeFlushPendingNotifications just before we call MaybeFlushThrottledStyles, it would be sufficient.

As a side note about MaybeFlushThrottledStyles, emilo has a plan to drop the MaybeFlushThrottledStyles call.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Crash reports are quite annoying in this case. They don't clearly indicate where the crash is happening. The lines depend on the platform and often don't make quite sense.
This isn't very common crash, and it is null+offset (I just haven't found what is null).

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

aha, it might be GetViewManager() which returns null.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Bug 1633019 - Crash in [@ mozilla::PresShell::EventHandler::MaybeFlushThrottledStyles], r=masayuki

GetViewManager() may return null if the relevant presshell is being destroyed.
This is a guess fix, but the crash reports seem to hint about this issue.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4de83f9f1880
Crash in [@ mozilla::PresShell::EventHandler::MaybeFlushThrottledStyles], r=masayuki

Comment 5

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/4de83f9f1880

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for ESR102 approval when you get a chance. It grafts cleanly.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 9302506 [details]
Bug 1633019 - Crash in [@ mozilla::PresShell::EventHandler::MaybeFlushThrottledStyles], r=masayuki

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Simple null check to prevent crashes
• User impact if declined:
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): null check

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9302506 [details]
Bug 1633019 - Crash in [@ mozilla::PresShell::EventHandler::MaybeFlushThrottledStyles], r=masayuki

Approved for 102.6esr.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr102/rev/db6a4af87843