Closed
Bug 1414853
Opened 4 years ago
Closed 3 years ago
Crash in nsDisplayImageContainer::ConfigureLayer
Categories
(Core :: Graphics: WebRender, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox57 | --- | disabled |
firefox58 | --- | disabled |
firefox59 | --- | verified |
People
(Reporter: marcia, Assigned: kechen)
References
Details
(Keywords: crash, regression, Whiteboard: [wr-mvp] [gfx-noted])
Crash Data
Attachments
(1 file)
This bug was filed from the Socorro interface and is report bp-32c5947b-f773-4bb3-9d47-c483c0171105. ============================================================= Seen while looking at crash stats - not exactly sure where to bucket it: http://bit.ly/2ya3K8b On 58 crashes started using 20171104100412. Install time indicates a few unique users crashed. Crash reason is EXCEPTION_ACCESS_VIOLATION_READ.
Reporter | ||
Comment 1•3 years ago
|
||
One comment "Tried to change the background colors (new page etc..) to something other than blinding white. I think I broke it. Sorry"
Reporter | ||
Updated•3 years ago
|
status-firefox59:
--- → affected
![]() |
||
Comment 2•3 years ago
|
||
New crash in 58 that only happens in the extension process. Appears to have something to do with painting.
Flags: needinfo?(amckay)
Comment 3•3 years ago
|
||
I wonder what "change the background colors (new page etc...)". Is that someone fiddling with userChrome.css?
I am getting this crash signature when trying to use the Page Shadow or Update Bookmark webextensions. When trying to use these extensions, this crash occurs, and I then cannot use any (other) webextension until after I close and restart the browser. Note: this occurs only with webrender enabled. https://crash-stats.mozilla.com/report/index/a9c886c2-eb84-4f3e-976a-3b00d0171116#tab-details
Comment 5•3 years ago
|
||
Based on comment 4, blocking on webrender.
Blocks: webrender
Flags: needinfo?(amckay)
Comment 7•3 years ago
|
||
I think this should be a P2. Kevin, please help to check this.
Assignee: nobody → kechen
Whiteboard: [wr-mvp] [gfx-noted]
Updated•3 years ago
|
Priority: -- → P2
Assignee | ||
Comment 8•3 years ago
|
||
This problem can be reproduced by installing google translator lite add-on and click the icon. A blank widget will be shown in Windows Firefox nightly with Webrender enabled and hit the crash (which should be to translator kit in the widget in the normal case). According to the crash stack, the process is trying to build layers from displaylist. This should not happen since we don't build layers when Webrender is enabled. The backend type of the layer manager might be wrong here[1]. I will keep investigating. [1] https://hg.mozilla.org/mozilla-central/annotate/e7fee7042d97/layout/painting/nsDisplayList.cpp#l2353
Updated•3 years ago
|
Whiteboard: [wr-mvp] [gfx-noted] → [wr-mvp] [triage] [gfx-noted]
Assignee | ||
Comment 9•3 years ago
|
||
According to Bug 1390741, because of transparent window problem and extensions.webextensions.remote pref enabled, we use BasicCompositor to render this kind of widget, that's why we use ClientLayerManager instead of WebrenderLayrManager. Now the problem is that we are try to build layer from a nsDisplayBackgroundImage without image data. Make a early return in [1] can skip the crash, but some component won't be shown on the add-on widget. I will keep investigating why these image data are missing. [1] https://searchfox.org/mozilla-central/rev/8839daefd69087d7ac2655b72790d3a25b6a815c/layout/painting/nsDisplayList.cpp#4408
Updated•3 years ago
|
Component: Graphics: Layers → Graphics: WebRender
Updated•3 years ago
|
Status: NEW → ASSIGNED
Priority: P2 → P1
Whiteboard: [wr-mvp] [triage] [gfx-noted] → [wr-mvp] [gfx-noted]
Comment hidden (mozreview-request) |
Assignee | ||
Comment 11•3 years ago
|
||
We added a condition in nsDisplayBackgroundImage::GetLayerState[1] to decide if we should create Webrender commands; however, by using BasicCompositor for remote extension process with Webrender enabled, it causes some unexpected behavior. This fix decouple the condition from GetLayerState, I will request for review after it passes try tests. [1] https://searchfox.org/mozilla-central/rev/9f3bd430c2b132c86c46126a0548661de876799a/layout/painting/nsDisplayList.cpp#3856
Comment hidden (mozreview-request) |
Comment 13•3 years ago
|
||
:kechen, we use ShouldUseAdvancedLayer() a lot of places and BasicCompositor usage was enabled by Bug 1390741. ShouldUseAdvancedLayer()s might cause another problems. It might better to change that ShouldUseAdvancedLayer() returns false for non WebRenderLayerManager case. It seems OK since gecko seems to use AdvancedLayer only with WebRender for now. How do you think about it? If it is OK, we could address the problem with the following change. --------------------------------------- bool nsDisplayItem::CanUseAdvancedLayer(LayerManager* aManager) const { if (!gfxPrefs::LayersAdvancedBasicLayerEnabled() && - aManager && aManager->GetBackendType() == layers::LayersBackend::LAYERS_BASIC) { + aManager && aManager->GetBackendType() != layers::LayersBackend::LAYERS_WR) { return false; }
Flags: needinfo?(kechen)
Assignee | ||
Comment 14•3 years ago
|
||
Hello Sotaro, thank you for the feedback. Yes, it seems more reasonable and can solve this kind of problems at one time. I will do the test and try before requesting the review.
Flags: needinfo?(kechen)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 17•3 years ago
|
||
The try looks good, the failures are related to the fix. https://treeherder.mozilla.org/#/jobs?repo=try&revision=c77d83449d17debbf8d71b31cd6eb650ab1d9f2f
Comment 18•3 years ago
|
||
mozreview-review |
Comment on attachment 8932771 [details] Bug 1414853 - Ensure LayerManager's backend type is LAYERS_WR in CanUseAdvancedLayer since BasicCompositor might be used for remote extension process; https://reviewboard.mozilla.org/r/203824/#review211438
Attachment #8932771 -
Flags: review?(sotaro.ikeda.g) → review+
Assignee | ||
Updated•3 years ago
|
Keywords: checkin-needed
Comment 19•3 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/699d482c86c9 Ensure LayerManager's backend type is LAYERS_WR in CanUseAdvancedLayer since BasicCompositor might be used for remote extension process; r=sotaro
Keywords: checkin-needed
Comment 20•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/699d482c86c9
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Updated•3 years ago
|
status-firefox57:
--- → disabled
status-firefox-esr52:
--- → unaffected
Comment 21•3 years ago
|
||
No more crashes in nightly 59 after the patch landed.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•