Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(218)

Issue 930503002: cc: Make raster source members const (Closed)

Created:
5 years, 10 months ago by enne (OOO)
Modified:
5 years, 10 months ago
Reviewers:
danakj
CC:
ajuma, cc-bugs_chromium.org, chromium-reviews, reveman, vmiura, vmpstr
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

cc: Make raster source members const Both PicturePileImpl and DisplayListRasterSource get used on multiple threads, so should not be mutable. Unfortunately, many tests try to mutate either the recordings or the state of PicturePileImpl, so they all need to be changed to touch the recording source and a bunch of state setters need to be moved from raster source to recording source. Committed: https://crrev.com/ffe5781a8645d2fcb9f15a218aca4a3be5fe452c Cr-Commit-Position: refs/heads/master@{#316364}

Patch Set 1 #

Patch Set 2 : Display lists too #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+481 lines, -366 lines) Patch
M cc/layers/picture_layer.cc View 2 chunks +4 lines, -3 lines 0 comments Download
M cc/layers/picture_layer_impl_unittest.cc View 8 chunks +39 lines, -25 lines 0 comments Download
M cc/output/renderer_pixeltest.cc View 9 chunks +63 lines, -41 lines 0 comments Download
M cc/resources/display_list_raster_source.h View 1 3 chunks +15 lines, -14 lines 0 comments Download
M cc/resources/display_list_raster_source.cc View 2 chunks +2 lines, -10 lines 0 comments Download
M cc/resources/display_list_recording_source.h View 2 chunks +4 lines, -0 lines 0 comments Download
M cc/resources/display_list_recording_source.cc View 2 chunks +10 lines, -0 lines 0 comments Download
M cc/resources/picture_pile.h View 2 chunks +5 lines, -0 lines 0 comments Download
M cc/resources/picture_pile.cc View 3 chunks +17 lines, -0 lines 0 comments Download
M cc/resources/picture_pile_impl.h View 3 chunks +17 lines, -16 lines 0 comments Download
M cc/resources/picture_pile_impl.cc View 6 chunks +6 lines, -27 lines 1 comment Download
M cc/resources/picture_pile_impl_unittest.cc View 19 chunks +130 lines, -83 lines 0 comments Download
M cc/resources/raster_source.h View 1 chunk +0 lines, -3 lines 0 comments Download
M cc/resources/recording_source.h View 1 chunk +2 lines, -0 lines 0 comments Download
M cc/test/fake_picture_pile.h View 4 chunks +44 lines, -0 lines 1 comment Download
M cc/test/fake_picture_pile.cc View 1 chunk +89 lines, -0 lines 0 comments Download
M cc/test/fake_picture_pile_impl.h View 4 chunks +10 lines, -49 lines 0 comments Download
M cc/test/fake_picture_pile_impl.cc View 6 chunks +24 lines, -95 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
enne (OOO)
So much plumbing and rearranging for just a few consts, but hopefully this will prevent ...
5 years, 10 months ago (2015-02-14 00:14:40 UTC) #2
danakj
LGTM https://codereview.chromium.org/930503002/diff/20001/cc/resources/picture_pile_impl.cc File cc/resources/picture_pile_impl.cc (left): https://codereview.chromium.org/930503002/diff/20001/cc/resources/picture_pile_impl.cc#oldcode43 cc/resources/picture_pile_impl.cc:43: clear_canvas_with_debug_color_(kDefaultClearCanvasSetting), I think you could have left this ...
5 years, 10 months ago (2015-02-14 01:34:17 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/930503002/20001
5 years, 10 months ago (2015-02-14 01:38:23 UTC) #5
commit-bot: I haz the power
Committed patchset #2 (id:20001)
5 years, 10 months ago (2015-02-14 02:37:28 UTC) #6
commit-bot: I haz the power
5 years, 10 months ago (2015-02-14 02:38:14 UTC) #7
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/ffe5781a8645d2fcb9f15a218aca4a3be5fe452c
Cr-Commit-Position: refs/heads/master@{#316364}

Powered by Google App Engine
This is Rietveld 408576698