added server-side breakpoint support to skiaserve

added server-side breakpoint support to skiaserve
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1707863003

Committed: https://skia.googlesource.com/skia/+/3ca1e1ae4fe710265056eadb086fc6ccc2caa57e

[ethannicholas, 2016-02-17 21:10:56]

Description was changed from

==========
added server-side breakpoint support to skiaserve
==========

to

==========
added server-side breakpoint support to skiaserve
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[ethannicholas, 2016-02-17 21:15:54]

ethannicholas@google.com changed reviewers:
+ jcgregorio@google.com, joshualitt@chromium.org

[ethannicholas, 2016-02-17 21:15:54]

This adds a command to find the next op in which a pixel changes. Request
/break/10/600/500 to find the first time after op 10 that the pixel at 600, 500
changes (I'm not in love with that URL format, but as far as I know we don't
have an actual URL parsing library and I didn't feel like mucking about with
query strings without one).

[jcgregorio, 2016-02-17 21:29:18]

I'm fine with the URL format.

https://codereview.chromium.org/1707863003/diff/1/tools/skiaserve/skiaserve.cpp
File tools/skiaserve/skiaserve.cpp (right):

https://codereview.chromium.org/1707863003/diff/1/tools/skiaserve/skiaserve.c...
tools/skiaserve/skiaserve.cpp:392: response["endOp"] =
Json::Value(Json::nullValue);
Can you make this -1? That way endOp is always a number value.

[ethannicholas, 2016-02-17 21:39:48]

On 2016/02/17 21:29:18, jcgregorio wrote:
> I'm fine with the URL format.
> 
>
https://codereview.chromium.org/1707863003/diff/1/tools/skiaserve/skiaserve.cpp
> File tools/skiaserve/skiaserve.cpp (right):
> 
>
https://codereview.chromium.org/1707863003/diff/1/tools/skiaserve/skiaserve.c...
> tools/skiaserve/skiaserve.cpp:392: response["endOp"] =
> Json::Value(Json::nullValue);
> Can you make this -1? That way endOp is always a number value.

Done, and while I was at it I figured I might as well always provide an
endColor, too.

[jcgregorio, 2016-02-17 21:42:07]

LGTM for the URI and JSON.

[jcgregorio, 2016-02-18 13:48:11]

https://codereview.chromium.org/1707863003/diff/20001/tools/skiaserve/skiaser...
File tools/skiaserve/skiaserve.cpp (right):

https://codereview.chromium.org/1707863003/diff/20001/tools/skiaserve/skiaser...
tools/skiaserve/skiaserve.cpp:394: for (int i = n + 1; i <
request->fDebugCanvas->getSize(); ++i) {
Actually can you wrap around? I.e. if you each the last op and the color hasn't
changed can you then look for a change in ops [0, n-1]? This would match what
currently happens when you reach the end of the list of ops.

[ethannicholas, 2016-02-18 17:33:05]

Added wraparound, fixed it not working properly on my desktop system.

[joshualitt, 2016-02-18 17:53:16]

On 2016/02/18 17:33:05, ethannicholas wrote:
> Added wraparound, fixed it not working properly on my desktop system.

nit + lgtm

[joshualitt, 2016-02-18 17:53:31]

joshualitt@google.com changed reviewers:
+ joshualitt@google.com

[joshualitt, 2016-02-18 17:53:31]

nit :(

https://codereview.chromium.org/1707863003/diff/40001/tools/skiaserve/skiaser...
File tools/skiaserve/skiaserve.cpp (right):

https://codereview.chromium.org/1707863003/diff/40001/tools/skiaserve/skiaser...
tools/skiaserve/skiaserve.cpp:353: static SkColor getPixel(Request* request, int
x, int y) {
GetPixel

[ethannicholas, 2016-02-18 17:59:38]

The CQ bit was checked by ethannicholas@google.com

[ethannicholas, 2016-02-18 17:59:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
jcgregorio@google.com, joshualitt@google.com
Link to the patchset: https://codereview.chromium.org/1707863003/#ps60001
(title: "fixed nit")

[commit-bot: I haz the power, 2016-02-18 17:59:53]

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

[commit-bot: I haz the power, 2016-02-18 18:22:36]

Description was changed from

==========
added server-side breakpoint support to skiaserve
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
added server-side breakpoint support to skiaserve
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/3ca1e1ae4fe710265056eadb086fc6ccc2caa57e
==========

[commit-bot: I haz the power, 2016-02-18 18:22:37]

Committed patchset #4 (id:60001) as
https://skia.googlesource.com/skia/+/3ca1e1ae4fe710265056eadb086fc6ccc2caa57e