Closed Bug 1414853 Opened 3 years ago Closed 3 years ago

Crash in nsDisplayImageContainer::ConfigureLayer

Categories

(Core :: Graphics: WebRender, defect, P1)

Unspecified
Windows 10
defect

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.
One comment "Tried to change the background colors (new page etc..) to something other than blinding white. I think I broke it. Sorry"
New crash in 58 that only happens in the extension process. Appears to have something to do with painting.
Flags: needinfo?(amckay)
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
Based on comment 4, blocking on webrender.
Blocks: webrender
Flags: needinfo?(amckay)
Webrender is disabled on FF 58.
I think this should be a P2.

Kevin, please help to check this.
Assignee: nobody → kechen
Whiteboard: [wr-mvp] [gfx-noted]
Priority: -- → P2
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
Whiteboard: [wr-mvp] [gfx-noted] → [wr-mvp] [triage] [gfx-noted]
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
Component: Graphics: Layers → Graphics: WebRender
Status: NEW → ASSIGNED
Priority: P2 → P1
Whiteboard: [wr-mvp] [triage] [gfx-noted] → [wr-mvp] [gfx-noted]
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
: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)
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)
Duplicate of this bug: 1416729
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+
Keywords: checkin-needed
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
https://hg.mozilla.org/mozilla-central/rev/699d482c86c9
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
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.