Make delegated software renderer work on Mac

Make delegated software renderer work on Mac

Add a device scale factor parameter to SoftwareOutputDevice::Resize and
plumb that parameter through to the contentsScale property of the CALayer
that is used to draw the software frame.

Change the way that RWHVMac draws software frames (both delegated and
non-delegated) so that the CALayer for the software frame has setContents
called on it with a CGImageRef of the software frame, instead of drawing
the CGImageRef to the inside the CALayer's displayInContext function. This
is the way that things should have been done to begin with (it involves a lot
less copying and a lot more idle time in the browser main thread), but wasn't
compatible with the (now-defunct) legacy software path.

Change the software rendering path to set the contentsScale of the software
CALayer at the time that the software frame is received, when the contents
are updated, instead of in RWHVMac::LayoutLayers.

BUG=314190
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=271839

[ccameron, 2014-05-20 06:34:27]

ptal

https://codereview.chromium.org/297573003/diff/40001/cc/output/software_outpu...
File cc/output/software_output_device.h (right):

https://codereview.chromium.org/297573003/diff/40001/cc/output/software_outpu...
cc/output/software_output_device.h:78: gfx::Size viewport_size_;
I was considering renaming all instances of this to viewport_pixel_size_, let me
know if you think that should be done.

[piman, 2014-05-20 07:04:17]

https://codereview.chromium.org/297573003/diff/40001/cc/output/software_outpu...
File cc/output/software_output_device.h (right):

https://codereview.chromium.org/297573003/diff/40001/cc/output/software_outpu...
cc/output/software_output_device.h:78: gfx::Size viewport_size_;
On 2014/05/20 06:34:27, ccameron1 wrote:
> I was considering renaming all instances of this to viewport_pixel_size_, let
me
> know if you think that should be done.

It might be a good idea indeed, so that it's obvious at the user sites.

https://codereview.chromium.org/297573003/diff/40001/content/browser/composit...
File content/browser/compositor/software_output_device_mac.h (right):

https://codereview.chromium.org/297573003/diff/40001/content/browser/composit...
content/browser/compositor/software_output_device_mac.h:28: ui::Compositor*
compositor_;
nit: DISALLOW_COPY_AND_ASSIGN

https://codereview.chromium.org/297573003/diff/40001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

https://codereview.chromium.org/297573003/diff/40001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_mac.mm:1593: NULL, pixels,
4 * info.fWidth * info.fHeight, NULL));
Doesn't this assume that CoreAnimation won't look at the pixels after this
function returns? Is it a valid assumption?

After the function returns, pixels may become invalid at any point, e.g. if
canvas gets resized.

Do you want to keep a reference on |canvas| maybe?

[ccameron, 2014-05-20 19:15:50]

Thanks! ptal

https://codereview.chromium.org/297573003/diff/40001/cc/output/software_outpu...
File cc/output/software_output_device.h (right):

https://codereview.chromium.org/297573003/diff/40001/cc/output/software_outpu...
cc/output/software_output_device.h:78: gfx::Size viewport_size_;
On 2014/05/20 07:04:17, piman wrote:
> On 2014/05/20 06:34:27, ccameron1 wrote:
> > I was considering renaming all instances of this to viewport_pixel_size_,
let
> me
> > know if you think that should be done.
> 
> It might be a good idea indeed, so that it's obvious at the user sites.

Done.

https://codereview.chromium.org/297573003/diff/40001/content/browser/composit...
File content/browser/compositor/software_output_device_mac.h (right):

https://codereview.chromium.org/297573003/diff/40001/content/browser/composit...
content/browser/compositor/software_output_device_mac.h:28: ui::Compositor*
compositor_;
On 2014/05/20 07:04:17, piman wrote:
> nit: DISALLOW_COPY_AND_ASSIGN

Added to Mac, Win, and X11 (only Ozone had it).

https://codereview.chromium.org/297573003/diff/40001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

https://codereview.chromium.org/297573003/diff/40001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_mac.mm:1593: NULL, pixels,
4 * info.fWidth * info.fHeight, NULL));
On 2014/05/20 07:04:17, piman wrote:
> Doesn't this assume that CoreAnimation won't look at the pixels after this
> function returns? Is it a valid assumption?
> 
> After the function returns, pixels may become invalid at any point, e.g. if
> canvas gets resized.
> 
> Do you want to keep a reference on |canvas| maybe?

The documentation is very unclear, but this does appear to be the case.

Actually, we need to make a copy of the data, because the we may re-use the data
while the compositor is scribbling on the buffer for the next frame (we get the
same buffer every frame).

Coalesced these calls into a single method on SoftwareLayer.

[piman, 2014-05-20 21:00:30]

lgtm

https://codereview.chromium.org/297573003/diff/40001/content/browser/composit...
File content/browser/compositor/software_output_device_mac.h (right):

https://codereview.chromium.org/297573003/diff/40001/content/browser/composit...
content/browser/compositor/software_output_device_mac.h:28: ui::Compositor*
compositor_;
On 2014/05/20 19:15:50, ccameron1 wrote:
> On 2014/05/20 07:04:17, piman wrote:
> > nit: DISALLOW_COPY_AND_ASSIGN
> 
> Added to Mac, Win, and X11 (only Ozone had it).

Thanks!

[ccameron, 2014-05-20 21:02:31]

Thanks!

[ccameron, 2014-05-20 21:02:36]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2014-05-20 21:05:23]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ccameron@chromium.org/297573003/80001

[Ken Russell (switch to Gerrit), 2014-05-20 21:32:36]

Nice work. LGTM FWIW.

[commit-bot: I haz the power, 2014-05-20 22:07:34]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[commit-bot: I haz the power, 2014-05-21 03:21:08]

Change committed as 271839