Closed Bug 1391262 Opened 2 years ago Closed 2 years ago

Crash in mozilla::layers::WebRenderBridgeChild::IPCOpen

Categories

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

Unspecified
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox-esr52 --- unaffected
firefox56 --- unaffected
firefox57 --- unaffected
firefox58 --- fixed

People

(Reporter: marcia, Assigned: sotaro)

References

(Blocks 1 open bug)

Details

(Keywords: crash, Whiteboard: [wr-mvp] [gfx-noted])

Crash Data

Attachments

(2 files, 5 obsolete files)

This bug was filed from the Socorro interface and is 
report bp-26d215f5-3039-47f2-abe6-5fb6e0170817.
=============================================================

Seen while looking at nightly data: http://bit.ly/2v4LYAO. 16 crashes/11 installs. Crashes started using 20170816100153
The WrBridge() call is probably returning null. The SetLayerObserverEpoch gets called pretty early sometimes and it has the side-effect of creating a new LayerManager without fully initializing it.
Blocks: wr-stability
See Also: → 1392316
Whiteboard: [gfx-noted]
Priority: P3 → P2
Whiteboard: [gfx-noted] → [wr-mvp] [gfx-noted]
See Also: → 1399850
Assignee: nobody → sotaro.ikeda.g
Status: NEW → ASSIGNED
Priority: P2 → P1
The crash still happens since Bug 1401849 fix. And all crash reports had "Failed to create WebRenderBridgeChild".
Depends on: 1404764
Attachment #8916848 - Attachment description: patch - Do not use remote LayerManager when its initialization fails → patch part 1 - Do not use remote LayerManager when its initialization fails
Attachment #8916850 - Attachment description: patch - Create TabChild::CreateRemoteLayerManager() → patch part 2 - Create TabChild::CreateRemoteLayerManager()
Attachment #8916848 - Flags: review?(dvander)
Attachment #8916850 - Flags: review?(dvander)
Comment on attachment 8916848 [details] [diff] [review]
patch part 1 - Do not use remote LayerManager when its initialization fails

Review of attachment 8916848 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/PuppetWidget.h
@@ +181,5 @@
>    GetLayerManager(PLayerTransactionChild* aShadowManager = nullptr,
>                    LayersBackend aBackendHint = mozilla::layers::LayersBackend::LAYERS_NONE,
>                    LayerManagerPersistence aPersistence = LAYER_MANAGER_CURRENT) override;
>  
> +  // This is used for crearing "remote" layer manager and for re-creaging it

// This is used for creating remote layer managers and for re-creating
// them after a compositor reset.  [...]
Attachment #8916848 - Flags: review?(dvander) → review+
Attachment #8916850 - Flags: review?(dvander) → review+
Apply the comment.
Attachment #8916848 - Attachment is obsolete: true
Attachment #8917205 - Flags: review+
Pushed by sikeda@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3dd22fb8666c
Do not use remote LayerManager when its initialization fails r=dvander
https://hg.mozilla.org/integration/mozilla-inbound/rev/fbd346f1d966
Create TabChild::CreateRemoteLayerManager() r=dvander
Backout by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/mozilla-inbound/rev/629e5c24737b
Backed out changeset fbd346f1d966 
https://hg.mozilla.org/integration/mozilla-inbound/rev/21156335f590
Backed out changeset 3dd22fb8666c for frequently asserting in mochitests on Windows 10 x64 debug, often in devtools/client/shared/webpack/shims/test/test_clipboard.html. r=backout
With the patches, When TabChild failed to allocate LayerTransactionChild, PuppetWidget::Paint() becomes to call "GetCurrentWidgetListener()->PaintWindow(this, region)". It seems to hit hidden bug.
Flags: needinfo?(sotaro.ikeda.g)
From the investigation, when mAttachedWidgetListener->GetView()->IsPrimaryFramePaintSuppressed() is true, PuppetWidget::mPreviouslyAttachedWidgetListener is used, in this case, assert of nsView::PaintWindow() could fail.
 https://dxr.mozilla.org/mozilla-central/source/view/nsView.cpp#1066
(In reply to Sotaro Ikeda [:sotaro] from comment #14)
> From the investigation, when
> mAttachedWidgetListener->GetView()->IsPrimaryFramePaintSuppressed() is true,
> PuppetWidget::mPreviouslyAttachedWidgetListener is used,

It was added by Bug 1157941.

But we do not need to call PaintWindow() in this case. We fallback to Basic window because of LayerTransactionChild creation failure. But we do not need to do painting here.
Attachment #8918292 - Flags: review+
Attachment #8918292 - Attachment description: patch part 1 - Do not use remote LayerManager when its initialization failspatch_1391262_5 → patch part 1 - Do not use remote LayerManager when its initialization fails
Attachment #8918293 - Flags: review+
Pushed by sikeda@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/38330dd0d9e5
Do not use remote LayerManager when its initialization fails r=dvander
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0c53e24ab2a
Create TabChild::CreateRemoteLayerManager() r=dvander
https://hg.mozilla.org/mozilla-central/rev/38330dd0d9e5
https://hg.mozilla.org/mozilla-central/rev/e0c53e24ab2a
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Duplicate of this bug: 1399850
Duplicate of this bug: 1406729
I think the root cause of this bug is the same as Bug 1406729, InternalSetDocShellIsActive might be called pretty early and CompositorBridge hasn't be fully initialized yet.

Since the crash volume in Bug 1406729 is not high and the patch is not a simple patch so I am not sure if we should uplift it to beta 57.

Sotaro, do you have any thought about this?
Flags: needinfo?(sotaro.ikeda.g)
The fix has a risk of regression like Bug 1408490 and the crash volume of Bug 1406729 is not high, it might better not up lift the fix.
Flags: needinfo?(sotaro.ikeda.g)
You need to log in before you can comment on or make changes to this bug.