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

Issue 546753003: Bugfix that cause skpdiff to segfault. (Closed)

Created:
6 years, 3 months ago by stephana
Modified:
6 years, 3 months ago
Reviewers:
mtklein, rmistry
CC:
reviews_skia.org, rmistry
Base URL:
https://skia.googlesource.com/skia.git@master
Project:
skia
Visibility:
Public.

Description

For certain input skpdiff segfaulted. This prevents the segfault and should fix the failed test in housekeeping. BUG=skia:2902 Committed: https://skia.googlesource.com/skia/+/7260d7292bde966f038b8e166f3fe41ddd8f2099

Patch Set 1 #

Total comments: 2

Patch Set 2 : Fixed tools self tests #

Total comments: 1

Messages

Total messages: 16 (6 generated)
stephana
https://codereview.chromium.org/546753003/diff/1/tools/skpdiff/SkDiffContext.cpp File tools/skpdiff/SkDiffContext.cpp (right): https://codereview.chromium.org/546753003/diff/1/tools/skpdiff/SkDiffContext.cpp#newcode292 tools/skpdiff/SkDiffContext.cpp:292: tg.wait(); This line should not be necessary, because wait() ...
6 years, 3 months ago (2014-09-05 17:03:21 UTC) #2
mtklein
https://codereview.chromium.org/546753003/diff/1/tools/skpdiff/SkDiffContext.cpp File tools/skpdiff/SkDiffContext.cpp (right): https://codereview.chromium.org/546753003/diff/1/tools/skpdiff/SkDiffContext.cpp#newcode292 tools/skpdiff/SkDiffContext.cpp:292: tg.wait(); On 2014/09/05 17:03:20, stephana wrote: > This line ...
6 years, 3 months ago (2014-09-05 17:18:51 UTC) #3
stephana
On 2014/09/05 17:18:51, mtklein wrote: > https://codereview.chromium.org/546753003/diff/1/tools/skpdiff/SkDiffContext.cpp > File tools/skpdiff/SkDiffContext.cpp (right): > > https://codereview.chromium.org/546753003/diff/1/tools/skpdiff/SkDiffContext.cpp#newcode292 > ...
6 years, 3 months ago (2014-09-05 18:38:16 UTC) #4
mtklein
https://codereview.chromium.org/546753003/diff/20001/tools/skpdiff/SkDiffContext.cpp File tools/skpdiff/SkDiffContext.cpp (right): https://codereview.chromium.org/546753003/diff/20001/tools/skpdiff/SkDiffContext.cpp#newcode287 tools/skpdiff/SkDiffContext.cpp:287: Ooh, I see. Try moving "SkTaskGroup tg;" down to ...
6 years, 3 months ago (2014-09-05 18:46:43 UTC) #5
stephana
On 2014/09/05 18:46:43, mtklein wrote: > https://codereview.chromium.org/546753003/diff/20001/tools/skpdiff/SkDiffContext.cpp > File tools/skpdiff/SkDiffContext.cpp (right): > > https://codereview.chromium.org/546753003/diff/20001/tools/skpdiff/SkDiffContext.cpp#newcode287 > ...
6 years, 3 months ago (2014-09-05 19:26:36 UTC) #6
stephana
I think this will turn the Housekeeping tests green. Ravi, could you review the python ...
6 years, 3 months ago (2014-09-05 19:52:25 UTC) #8
rmistry
LGTM
6 years, 3 months ago (2014-09-05 19:53:38 UTC) #9
mtklein
On 2014/09/05 19:26:36, stephana wrote: > On 2014/09/05 18:46:43, mtklein wrote: > > > https://codereview.chromium.org/546753003/diff/20001/tools/skpdiff/SkDiffContext.cpp ...
6 years, 3 months ago (2014-09-05 20:01:04 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/stephana@google.com/546753003/20001
6 years, 3 months ago (2014-09-05 20:06:10 UTC) #15
commit-bot: I haz the power
6 years, 3 months ago (2014-09-05 20:21:46 UTC) #16
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as 7260d7292bde966f038b8e166f3fe41ddd8f2099

Powered by Google App Engine
This is Rietveld 408576698