Fix webrender Key leaks of WebRenderBridgeParent on abnormal shutdown

RESOLVED FIXED in Firefox 56

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: sotaro, Assigned: sotaro)

Tracking

unspecified
mozilla56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Assignee

Description

2 years ago
There are still some possible key leaks in WebRenderBridgeParent
Assignee

Updated

2 years ago
Assignee: nobody → sotaro.ikeda.g
Blocks: 1335335
Assignee

Updated

2 years ago
Attachment #8877082 - Flags: review?(nical.bugzilla)
Comment on attachment 8877082 [details] [diff] [review]
patch - Fix possible Key leaks of WebRenderBridgeParent

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

Same comment as in bug 1372066.
Attachment #8877082 - Flags: review?(nical.bugzilla) → review?(bugmail)
Comment on attachment 8877082 [details] [diff] [review]
patch - Fix possible Key leaks of WebRenderBridgeParent

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

Same comment as in bug 1372066 - please explain where the leak is coming from. Without a commit message or any description in the bug it's hard to say if this patch is valid and it's impossible to properly review.
Attachment #8877082 - Flags: review?(bugmail) → review-
Assignee

Updated

2 years ago
Summary: Fix possible Key leaks of WebRenderBridgeParent → Fix webrender Key leaks of WebRenderBridgeParent
Assignee

Comment 5

2 years ago
The leaks are very similar to Bug 1372066. The leak could happen on abnormal client shut down case and tab move case.
See Bug 1372066 Comment 9.

In the abnormal client shut down case, ImageKeys except keys of external image are leaked and FontKeys are leaked.

In the tab move case, the WebRenderBridgeParent will need to be re-bound to a different CompositorBridgeParent and webrender and so will need to clear all its active ImageKeys and FontKeys from the old webrender.
Assignee

Updated

2 years ago
Summary: Fix webrender Key leaks of WebRenderBridgeParent → Fix webrender Key leaks of WebRenderBridgeParent on abnormal shutdown
Assignee

Updated

2 years ago
Depends on: 1372066
Assignee

Updated

2 years ago
Attachment #8877935 - Flags: review?(bugmail)
Comment on attachment 8877935 [details] [diff] [review]
patch - Fix webrender Key leaks of WebRenderBridgeParent on abnormal shutdown

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

your commit message has a typo: s/webernder/webrender/

r=me with typo fixed and comment below addressed

::: gfx/layers/wr/WebRenderBridgeParent.h
@@ +247,5 @@
>    std::vector<wr::ImageKey> mKeysToDelete;
> +  // mActiveImageKeys and mFontKeys are used to avoid leaking animations when
> +  // WebRenderBridgeParent is destroyed abnormally and Tab move between different windows.
> +  nsDataHashtable<nsUint64HashKey, wr::ImageKey> mActiveImageKeys;
> +  nsDataHashtable<nsUint64HashKey, wr::FontKey> mFontKeys;

Again, use std::unordered_set<uint64_t> for both of these, don't need a hashtable.
Attachment #8877935 - Flags: review?(bugmail) → review+
Assignee

Comment 8

2 years ago
Addressed the comments.
Attachment #8877935 - Attachment is obsolete: true
Assignee

Updated

2 years ago
Attachment #8878299 - Flags: review+

Comment 10

2 years ago
Pushed by sikeda@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/706d0255f5a5
Fix webrender Key leaks of WebRenderBridgeParent on abnormal shutdown r=kats

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/706d0255f5a5
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.