1372512 - Fix webrender Key leaks of WebRenderBridgeParent on abnormal shutdown

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Sotaro Ikeda [:sotaro]]

There are still some possible key leaks in WebRenderBridgeParent

Comment 1

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Fix possible Key leaks of WebRenderBridgeParent

Comment 2

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=581f85e96cad7565c01ea4f2f6f1110b39170594

Comment 3

[Nicolas Silva [:nical]]

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.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 5

[Sotaro Ikeda [:sotaro]]

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.

Comment 6

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Fix webrender Key leaks of WebRenderBridgeParent on abnormal shutdown

Comment 7

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 8

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Fix webrender Key leaks of WebRenderBridgeParent on abnormal shutdown

Addressed the comments.

Comment 9

[Sotaro Ikeda [:sotaro]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=150861d343ef768d9b1cd515e913e8e72ee228cf

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/706d0255f5a5
Fix webrender Key leaks of WebRenderBridgeParent on abnormal shutdown r=kats

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/706d0255f5a5