Crash in nsBlockFrame::GetMinISize - stack overflow caused by AccessibleCaretEventHub::Reflow callback

RESOLVED FIXED in Firefox 51

Status

()

--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mats, Assigned: TYLin)

Tracking

(Blocks: 1 bug, {crash})

Trunk
mozilla51
x86
Windows 10
crash
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 fixed)

Details

(crash signature)

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
This bug was filed from the Socorro interface and is 
report bp-7d0aed6a-35aa-41b9-bc61-9b3892160826.
=============================================================

It looks like AccessibleCaret causes infinite recursion through flushing layout.
Flags: needinfo?(tlin)
(Assignee)

Updated

2 years ago
Assignee: nobody → tlin
Blocks: 1124074, 1244402
Flags: needinfo?(tlin)
Comment hidden (mozreview-request)
(Reporter)

Comment 2

2 years ago
mozreview-review
Comment on attachment 8785831 [details]
Bug 1298704 - Use flag to avoid calling AccessibleCaretEventHub::Reflow() recursively.

https://reviewboard.mozilla.org/r/74902/#review72792

::: layout/base/AccessibleCaretEventHub.h:135
(Diff revision 1)
>    bool mInitialized = false;
>  
> +  // Flag to avoid calling Reflow() callback recursively.
> +  bool mIsInReflowCallback = false;

Nit: perhaps we should move both these bool members to after 'mActiveTouchId' instead? (to avoid spilling due to alignment).

::: layout/base/AccessibleCaretEventHub.cpp:686
(Diff revision 1)
>  
>  NS_IMETHODIMP
>  AccessibleCaretEventHub::ReflowInterruptible(DOMHighResTimeStamp aStart,
>                                               DOMHighResTimeStamp aEnd)
>  {
> -  if (!mInitialized) {
> +  // Defer the error checking in Reflow().

s/in/to/
(Reporter)

Comment 3

2 years ago
mozreview-review
Comment on attachment 8785831 [details]
Bug 1298704 - Use flag to avoid calling AccessibleCaretEventHub::Reflow() recursively.

https://reviewboard.mozilla.org/r/74902/#review72796
Attachment #8785831 - Flags: review?(mats) → review+
(Assignee)

Comment 4

2 years ago
mozreview-review-reply
Comment on attachment 8785831 [details]
Bug 1298704 - Use flag to avoid calling AccessibleCaretEventHub::Reflow() recursively.

https://reviewboard.mozilla.org/r/74902/#review72792

> Nit: perhaps we should move both these bool members to after 'mActiveTouchId' instead? (to avoid spilling due to alignment).

Before applying my patch, `sizeof(AccessibleCaretEventHub)` is 128 on my machine. Though adding the flag to after `mInitialized` does not increase its size, but moving both bool members to after `mActiveTouchId` reduces the size from 128 to 120. Yeh!
Comment hidden (mozreview-request)

Comment 6

2 years ago
Pushed by tlin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e11d9697af59
Use flag to avoid calling AccessibleCaretEventHub::Reflow() recursively. r=mats

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e11d9697af59
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox51: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.