Closed
Bug 1394337
Opened 7 years ago
Closed 7 years ago
Fix uninitailzed mPipelineId by WebRenderBridgeParent::CreateDestroyed()
Categories
(Core :: Graphics: WebRender, enhancement)
Core
Graphics: WebRender
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: sotaro, Assigned: sotaro)
References
Details
Attachments
(1 file, 2 obsolete files)
5.95 KB,
patch
|
kats
:
review+
|
Details | Diff | Splinter Review |
Current WebRenderBridgeParent::CreateDestroyed() causes to leak CompositorBridgeParent::LayerTreeState.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → sotaro.ikeda.g
Assignee | ||
Comment 1•7 years ago
|
||
(In reply to Sotaro Ikeda [:sotaro PTO 1/Sep - 13/Sep] from comment #0) > Current WebRenderBridgeParent::CreateDestroyed() causes to leak > CompositorBridgeParent::LayerTreeState. WebRenderBridgeParent holded uninitailzed mPipelineId when it was created by WebRenderBridgeParent::CreateDestroyed().
Assignee | ||
Comment 2•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8901720 -
Flags: review?(bugmail)
Assignee | ||
Comment 3•7 years ago
|
||
(In reply to Sotaro Ikeda [:sotaro PTO 1/Sep - 13/Sep] from comment #1) > > WebRenderBridgeParent holded uninitailzed mPipelineId when it was created by > WebRenderBridgeParent::CreateDestroyed(). mPipelineId is used as layers id like CrossProcessCompositorBridgeParent::DeallocPWebRenderBridgeParent() https://dxr.mozilla.org/mozilla-central/source/gfx/layers/ipc/CrossProcessCompositorBridgeParent.cpp#251
Comment 4•7 years ago
|
||
Comment on attachment 8901720 [details] [diff] [review] patch - Fix WebRenderBridgeParent::CreateDestroyed() Review of attachment 8901720 [details] [diff] [review]: ----------------------------------------------------------------- Please update the commit message to describe in more detail what the problem is how the patch fixes it. From looking at the code I don't see how this is a memory leak, because the CrossProcessCompositorBridgeParent should never be getting registered in the sIndirectLayerTrees structure at all. Instead what I would expect is that when CrossProcessCompositorBridgeParent::DeallocPWebRenderBridgeParent is called, it will call EraseLayerState with some garbage uninitialized value, and so it will erase some random layer state entry. Is that correct?
Assignee | ||
Comment 5•7 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #4) > > Please update the commit message to describe in more detail what the problem > is how the patch fixes it. > > From looking at the code I don't see how this is a memory leak, because the > CrossProcessCompositorBridgeParent should never be getting registered in the > sIndirectLayerTrees structure at all. Instead what I would expect is that > when CrossProcessCompositorBridgeParent::DeallocPWebRenderBridgeParent is > called, it will call EraseLayerState with some garbage uninitialized value, > and so it will erase some random layer state entry. Is that correct? Yes. I am going to update the commit message.
Assignee | ||
Comment 6•7 years ago
|
||
Updated the comment.
Attachment #8901720 -
Attachment is obsolete: true
Attachment #8901720 -
Flags: review?(bugmail)
Assignee | ||
Updated•7 years ago
|
Attachment #8902062 -
Flags: review?(bugmail)
Assignee | ||
Updated•7 years ago
|
Summary: Fix WebRenderBridgeParent::CreateDestroyed() → Fix uninitailzed mPipelineId by WebRenderBridgeParent::CreateDestroyed()
Assignee | ||
Updated•7 years ago
|
Attachment #8902062 -
Flags: review?(bugmail)
Assignee | ||
Comment 7•7 years ago
|
||
Constructor needs to be updated. https://treeherder.mozilla.org/#/jobs?repo=try&revision=9c56bf1d31ff60032bd02939ab7fc31d27b3f245
Assignee | ||
Comment 8•7 years ago
|
||
Attachment #8902062 -
Attachment is obsolete: true
Assignee | ||
Comment 9•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=79ea8300d99bbdb5ab8e30fd9c7fc49c445e275f
Assignee | ||
Updated•7 years ago
|
Attachment #8902073 -
Flags: review?(bugmail)
Comment 10•7 years ago
|
||
Comment on attachment 8902073 [details] [diff] [review] patch - Fix uninitailzed mPipelineId by WebRenderBridgeParent::CreateDestroyed() Review of attachment 8902073 [details] [diff] [review]: ----------------------------------------------------------------- s/uninitailzed/uninitialized/ in the commit message
Attachment #8902073 -
Flags: review?(bugmail) → review+
Comment 11•7 years ago
|
||
Pushed by sikeda@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/cf44180cc29a Fix uninitialized mPipelineId by WebRenderBridgeParent::CreateDestroyed() r=kats
Comment 12•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/cf44180cc29a
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•