Fix WebRenderLayerManager::FlushRendering()

RESOLVED FIXED in Firefox 55

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: sotaro, Assigned: sotaro)

Tracking

(Blocks 1 bug)

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

Assignee

Description

2 years ago
It seems necessary to fix  WebRenderLayerManager::FlushRendering() work like ClientLayerManager::FlushRendering().
Assignee

Updated

2 years ago
Assignee: nobody → sotaro.ikeda.g
Blocks: webrender
Here you have changed something WebRender specific and it has to do with mozilla::ipc::IPCResult. I had sometimes crashes with Ctrl+W tab closing when webrender was enabled. bp-0c38dcef-1d98-4336-b9dc-5fb310170510 (bug 1354198). Maybe this is the fix? :O
Assignee

Updated

2 years ago
See Also: → 1361257
Assignee

Comment 3

2 years ago
(In reply to Darkspirit from comment #2)
> Here you have changed something WebRender specific and it has to do with
> mozilla::ipc::IPCResult. I had sometimes crashes with Ctrl+W tab closing
> when webrender was enabled. bp-0c38dcef-1d98-4336-b9dc-5fb310170510 (bug
> 1354198). Maybe this is the fix? :O

I looked into Bug 1354198, it seems like a different problem.
Assignee

Comment 4

2 years ago
Rebased.
Attachment #8866643 - Attachment is obsolete: true
Assignee

Comment 5

2 years ago
Attachment #8871168 - Attachment is obsolete: true
Assignee

Updated

2 years ago
Attachment #8872491 - Flags: review?(nical.bugzilla)
Comment on attachment 8872491 [details] [diff] [review]
patch - Fix WebRenderLayerManager::FlushRendering()

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

::: gfx/layers/wr/WebRenderBridgeParent.cpp
@@ +937,5 @@
>    }
>  }
>  
>  void
> +WebRenderBridgeParent::FlushRendering(bool aSync)

nit: (very lame nit, sorry): my eyes always trip on the convention that forces us to put 'a' before 'Sync' for arguments which make it look like the exact opposite word (async). "aIsSync" or "aForceSync" are less elegant but they don't cause me to misinterpret what I read. Our layers code is filled with aSync parameters so it's not a problem of this patch, but it'd be nice if we took on the habit of naming this parameter some way that is harder to misread (or maybe I'll just get used to it eventually).
Attachment #8872491 - Flags: review?(nical.bugzilla) → review+
Assignee

Updated

2 years ago
Attachment #8872491 - Attachment is obsolete: true

Comment 9

2 years ago
Pushed by sikeda@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/63cd7aaf05d8
Fix WebRenderLayerManager::FlushRendering() r=nical

Comment 10

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