cc: Remove the base RasterSource

cc: Remove the base RasterSource

We only have one raster source now, we can remove the base class.

Also, the helper is no longer needed because we only have one
implementation now.

Change-Id: I97c63f37a039ea31fa8eaa7b0c3e4f0e8bb6f8bf

BUG=536104
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/c7014b2e5b584ddd51e54189cb3e1a5f6970c502
Cr-Commit-Position: refs/heads/master@{#354915}

[hendrikw, 2015-10-15 02:38:16]

hendrikw@chromium.org changed reviewers:
+ vmpstr@chromium.org

[hendrikw, 2015-10-15 02:38:16]

PTAL

[ericrk, 2015-10-15 16:06:37]

ericrk@chromium.org changed reviewers:
+ ericrk@chromium.org

[ericrk, 2015-10-15 16:06:37]

Makes sense to me - one question though - given that we only have one
RasterSource, would it make sense to rename DisplayListRasterSource to
RasterSource for conciseness? WDYT?

[hendrikw, 2015-10-15 16:12:16]

On 2015/10/15 16:06:37, ericrk wrote:
> Makes sense to me - one question though - given that we only have one
> RasterSource, would it make sense to rename DisplayListRasterSource to
> RasterSource for conciseness? WDYT?

Yeah, that thought crossed my mind, and many of the functions are already named
that way.  We can do that in a follow up.

[vmpstr, 2015-10-15 17:33:32]

On 2015/10/15 16:06:37, ericrk wrote:
>> Makes sense to me - one question though - given that we only have one
>> RasterSource, would it make sense to rename DisplayListRasterSource to
>> RasterSource for conciseness? WDYT?

> Yeah, that thought crossed my mind, and many of the
> functions are already named
> that way.  We can do that in a follow up.

I'm not sure. I think we should keep it as DisplayListRasterSource for now,
since it's commonly referred to as that. We can maybe change that later once
slimming paint solidifies (dries?)

https://codereview.chromium.org/1405883002/diff/1/cc/playback/display_list_ra...
File cc/playback/display_list_raster_source.cc (right):

https://codereview.chromium.org/1405883002/diff/1/cc/playback/display_list_ra...
cc/playback/display_list_raster_source.cc:82: bool partial_update =
canvas_bitmap_rect != canvas_playback_rect;
Can you put this code in a helper function?

[hendrikw, 2015-10-15 17:43:18]

> We can maybe change that later once slimming paint solidifies (dries?)

good one!

https://codereview.chromium.org/1405883002/diff/1/cc/playback/display_list_ra...
File cc/playback/display_list_raster_source.cc (right):

https://codereview.chromium.org/1405883002/diff/1/cc/playback/display_list_ra...
cc/playback/display_list_raster_source.cc:82: bool partial_update =
canvas_bitmap_rect != canvas_playback_rect;
On 2015/10/15 17:33:32, vmpstr wrote:
> Can you put this code in a helper function?

Then we'd have one function with 80 lines (instead of 82), and a second function
with 2 lines.  If there were a clear way to split this down the middle
somewhere, I'd say yes, but in this case, I don't think a helper function will
add much clarity to this.

[vmpstr, 2015-10-15 17:55:26]

https://codereview.chromium.org/1405883002/diff/1/cc/playback/display_list_ra...
File cc/playback/display_list_raster_source.cc (right):

https://codereview.chromium.org/1405883002/diff/1/cc/playback/display_list_ra...
cc/playback/display_list_raster_source.cc:82: bool partial_update =
canvas_bitmap_rect != canvas_playback_rect;
On 2015/10/15 17:43:18, hendrikw wrote:
> On 2015/10/15 17:33:32, vmpstr wrote:
> > Can you put this code in a helper function?
> 
> Then we'd have one function with 80 lines (instead of 82), and a second
function
> with 2 lines.  If there were a clear way to split this down the middle
> somewhere, I'd say yes, but in this case, I don't think a helper function will
> add much clarity to this.

Oh, I don't mean that this function is too long. I meant more of a logical
split. For instance, seeing the difference between PlaybackToCanvas and
PlaybackToSharedCanvas is a lot easier if you just see one extra call. And based
on the name of the function, you can assume that it doesn't do anything like
translate the canvas (without restoring). I think it's still useful.

[hendrikw, 2015-10-17 00:49:54]

Respectfully disagree, making a function with a generic name that does a bunch
of random stuff for one other function is not the correct approach, IMO, but,
sometimes to get the L-G-T-M, you gotta do what you gotta do :)

As discussed offline, maybe this can be split up in the future, but probably
won't happen.

PTAL, thanks.

[vmpstr, 2015-10-19 19:00:01]

lgtm

https://codereview.chromium.org/1405883002/diff/20001/cc/playback/display_lis...
File cc/playback/display_list_raster_source.cc (right):

https://codereview.chromium.org/1405883002/diff/20001/cc/playback/display_lis...
cc/playback/display_list_raster_source.cc:88: void
DisplayListRasterSource::PrepareForPlaybackToCanvas(
nit: can you leave a TODO here to split it up?

[hendrikw, 2015-10-19 21:21:38]

The CQ bit was checked by hendrikw@chromium.org

[hendrikw, 2015-10-19 21:21:38]

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

[commit-bot: I haz the power, 2015-10-19 21:22:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1405883002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1405883002/40001

[commit-bot: I haz the power, 2015-10-19 21:26:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-19 21:26:51]

Try jobs failed on following builders:
  cast_shell_android on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_andr...)
  chromeos_daisy_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[hendrikw, 2015-10-19 22:13:56]

The CQ bit was checked by hendrikw@chromium.org

[hendrikw, 2015-10-19 22:13:56]

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

[commit-bot: I haz the power, 2015-10-19 22:14:53]

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

[commit-bot: I haz the power, 2015-10-19 23:45:17]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-10-19 23:46:10]

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