Closed
Bug 1438397
Opened 6 years ago
Closed 6 years ago
RepaintSelectionRunner might run after the presshell is destroyed
Categories
(Core :: DOM: Editor, defect)
Core
DOM: Editor
Tracking
()
RESOLVED
FIXED
mozilla60
Tracking | Status | |
---|---|---|
firefox60 | --- | fixed |
People
(Reporter: hiro, Assigned: hiro)
References
Details
Attachments
(1 file)
In bug 1432396, I did add an assertion that checks the docshell is NOT being destroyed in nsDocShell::EnsureTransferableHookData() [1]. With the assertion, testing/marionette/harness/marionette_harness/tests/unit/test_elementsize_chrome.py TestElementSizeChrome.testShouldReturnTheSizeOfAnInput hits the assertion on MacOSX [2]. We should bail out from RepaintSelectionRunner if the docshell (I think the presshell too) is being destroyed. [1] https://hg.mozilla.org/try/rev/12f8796def09f46b6205309f4183238862279e7a#l1.351 [2] https://treeherder.mozilla.org/logviewer.html#?job_id=162291707&repo=try&lineNumber=5731 https://treeherder.mozilla.org/#/jobs?repo=try&revision=099673b8ac9cc8199c3c969505101036c3638d3a
Assignee | ||
Updated•6 years ago
|
Summary: RepaintSelectionRunner might runs after the presshell is destroyed → RepaintSelectionRunner might run after the presshell is destroyed
Comment hidden (mozreview-request) |
Comment 2•6 years ago
|
||
mozreview-review |
Comment on attachment 8951126 [details] Bug 1438397 - Don't process RepaintSelection() if the presshell is being destroyed. https://reviewboard.mozilla.org/r/220384/#review226344 ::: editor/libeditor/EditorBase.cpp:5235 (Diff revision 1) > { > } > > NS_IMETHOD Run() override > { > + nsCOMPtr<nsIPresShell> shell = do_QueryInterface(mSelectionController); Hmm, we need QI here :-(
Attachment #8951126 -
Flags: review?(masayuki) → review+
Assignee | ||
Comment 3•6 years ago
|
||
Thanks for the review! https://treeherder.mozilla.org/#/jobs?repo=try&revision=657fffa11dafd927860fc33bc3bf03e9c5dae5bb
Pushed by hikezoe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a281dd360b0a Don't process RepaintSelection() if the presshell is being destroyed. r=masayuki
Comment 5•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a281dd360b0a
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Comment 6•6 years ago
|
||
Would it be cleaner to just check mIsDestroying in PresShell instead of doing QI? Happy to submit a patch if so.
Flags: needinfo?(masayuki)
Comment 7•6 years ago
|
||
I guess it's not a big deal and the other callers may not need the check... shrug. Sorry for the drive-by comment, I just saw the commit in the pushlog :)
Flags: needinfo?(masayuki)
Comment 8•6 years ago
|
||
Yeah, in most cases, the QI is necessary anyway. We need to improve the such major case.
You need to log in
before you can comment on or make changes to this bug.
Description
•