Share backing canvases between browser compositors.

Share backing canvases between browser compositors.

The software compositor's canvas is only used between BeginPaint and EndPaint. Those happen consecutively for a compositor, so we can share a single buffer to back multiple canvases, one per compositor. This cuts down on memory used to that necessary to contain just the largest window of the process.

Committed: https://crrev.com/8fe39ccf33d9f1a2d785c4deebce4cc7afad7d1e
Cr-Commit-Position: refs/heads/master@{#329525}

[jbauman, 2015-05-11 22:15:46]

jbauman@chromium.org changed reviewers:
+ piman@chromium.org

[piman, 2015-05-11 23:20:57]

Mostly nits, otherwise LGTM.

https://codereview.chromium.org/1132133004/diff/20001/content/browser/composi...
File content/browser/compositor/software_output_device_win.cc (right):

https://codereview.chromium.org/1132133004/diff/20001/content/browser/composi...
content/browser/compositor/software_output_device_win.cc:24: static
SharedMemoryBacking* GetInstance() {
nit: could the SharedMemoryBacking be owned by the GpuProcessTransportFactory
and passed to the SoftwareOutputDeviceWin? Avoiding globals can be helpful for
testing.

https://codereview.chromium.org/1132133004/diff/20001/content/browser/composi...
content/browser/compositor/software_output_device_win.cc:126: if
(!is_hwnd_composited_)
nit: needs {}

https://codereview.chromium.org/1132133004/diff/20001/content/browser/composi...
content/browser/compositor/software_output_device_win.cc:176: void
SoftwareOutputDeviceWin::ReleaseContents() {
nit: any way to DCHECK we're not between BeginPaint and EndPaint?

[jbauman, 2015-05-12 02:20:29]

On 2015/05/11 23:20:57, piman (Very slow to review) wrote:
> Mostly nits, otherwise LGTM.
> 
>
https://codereview.chromium.org/1132133004/diff/20001/content/browser/composi...
> File content/browser/compositor/software_output_device_win.cc (right):
> 
>
https://codereview.chromium.org/1132133004/diff/20001/content/browser/composi...
> content/browser/compositor/software_output_device_win.cc:24: static
> SharedMemoryBacking* GetInstance() {
> nit: could the SharedMemoryBacking be owned by the GpuProcessTransportFactory
> and passed to the SoftwareOutputDeviceWin? Avoiding globals can be helpful for
> testing.
> 
That works.

>
https://codereview.chromium.org/1132133004/diff/20001/content/browser/composi...
> content/browser/compositor/software_output_device_win.cc:126: if
> (!is_hwnd_composited_)
> nit: needs {}
> 
>
https://codereview.chromium.org/1132133004/diff/20001/content/browser/composi...
> content/browser/compositor/software_output_device_win.cc:176: void
> SoftwareOutputDeviceWin::ReleaseContents() {
> nit: any way to DCHECK we're not between BeginPaint and EndPaint?

Sure, added another variable so we can check that.

[piman, 2015-05-12 03:03:51]

lgtm

https://codereview.chromium.org/1132133004/diff/60001/content/browser/composi...
File content/browser/compositor/software_output_device_win.cc (right):

https://codereview.chromium.org/1132133004/diff/60001/content/browser/composi...
content/browser/compositor/software_output_device_win.cc:75: // TODO(skaslev)
Remove this when crbug.com/180702 is fixed.
nit: BTW you can remove this comment now, since that bug is WontFix.

[jbauman, 2015-05-12 22:02:12]

The CQ bit was checked by jbauman@chromium.org

[jbauman, 2015-05-12 22:02:12]

The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1132133004/#ps80001
(title: " ")

[commit-bot: I haz the power, 2015-05-12 22:04:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1132133004/80001

[commit-bot: I haz the power, 2015-05-12 23:02:21]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-05-12 23:04:17]

Patchset 5 (id:??) landed as
https://crrev.com/8fe39ccf33d9f1a2d785c4deebce4cc7afad7d1e
Cr-Commit-Position: refs/heads/master@{#329525}