Fixed rebasline iamge....

Fixed rebasline iamge.

BUG=11072
TEST=Did a rebasline of a test that didn't have and image change and 
     saw that the image was still correct after the rebaseline.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=14748

[levin, 2009-04-27 18:29:26]

This solution feels hacky.  If you have another idea for how to solve this
better, I'd love to hear it.

dave

[tony, 2009-04-27 18:53:45]

Seems fine to me.  Adding in Ojan in case he has a better idea.

http://codereview.chromium.org/99064/diff/1/2
File webkit/tools/layout_tests/layout_package/test_shell_thread.py (right):

http://codereview.chromium.org/99064/diff/1/2#newcode369
Line 369: image_hash = "a"
Maybe set it to "new_baseline" or something so it will be a bit more obvious to
someone debugging the test_shell?

[ojan, 2009-04-27 19:45:06]

http://codereview.chromium.org/99064/diff/1/2
File webkit/tools/layout_tests/layout_package/test_shell_thread.py (right):

http://codereview.chromium.org/99064/diff/1/2#newcode369
Line 369: image_hash = "a"
On 2009/04/27 18:53:45, tony wrote:
> Maybe set it to "new_baseline" or something so it will be a bit more obvious
to
> someone debugging the test_shell?

It should work to just not write the image hash to stdin. How about something
like:

test_string = "%s %s" % (test_info.uri, test_info.timeout)
if test_info.image_hash and self._test_args.new_baseline:
  test_string = test_string + " " + image_hash
test_string = test_string + '\n"

self._test_shell_proc.stdin.write(test_string)

[levin, 2009-04-27 21:22:19]

I tried Ojan's solution but it seemed that test_shell wouldn't generate an image
in this case, so I just did the simple thing and changed the string as following
Tony's idea.

[ojan, 2009-04-27 22:35:30]

On 2009/04/27 21:22:19, levin wrote:
> I tried Ojan's solution but it seemed that test_shell wouldn't generate an
image
> in this case, so I just did the simple thing and changed the string as
following
> Tony's idea.

Ah, I think this can be attributed to my C++ n00b-dom. In test_shell_main.cc, we
tokenize the stdin string on spaces. If there is a third token, we set that to
be the pixel_hash, if not, then the pixel hash is just the empty string. When we
go to dump the image in TestShell::DumpImage, we call the following:

if (md5hash.compare(0, pixel_hash.length(), pixel_hash) != 0) {
  // dump the image
}

I think we're comparing 0 characters because pixel_hash.length() returns 0. Doh!
We should just reverse the order of those in the C++ code. That feels less hacky
to me than passing in a bogus hash.

That said, I don't feel too strongly about this. It's not that bad of a hack
really.

[levin, 2009-04-28 00:33:55]

Ojan, how about this?

[ojan, 2009-04-28 01:51:56]

LGTM