This CL adds client.toDataURL which gets the contents...

This CL adds client.toDataURL which gets the contents
of the client area as a data url (which is a base64
encoded string of a png file)

data urls are part of the HTML5 standard and supported
by firefox, safari and chrome.
http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#dom-canvas-todataurl

That means you can now do this

var dataURL = client.toDataURL();
// make an IMG tag display the screenshot
someImgTag.src = dataURL;  
// use the IMG tag to draw into a canvas
someCanvasContext.drawImage(someImageTag, ...);

It also means there is no need for the test builds
anymore "test-dbg-d3d", "test-opt-d3d" etc as
toDataURL is part of the public API and can
therefore always be used to take screenshots
in any build.

I updated the selenium code to use it.

There are a few issues:

1) The GL version has the same limitations as taking
a screenshot before. Namely that the client area
must be on screen. We need to fix this.

2) We need to add support for origin-clean. See
https://tracker.corp.google.com/story/show/180334


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

[apatrick, 2009-08-07 18:25:45]

Could you move the data url and png specific stuff out of the renderer? Maybe
have all the ToDataUrl functions return a reference counted bitmap instead and
then turn that into a data url at the last moment in the client.

http://codereview.chromium.org/164130/diff/1/13
File core/cross/bitmap_png.cc (right):

http://codereview.chromium.org/164130/diff/1/13#newcode284
Line 284: //    is implemented.
isn't this also in core/cross/base64.cc

http://codereview.chromium.org/164130/diff/1/13#newcode298
Line 298: size_t Base64Encode(const void* srcPtr, size_t length, void* dstPtr) {
src_ptr, dst_ptr

http://codereview.chromium.org/164130/diff/1/13#newcode341
Line 341: return String("data:,");
Could these refer to a constant for empty data urls?

http://codereview.chromium.org/164130/diff/1/13#newcode368
Line 368: if (setjmp(png_jmpbuf(png_ptr))) {
setjmp messes up invocation of constructors and destructors for local variables.
I bet if there's a failure, the stream vector won't be destroyed correctly. Have
you checked? Or maybe there's another way to detect errors?

http://codereview.chromium.org/164130/diff/1/15
File core/win/d3d9/renderer_d3d9.cc (right):

http://codereview.chromium.org/164130/diff/1/15#newcode1782
Line 1782: while (data != end) {
This looks like it wants to be a for loop.

http://codereview.chromium.org/164130/diff/1/22
File utils/cross/dataurl.h (right):

http://codereview.chromium.org/164130/diff/1/22#newcode43
Line 43: extern const char* kEmptyDataURL;
const char* const

[piman, 2009-08-07 19:04:32]

http://codereview.chromium.org/164130/diff/1/13
File core/cross/bitmap_png.cc (right):

http://codereview.chromium.org/164130/diff/1/13#newcode368
Line 368: if (setjmp(png_jmpbuf(png_ptr))) {
On 2009/08/07 18:25:45, apatrick wrote:
> setjmp messes up invocation of constructors and destructors for local
variables.
> I bet if there's a failure, the stream vector won't be destroyed correctly.
Have
> you checked? Or maybe there's another way to detect errors?

setjmp/longjmp are unfortunately the standard way of handling errors in libpng.

In my experience for having looked at and fixed the other pieces of code in this
file, any object that you declare before the setjmp get properly destroyed at
the return below in the error path. So if you move the declaration of 'stream'
above the setjmp it should get properly destroyed.

I don't know how that property survives aggressive optimizations, but when I
tested on both gcc and msvc under
-O2/whatever-is-the-optimized-settings-on-msvc, it was working fine.

[gman, 2009-08-07 21:34:31]

http://codereview.chromium.org/164130/diff/1/13
File core/cross/bitmap_png.cc (right):

http://codereview.chromium.org/164130/diff/1/13#newcode284
Line 284: //    is implemented.
On 2009/08/07 18:25:45, apatrick wrote:
> isn't this also in core/cross/base64.cc

doh, forgot to delete this.

Done

http://codereview.chromium.org/164130/diff/1/13#newcode298
Line 298: size_t Base64Encode(const void* srcPtr, size_t length, void* dstPtr) {
On 2009/08/07 18:25:45, apatrick wrote:
> src_ptr, dst_ptr

Done.

http://codereview.chromium.org/164130/diff/1/13#newcode341
Line 341: return String("data:,");
On 2009/08/07 18:25:45, apatrick wrote:
> Could these refer to a constant for empty data urls?

strange. for some reason this didn't get uploaded

done

http://codereview.chromium.org/164130/diff/1/13#newcode368
Line 368: if (setjmp(png_jmpbuf(png_ptr))) {
On 2009/08/07 18:25:45, apatrick wrote:
> setjmp messes up invocation of constructors and destructors for local
variables.
> I bet if there's a failure, the stream vector won't be destroyed correctly.
Have
> you checked? Or maybe there's another way to detect errors?

I put it in another function and hope that's enough.

http://codereview.chromium.org/164130/diff/1/15
File core/win/d3d9/renderer_d3d9.cc (right):

http://codereview.chromium.org/164130/diff/1/15#newcode1782
Line 1782: while (data != end) {
On 2009/08/07 18:25:45, apatrick wrote:
> This looks like it wants to be a for loop.

Done.

http://codereview.chromium.org/164130/diff/1/22
File utils/cross/dataurl.h (right):

http://codereview.chromium.org/164130/diff/1/22#newcode43
Line 43: extern const char* kEmptyDataURL;
On 2009/08/07 18:25:45, apatrick wrote:
> const char* const

Done. (seems like there's a bunch of other string constants that also need to be
changed)

[apatrick, 2009-08-07 23:15:17]

LGTM