add basic HTTP server for viewing skpdiff results

add basic HTTP server for viewing skpdiff results

R=epoger@google.com

Committed: https://code.google.com/p/skia/source/detail?r=10371

[Zach Reizner, 2013-07-23 22:12:20]

This is a simple Python HTTP web server I cooked up that will serve skpdiff
results. It will be the basis of user interactivity with rebaseling.

[epoger, 2013-07-24 17:46:53]

https://codereview.chromium.org/19929007/diff/1/tools/skpdiff/skpdiff_server.py
File tools/skpdiff/skpdiff_server.py (right):

https://codereview.chromium.org/19929007/diff/1/tools/skpdiff/skpdiff_server....
tools/skpdiff/skpdiff_server.py:1: #!/usr/bin/python
I don't remember where this is documented, but for Python our style guide calls
for 80 column wrap and 2-space indents.  Please reformat.

https://codereview.chromium.org/19929007/diff/1/tools/skpdiff/skpdiff_server....
tools/skpdiff/skpdiff_server.py:23: if len(extension) >= 1:
FYI: I don't care one way or the other, but I think many Python reviewers would
say just "if extension:" is more Pythony.

https://codereview.chromium.org/19929007/diff/1/tools/skpdiff/skpdiff_server....
tools/skpdiff/skpdiff_server.py:38: # Determine if the file exists relative to
the given dir_path AND exists under the dir_path
Please add a note indicating the security reasons for making this check.

https://codereview.chromium.org/19929007/diff/1/tools/skpdiff/skpdiff_server....
tools/skpdiff/skpdiff_server.py:55: # The [1:] chops of the leading '/'
of -> off

https://codereview.chromium.org/19929007/diff/1/tools/skpdiff/skpdiff_server....
tools/skpdiff/skpdiff_server.py:62: # WARNING: Using the root is a big ol' hack.
Incredibly insecure. Only allow serving to
Indeed it is.  Is there some other way we can handle this, without such
exposure?

One thing to note is that the client's browser can grab images directly from
Google Storage via URLs like this:
http://chromium-skia-gm.commondatastorage.googleapis.com/gm/bitmap-64bitMD5/a...

Maybe you could do that instead?  Let's discuss live...

[Zach Reizner, 2013-07-24 20:16:36]

https://codereview.chromium.org/19929007/diff/1/tools/skpdiff/skpdiff_server.py
File tools/skpdiff/skpdiff_server.py (right):

https://codereview.chromium.org/19929007/diff/1/tools/skpdiff/skpdiff_server....
tools/skpdiff/skpdiff_server.py:1: #!/usr/bin/python
On 2013/07/24 17:46:53, epoger wrote:
> I don't remember where this is documented, but for Python our style guide
calls
> for 80 column wrap and 2-space indents.  Please reformat.

According to http://www.python.org/dev/peps/pep-0008/#indentation, python
indentation should always be a multiple of 4. In addition rebaseline.py,
svndiff.py, and jsondiff.py all use 4 space indents.

[epoger, 2013-07-24 22:00:25]

https://codereview.chromium.org/19929007/diff/9001/tools/skpdiff/skpdiff_serv...
File tools/skpdiff/skpdiff_server.py (right):

https://codereview.chromium.org/19929007/diff/9001/tools/skpdiff/skpdiff_serv...
tools/skpdiff/skpdiff_server.py:77: server_address = ('127.0.0.1', 8080)
Please add a note indicating the security problem that will follow if someone
binds to a server_address allowing access from machines other than localhost.

[djsollen, 2013-07-25 12:02:56]

https://codereview.chromium.org/19929007/diff/9001/tools/skpdiff/skpdiff_serv...
File tools/skpdiff/skpdiff_server.py (right):

https://codereview.chromium.org/19929007/diff/9001/tools/skpdiff/skpdiff_serv...
tools/skpdiff/skpdiff_server.py:69: if self.serve_if_in_dir('/', file_path):
aren't we going to start the server after we know the directories that the
images are in?  Why not pass those params to the server at startup and only
serve from those directories?

[Zach Reizner, 2013-07-25 13:24:46]

https://codereview.chromium.org/19929007/diff/9001/tools/skpdiff/skpdiff_serv...
File tools/skpdiff/skpdiff_server.py (right):

https://codereview.chromium.org/19929007/diff/9001/tools/skpdiff/skpdiff_serv...
tools/skpdiff/skpdiff_server.py:69: if self.serve_if_in_dir('/', file_path):
On 2013/07/25 12:02:57, djsollen wrote:
> aren't we going to start the server after we know the directories that the
> images are in?  Why not pass those params to the server at startup and only
> serve from those directories?

Due to the way skpdiff_output.json is structured, the filenames inside it could
be relative or absolute. A future CL will change this behavior because it will
generate its own skpdiff_output.json from images it puts in a directory.
Therefore it will have much better control and knowledge over where the images
are in the filesystem.

[epoger, 2013-07-25 14:50:25]

LGTM

On 2013/07/25 13:24:46, Zach Reizner wrote:
>
https://codereview.chromium.org/19929007/diff/9001/tools/skpdiff/skpdiff_serv...
> File tools/skpdiff/skpdiff_server.py (right):
> 
>
https://codereview.chromium.org/19929007/diff/9001/tools/skpdiff/skpdiff_serv...
> tools/skpdiff/skpdiff_server.py:69: if self.serve_if_in_dir('/', file_path):
> On 2013/07/25 12:02:57, djsollen wrote:
> > aren't we going to start the server after we know the directories that the
> > images are in?  Why not pass those params to the server at startup and only
> > serve from those directories?
> 
> Due to the way skpdiff_output.json is structured, the filenames inside it
could
> be relative or absolute. A future CL will change this behavior because it will
> generate its own skpdiff_output.json from images it puts in a directory.
> Therefore it will have much better control and knowledge over where the images
> are in the filesystem.

Given the plan, and the addition of the warning in patchset 4, I think it's fine
to commit.

[Zach Reizner, 2013-07-25 19:49:23]

Committed patchset #4 manually as r10371 (presubmit successful).