aura: Use platform independent skia::PlatformCanvas when possible.

aura: Use platform independent canvas by default.

BUG=111587
TEST=none


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=128039

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=128354

[Alexei Svitkine (slow), 2012-03-21 14:16:36]

LGTM++

[reveman, 2012-03-21 14:23:35]

This switches aura over to always using platform independent SkCanvas and
ui/gfx/canvas_skia.cc instead of ui/gfx/canvas_linux.cc.
--ui-enable-per-tile-painting should work properly with all aura builds once
this lands.

[piman, 2012-03-21 16:54:19]

https://chromiumcodereview.appspot.com/9722032/diff/2001/skia/skia.gyp
File skia/skia.gyp (right):

https://chromiumcodereview.appspot.com/9722032/diff/2001/skia/skia.gyp#newcod...
skia/skia.gyp:864: ['include', 'ext/platform_canvas_skia\\.cc$'],
Unless I missed something, this will compile and link platform_canvas_skia.cc on
all platforms which I don't think is what you want (see also your trybot error).
You don't need the 'include' line (since nothing else is explicitly excluding
it), but you can add a second stance in this condition block (the 'else' clause)
that excludes it.

[ 'use_aura==1' {
'sources/': [
  ['exclude', 'ext/platform_canvas_mac\\.cc$'],
  ['exclude', 'ext/platform_canvas_linux\\.cc$'],
  ['exclude', 'ext/platform_canvas_win\\.cc$'],
]
}, {
'sources/': [
  ['exclude', 'ext/platform_canvas_skia\\.cc$'],
]
}]

https://chromiumcodereview.appspot.com/9722032/diff/2001/ui/ui.gyp
File ui/ui.gyp (right):

https://chromiumcodereview.appspot.com/9722032/diff/2001/ui/ui.gyp#newcode376
ui/ui.gyp:376: ['use_canvas_skia==1 or use_aura==1', {
I think it would be more maintainable to set use_canvas_skia in common.gypi
under the use_aura==1 condition.

[Alexei Svitkine (slow), 2012-03-21 16:59:20]

https://chromiumcodereview.appspot.com/9722032/diff/2001/ui/ui.gyp
File ui/ui.gyp (right):

https://chromiumcodereview.appspot.com/9722032/diff/2001/ui/ui.gyp#newcode376
ui/ui.gyp:376: ['use_canvas_skia==1 or use_aura==1', {
On 2012/03/21 16:54:19, piman wrote:
> I think it would be more maintainable to set use_canvas_skia in common.gypi
> under the use_aura==1 condition.

Actually, I agree on this front. Was a little trigger-happy on the LGTM.

Also, maybe make it a separate change to turn use_canvas_skia=1 on by default
for aura.

[google-reveman, 2012-03-21 17:36:43]

> Also, maybe make it a separate change to turn use_canvas_skia=1 on by
> default for aura.

ok sounds good. I'll split this into two separate changes.

[reveman, 2012-03-21 18:18:17]

This is now only includes the skia::PlatformCanvas part.

[Alexei Svitkine (slow), 2012-03-21 18:28:23]

LGTM

[piman, 2012-03-21 18:40:17]

lgtm

[commit-bot: I haz the power, 2012-03-21 18:46:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/reveman@chromium.org/9722032/8004

[commit-bot: I haz the power, 2012-03-21 20:02:08]

Change committed as 128039

[reveman, 2012-03-21 21:20:59]

This needs to land after https://chromiumcodereview.appspot.com/9813017/ or
things break badly.

[Alexei Svitkine (slow), 2012-03-21 21:23:59]

https://chromiumcodereview.appspot.com/9722032/diff/1004/skia/skia.gyp
File skia/skia.gyp (right):

https://chromiumcodereview.appspot.com/9722032/diff/1004/skia/skia.gyp#newcod...
skia/skia.gyp:859: [ 'use_aura == 1', {
Maybe make this 'use_aura == 1 && use_canvas_skia == 1', so that things don't
break without https://chromiumcodereview.appspot.com/9813017/

[reveman, 2012-03-22 20:11:53]

hi, need a lgtm on latest patch. had to add some more ifdef'd code to
platform_canvas_skia.cc. I'm not happy about the current code duplication in
platform_canvas_skia.cc but I prefer to fix that in a separate CL when I have
more time. created this bug to track it: 
http://code.google.com/p/chromium/issues/detail?id=119555

[Alexei Svitkine (slow), 2012-03-22 20:16:46]

LGTM.

Can you add a comment to the file referencing the bug?

[google-reveman, 2012-03-22 20:19:35]

sure, I'll do that. somehow this just landed (don't remember asking
for it to be committed yet but that's fine..)

On Thu, Mar 22, 2012 at 4:16 PM,  <asvitkine@chromium.org> wrote:
> LGTM.
>
> Can you add a comment to the file referencing the bug?
>
> https://chromiumcodereview.appspot.com/9722032/

[reveman, 2012-03-22 20:30:51]

hasn't landed yet. I was confused by the earlier version having been reverted. I
added the comment that was asked for to the latest patch and will land asap.
sorry for the noise.

[commit-bot: I haz the power, 2012-03-22 22:41:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/reveman@chromium.org/9722032/14002

[commit-bot: I haz the power, 2012-03-23 00:13:22]

Change committed as 128354