Crash in [@ RetainedDisplayListBuilder::IncrementSubDocPresShellPaintCount]
Categories
(Core :: Print Preview, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox-esr68 | --- | unaffected |
firefox68 | --- | unaffected |
firefox69 | --- | verified |
firefox70 | --- | verified |
People
(Reporter: alice0775, Assigned: tnikkel)
References
(Regression)
Details
(Keywords: crash, regression, reproducible)
Crash Data
Attachments
(3 files)
(deleted),
text/plain
|
Details | |
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
|
Details |
(deleted),
patch
|
Details | Diff | Splinter Review |
This bug is for crash report bp-9602e09c-1602-48c4-8b06-b60e40190714.
Top 10 frames of crashing thread:
0 xul.dll void RetainedDisplayListBuilder::IncrementSubDocPresShellPaintCount layout/painting/RetainedDisplayListBuilder.cpp:321
1 xul.dll bool RetainedDisplayListBuilder::PreProcessDisplayList layout/painting/RetainedDisplayListBuilder.cpp:294
2 xul.dll bool RetainedDisplayListBuilder::PreProcessDisplayList layout/painting/RetainedDisplayListBuilder.cpp:259
3 xul.dll RetainedDisplayListBuilder::AttemptPartialUpdate layout/painting/RetainedDisplayListBuilder.cpp:1450
4 xul.dll nsLayoutUtils::PaintFrame layout/base/nsLayoutUtils.cpp:3984
5 xul.dll mozilla::PresShell::Paint layout/base/PresShell.cpp:6155
6 xul.dll void nsViewManager::ProcessPendingUpdatesPaint view/nsViewManager.cpp:461
7 xul.dll nsViewManager::ProcessPendingUpdatesForView view/nsViewManager.cpp:396
8 xul.dll nsViewManager::ProcessPendingUpdates view/nsViewManager.cpp:1019
9 xul.dll void nsRefreshDriver::Tick layout/base/nsRefreshDriver.cpp:2104
Reproducible: 100%
Steps To Reproduce:
- Menu > Options... > Search
- Menu > Print... if not crash then repeat step2
Actual results:
Browser crashes
Reporter | ||
Updated•5 years ago
|
Comment 1•5 years ago
|
||
:alice0775, could you try to find a regression range in using for example mozregression?
Reporter | ||
Comment 2•5 years ago
|
||
Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=0ca9a03071b76cb0aca81b83ed451dedd2b7c1f5&tochange=87409f291fa6a003f00ae782528bcd4734eeaee0
Regressed by: 87409f291fa6a003f00ae782528bcd4734eeaee0 Miko Mynttinen — Bug 1413546 - Enable retained display lists for parent process r=mattwoodrow
Reporter | ||
Comment 3•5 years ago
|
||
Comment 4•5 years ago
|
||
Clearing the keyword section based on comment 2.
Comment 5•5 years ago
|
||
Adding a ni on :miko based on the regression range.
Assignee | ||
Comment 6•5 years ago
|
||
I do intend to look at this soon.
Comment 7•5 years ago
|
||
(In reply to Timothy Nikkel (:tnikkel) from comment #6)
I do intend to look at this soon.
Thank you Timothy, let me know if you do not have time for it.
Assignee | ||
Comment 8•5 years ago
|
||
We paint and create an nsDisplaySubDocument, the subdoc doesn't have a root frame at this time, so the nsDisplaySubDocument only has a reference to the subdocument frame (in the parent document), not anything in the child document. The subdoc is about:blank at this point. The document loads, gets put in print preview mode. We try to clone the document to print it, this fails because xul documents can't be cloned. This bubbles up to https://searchfox.org/mozilla-central/rev/da855d65d1fbdd714190cab2c46130f7422f3699/layout/printing/nsPrintJob.cpp#712 where we turn off print preview because of the failure. This calls nsDocumentViewer::SetIsPrintPreview(false) and we clear the mPresContext pointer on the nsDocumentViewer here https://searchfox.org/mozilla-central/rev/da855d65d1fbdd714190cab2c46130f7422f3699/layout/base/nsDocumentViewer.cpp#4036 putting the document viewer into a kind of dead state. None of this would effect the nsDisplaySubDocument from the perspective of retained display lists because it is for a frame in the parent document I guess.
So either we can insert some invalidation/marking modified somewhere in that sequence of events or just allow nsDisplaySubDocument to not have a subdocument presshell.
These nsDisplaySubDocument's that don't have a reference to the root frame of the subdocument seem like trouble. Not relevant to this bug, but what would mark them modified when the subdoc gets a root frame? I guess this works today because the root frame creation would cause the rebuild region to intersect the nsSubdocumentFrame area and so we rebuild it. Is it possible we'd end up reusing the old item in that case? It seems like it? If that is the case we should mark the subdoc frame modified when we create a root frame.
Assignee | ||
Comment 9•5 years ago
|
||
If the subdocument display item gets created when the subdocument doesn't have a root frame it'll be based on the subdocument frame in the parent document. This means that the prescontext pointer on the document viewer of the subdocument can go away without the subdocument display item getting any kind of invalidation. So we make sure the subdocument display item gets rebuilt if that happens. The reason the prescontext pointer is important is that is what nsSubDocumentFrame::GetSubdocumentPresShellForPainting uses (via nsDocShell::GetPresShell) to get the subdocument presshell. So it will fail to get a presshell and RetainedDisplayListBuilder::IncrementSubDocPresShellPaintCount will crash.
Comment 10•5 years ago
|
||
Updated•5 years ago
|
Comment 11•5 years ago
|
||
bugherder |
Updated•5 years ago
|
Comment 12•5 years ago
|
||
Please nominate this for Beta approval when you get a chance.
Assignee | ||
Comment 13•5 years ago
|
||
Comment on attachment 9080563 [details]
Bug 1565922. When the document viewer loses it prescontext pointer we need to make sure that any potential subdocument display items get rebuilt. r?mattwoodrow
Beta/Release Uplift Approval Request
- User impact if declined: crashes
- Is this code covered by automated tests?: No
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): Just invalidates a display item
- String changes made/needed:
Comment 14•5 years ago
|
||
Comment on attachment 9080563 [details]
Bug 1565922. When the document viewer loses it prescontext pointer we need to make sure that any potential subdocument display items get rebuilt. r?mattwoodrow
Fixes a crash. Approved for 69.0b10.
Comment 15•5 years ago
|
||
The beta uplift conflicts with bug 1218456 / https://hg.mozilla.org/mozilla-central/rev/fc09b999be439a18f092ac62bc16347551ec007b
Assignee | ||
Comment 16•5 years ago
|
||
Comment 17•5 years ago
|
||
bugherder uplift |
Updated•5 years ago
|
Updated•5 years ago
|
Comment 18•5 years ago
|
||
I have managed to reproduce this issue using Firefox 69.0b4 on Windows 10 64bit.
This issue is verified fixed using Firefox 70.0a1 (BuildId:20190801214227) and Firefox 69.0b10 (BuildId:20190731232009) on Windows 10 64bit and Ubuntu 18.04 64bit.
Updated•5 years ago
|
Updated•3 years ago
|
Description
•