Closed Bug 1152432 Opened 7 years ago Closed 3 years ago
Carets needs to deal with stuff being destroyed after Flush _Layout
http://mxr.mozilla.org/mozilla-central/source/layout/base/SelectionCarets.cpp?rev=8df7f01875e8#442 The FlushPendingNotifications call on line 465 might lead to Destroy() on mPresShell which calls Terminate on |this| and then nulls out its reference which might delete |this|. So we should proabably move this Flush to the start of the method and make sure all callers of UpdateSelectionCarets() holds a strong reference on the object they use for the call (and bail out if mPresShell is null after that call). We also need to make sure all callers of those methods deals with it, recursively. There is another Flush_Layout call here: http://mxr.mozilla.org/mozilla-central/source/layout/base/SelectionCarets.cpp?rev=8df7f01875e8#1074 We need to fix this in the same way. Also, callers of FlushPendingNotifications are required to hold a strong ref on the shell for the duration of the call, for example by holding a strong ref on the stack.
Mark this as WONTFIX because SelectionCarets are replaced by AccessibleCaret.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
AccessibleCaretManager::FlushLayout() has essentially the same problem though... https://searchfox.org/mozilla-central/rev/fcfb479e6ff63aea017d063faa17877ff750b4e5/layout/base/AccessibleCaretManager.cpp#978 but I guess we'll fix that by removing the flush in bug 1445794.
We had ensured all the entry points of AccessibleCaretEventHub' holds an extra strong reference in bug 1248847, and AccessibleCaretEventHub owns AccessibleCaretManager . I think that implies AccessibleCaretManager's validity after layout flush, right?  https://searchfox.org/mozilla-central/source/layout/base/AccessibleCaretEventHub.h#162
True, but even if AccessibleCaretManager/AccessibleCaretEventHub is robust against layout changes from the flush doesn't mean everything else is. It was used to trigger bug 1484559 and bug 1490561 just recently for example.
You need to log in before you can comment on or make changes to this bug.