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

Issue 24826002: Allow creating a picture from skp to fail. (Closed)

Created:
7 years, 2 months ago by scroggo
Modified:
7 years, 2 months ago
Reviewers:
mtklein, caryclark, reed1
CC:
skia-review_googlegroups.com
Visibility:
Public.

Description

Allow creating a picture from skp to fail. Replace the current constructor for creating an SkPicturePlayback to a factory. In the factory, check for incorrect data that would result in an invalid playback. If the playback is invalid, return NULL, and return NULL from SkPicture's factory as well. Update SkTimedPicture(Playback) as well. BUG=skia:1672 R=caryclark@google.com Committed: https://code.google.com/p/skia/source/detail?r=11554

Patch Set 1 #

Total comments: 2

Patch Set 2 : Interesting bit - return false on error. #

Total comments: 4

Patch Set 3 : Use a factory #

Patch Set 4 : Fix a typo #

Patch Set 5 : Fix debugger. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+92 lines, -30 lines) Patch
M debugger/QT/SkDebuggerGUI.cpp View 1 2 3 4 3 chunks +27 lines, -6 lines 0 comments Download
M src/core/SkPicture.cpp View 1 2 1 chunk +4 lines, -1 line 0 comments Download
M src/core/SkPicturePlayback.h View 1 2 3 4 3 chunks +6 lines, -3 lines 0 comments Download
M src/core/SkPicturePlayback.cpp View 1 2 3 4 6 chunks +55 lines, -20 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
scroggo
https://codereview.chromium.org/24826002/diff/1/debugger/QT/SkDebuggerGUI.cpp File debugger/QT/SkDebuggerGUI.cpp (right): https://codereview.chromium.org/24826002/diff/1/debugger/QT/SkDebuggerGUI.cpp#newcode1 debugger/QT/SkDebuggerGUI.cpp:1: /* Patchset 1 reverts r7844: "Remove bogus ability for ...
7 years, 2 months ago (2013-09-26 21:06:28 UTC) #1
caryclark
https://codereview.chromium.org/24826002/diff/3001/src/core/SkPicture.cpp File src/core/SkPicture.cpp (right): https://codereview.chromium.org/24826002/diff/3001/src/core/SkPicture.cpp#newcode313 src/core/SkPicture.cpp:313: bool isValid = false; It looks like this argues ...
7 years, 2 months ago (2013-09-26 21:12:46 UTC) #2
caryclark
7 years, 2 months ago (2013-09-26 21:12:48 UTC) #3
reed1
https://codereview.chromium.org/24826002/diff/3001/src/core/SkPicture.cpp File src/core/SkPicture.cpp (right): https://codereview.chromium.org/24826002/diff/3001/src/core/SkPicture.cpp#newcode313 src/core/SkPicture.cpp:313: bool isValid = false; On 2013/09/26 21:12:46, caryclark wrote: ...
7 years, 2 months ago (2013-09-26 21:16:06 UTC) #4
scroggo
https://codereview.chromium.org/24826002/diff/3001/src/core/SkPicture.cpp File src/core/SkPicture.cpp (right): https://codereview.chromium.org/24826002/diff/3001/src/core/SkPicture.cpp#newcode313 src/core/SkPicture.cpp:313: bool isValid = false; On 2013/09/26 21:16:07, reed1 wrote: ...
7 years, 2 months ago (2013-09-26 22:17:46 UTC) #5
caryclark
On 2013/09/26 22:17:46, scroggo wrote: > https://codereview.chromium.org/24826002/diff/3001/src/core/SkPicture.cpp > File src/core/SkPicture.cpp (right): > > https://codereview.chromium.org/24826002/diff/3001/src/core/SkPicture.cpp#newcode313 > ...
7 years, 2 months ago (2013-09-27 11:50:00 UTC) #6
scroggo
Latest patch replaces the constructor with a factory.
7 years, 2 months ago (2013-09-30 21:26:37 UTC) #7
caryclark
lgtm
7 years, 2 months ago (2013-10-01 12:42:09 UTC) #8
scroggo
7 years, 2 months ago (2013-10-01 15:31:06 UTC) #9
Message was sent while issue was closed.
Committed patchset #5 manually as r11554 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698