Changed SoftwareOutputDevice interface.

Changed SoftwareOutputDevice interface.
+ Some of the necessary changes to the renderer for software compositing.


BUG=124671, 161008
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=186310

[slavi, 2013-03-01 21:54:24]

Those are the renderer side changes from the software compositing for Aura patch
https://codereview.chromium.org/12340015/.

The rest of the changes will be landed in another patch.

[Ken Russell (switch to Gerrit), 2013-03-01 23:17:09]

LGTM

https://codereview.chromium.org/12379055/diff/1/cc/software_output_device.cc
File cc/software_output_device.cc (right):

https://codereview.chromium.org/12379055/diff/1/cc/software_output_device.cc#...
cc/software_output_device.cc:54: NOTIMPLEMENTED();
Should there be some way to inform callers that this and ReclaimDIB can't be
called? Or should these methods just be removed, or made protected?

[slavi, 2013-03-01 23:28:55]

On 2013/03/01 23:17:09, kbr wrote:
> LGTM
> 
> https://codereview.chromium.org/12379055/diff/1/cc/software_output_device.cc
> File cc/software_output_device.cc (right):
> 
>
https://codereview.chromium.org/12379055/diff/1/cc/software_output_device.cc#...
> cc/software_output_device.cc:54: NOTIMPLEMENTED();
> Should there be some way to inform callers that this and ReclaimDIB can't be
> called? Or should these methods just be removed, or made protected?

ReclaimDIB should be called only from the renderer process when the browser
Ack-s a new dib and gives us back the previous one for recycling.
Since this is just a "hack" that will be replaced by UberComp, I would suggest
keeping it simple as it is.

[enne (OOO), 2013-03-01 23:51:34]

https://codereview.chromium.org/12379055/diff/1/cc/software_renderer.cc
File cc/software_renderer.cc (right):

https://codereview.chromium.org/12379055/diff/1/cc/software_renderer.cc#newco...
cc/software_renderer.cc:135: m_outputDevice->ReclaimDIB(ack.last_content_dib);
It seems weird to land this function that calls ReclaimDIB which is NOTREACHED. 
I assume this is just a wart of landing a small part of a larger patch sequence,
but what's the larger context of this change?

[slavi, 2013-03-02 00:12:09]

On 2013/03/01 23:51:34, enne wrote:
> https://codereview.chromium.org/12379055/diff/1/cc/software_renderer.cc
> File cc/software_renderer.cc (right):
> 
>
https://codereview.chromium.org/12379055/diff/1/cc/software_renderer.cc#newco...
> cc/software_renderer.cc:135: m_outputDevice->ReclaimDIB(ack.last_content_dib);
> It seems weird to land this function that calls ReclaimDIB which is
NOTREACHED. 
> I assume this is just a wart of landing a small part of a larger patch
sequence,
> but what's the larger context of this change?

https://codereview.chromium.org/12340015/

[enne (OOO), 2013-03-02 02:55:58]

"Big patch." So very informative.  ;)

So the short of it is that you've got a derived class that will implement that
soon enough and this is only a transitory state.

webkit/compositor_bindings lgtm

[jamesr, 2013-03-05 21:41:30]

cc/ lgtm

https://codereview.chromium.org/12379055/diff/17001/cc/picture_layer_impl_uni...
File cc/picture_layer_impl_unittest.cc (right):

https://codereview.chromium.org/12379055/diff/17001/cc/picture_layer_impl_uni...
cc/picture_layer_impl_unittest.cc:14: #include
"third_party/skia/include/core/SkDevice.h"
Do you need this new include?

[slavi, 2013-03-05 21:47:50]

https://codereview.chromium.org/12379055/diff/17001/cc/picture_layer_impl_uni...
File cc/picture_layer_impl_unittest.cc (right):

https://codereview.chromium.org/12379055/diff/17001/cc/picture_layer_impl_uni...
cc/picture_layer_impl_unittest.cc:14: #include
"third_party/skia/include/core/SkDevice.h"
On 2013/03/05 21:41:30, jamesr wrote:
> Do you need this new include?

Yes, fails to compile without it.

This file is indeed a user of SkDevice but didn't include its header. I guess
that I changed the include to forward decl somewhere in its headers.

[commit-bot: I haz the power, 2013-03-05 22:50:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/skaslev@chromium.org/12379055/20004

[commit-bot: I haz the power, 2013-03-06 01:01:59]

Change committed as 186310

[danakj, 2013-03-06 01:07:36]

FYI: A more descriptive changelog/description would be very valuable here.