Tick off some TODOs:

Tick off some TODOs:

  - support fRecord in copy constructor
  - support SkDrawPictureCallback

Moved SkDrawPictureCallback to its own header so
SkRecordDraw can include it without pulling in all of
SkPicture.

Adding an SkAutoSaveRestore to SkRecordDraw was the easiest
way to match the balance guarantees of the callback, and
probably not a bad idea in general.  Updated its tests.

BUG=skia:
R=robertphillips@google.com

Committed: https://skia.googlesource.com/skia/+/c11530e

[robertphillips, 2014-06-24 15:11:08]

lgtm + nits & suggestions

https://codereview.chromium.org/349973008/diff/40001/src/core/SkPicture.cpp
File src/core/SkPicture.cpp (right):

https://codereview.chromium.org/349973008/diff/40001/src/core/SkPicture.cpp#n...
src/core/SkPicture.cpp:169: 
NULL != ? (and above?)

https://codereview.chromium.org/349973008/diff/40001/src/core/SkPicture.cpp#n...
src/core/SkPicture.cpp:511: #ifdef SK_BUILD_FOR_ANDROID
// fix by updating Android to not use it and then deleting this method ?

https://codereview.chromium.org/349973008/diff/40001/src/core/SkRecordDraw.cpp
File src/core/SkRecordDraw.cpp (right):

https://codereview.chromium.org/349973008/diff/40001/src/core/SkRecordDraw.cp...
src/core/SkRecordDraw.cpp:10: void SkRecordDraw(const SkRecord& record,
SkCanvas* canvas, SkDrawPictureCallback* callback) {
maybe:
static const bool kBracketWithAdditionalSaveAndRestore = true;
SkAutoCanvasRestore saveRestore(canvas, kBracketWithAdditionalSaveAndRestore);
?

https://codereview.chromium.org/349973008/diff/40001/src/core/SkRecordDraw.cp...
src/core/SkRecordDraw.cpp:12: for (SkRecords::Draw draw(canvas); draw.index() <
record.count(); draw.next()) {
NULL != ?

https://codereview.chromium.org/349973008/diff/40001/tests/RecordDrawTest.cpp
File tests/RecordDrawTest.cpp (right):

https://codereview.chromium.org/349973008/diff/40001/tests/RecordDrawTest.cpp...
tests/RecordDrawTest.cpp:51: 
Remove this SkDebugf?

[mtklein, 2014-06-24 15:16:47]

https://codereview.chromium.org/349973008/diff/40001/src/core/SkPicture.cpp
File src/core/SkPicture.cpp (right):

https://codereview.chromium.org/349973008/diff/40001/src/core/SkPicture.cpp#n...
src/core/SkPicture.cpp:169: 
On 2014/06/24 15:11:08, robertphillips wrote:
> NULL != ? (and above?)

Done.

https://codereview.chromium.org/349973008/diff/40001/src/core/SkPicture.cpp#n...
src/core/SkPicture.cpp:511: #ifdef SK_BUILD_FOR_ANDROID
On 2014/06/24 15:11:08, robertphillips wrote:
> // fix by updating Android to not use it and then deleting this method ?

Done.

https://codereview.chromium.org/349973008/diff/40001/src/core/SkRecordDraw.cpp
File src/core/SkRecordDraw.cpp (right):

https://codereview.chromium.org/349973008/diff/40001/src/core/SkRecordDraw.cp...
src/core/SkRecordDraw.cpp:10: void SkRecordDraw(const SkRecord& record,
SkCanvas* canvas, SkDrawPictureCallback* callback) {
On 2014/06/24 15:11:08, robertphillips wrote:
> maybe:
> static const bool kBracketWithAdditionalSaveAndRestore = true;
> SkAutoCanvasRestore saveRestore(canvas, kBracketWithAdditionalSaveAndRestore);
> ?

Done.

https://codereview.chromium.org/349973008/diff/40001/src/core/SkRecordDraw.cp...
src/core/SkRecordDraw.cpp:12: for (SkRecords::Draw draw(canvas); draw.index() <
record.count(); draw.next()) {
On 2014/06/24 15:11:08, robertphillips wrote:
> NULL != ?

Done.

https://codereview.chromium.org/349973008/diff/40001/tests/RecordDrawTest.cpp
File tests/RecordDrawTest.cpp (right):

https://codereview.chromium.org/349973008/diff/40001/tests/RecordDrawTest.cpp...
tests/RecordDrawTest.cpp:51: 
On 2014/06/24 15:11:08, robertphillips wrote:
> Remove this SkDebugf?

Done.

[mtklein_C, 2014-06-24 15:29:10]

Committed patchset #4 manually as rc11530e.