Add testRunner.copyAtAndCapturePixelsAsyncThen() method

Add testRunner.copyAtAndCapturePixelsAsyncThen() method

Added method to allow "copy image" menu tests for elements that
need capturePixels to verify test results.

BUG=392765
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=284098

[hj.r.chung, 2014-07-16 09:10:49]

This is needed for the test case in https://codereview.chromium.org/389273002/ .
Please take a look

[Ken Russell (switch to Gerrit), 2014-07-16 20:40:50]

Please reference bug 392765 from this CL's description. In general, please try
to avoid dangling CLs for any nontrivial bug or feature.

https://codereview.chromium.org/396953002/diff/1/content/shell/renderer/test_...
File content/shell/renderer/test_runner/test_runner.cc (right):

https://codereview.chromium.org/396953002/diff/1/content/shell/renderer/test_...
content/shell/renderer/test_runner/test_runner.cc:532:
.SetMethod("copyAtAndCapturePixelsAsyncThen",
Please name this copyImageAtAndCapturePixelsAsyncThen. Or perhaps
copyImageAtAndReadbackAsyncThen.

https://codereview.chromium.org/396953002/diff/1/content/shell/renderer/test_...
File content/shell/renderer/test_runner/test_runner.h (right):

https://codereview.chromium.org/396953002/diff/1/content/shell/renderer/test_...
content/shell/renderer/test_runner/test_runner.h:546: void
CopyAtAndCapturePixelsAsyncThen(int x, int y, const v8::Handle<v8::Function>
callback);
Please document this. "Similar to CapturePixelsAsyncThen(). Copies to the
clipboard the image located at a particular point in the WebView (if there is
such an image), reads back its pixels, and provides the snapshot to the
callback. If there is no image at that point, calls the callback with (0, 0,
null)."

Also, please rename to CopyImageAtAndCapturePixelsAsyncThen here and throughout.

https://codereview.chromium.org/396953002/diff/1/content/shell/renderer/test_...
File content/shell/renderer/test_runner/web_test_proxy.cc (right):

https://codereview.chromium.org/396953002/diff/1/content/shell/renderer/test_...
content/shell/renderer/test_runner/web_test_proxy.cc:479: blink::WebData data =
blink::Platform::current()->clipboard()->readImage(blink::WebClipboard::Buffer());
You'll need to handle the case where WebView::copyImageAt doesn't find an image
at the given (x, y) point. Please make sure to add a layout test for this.

https://codereview.chromium.org/396953002/diff/1/content/shell/renderer/test_...
File content/shell/renderer/test_runner/web_test_proxy.h (right):

https://codereview.chromium.org/396953002/diff/1/content/shell/renderer/test_...
content/shell/renderer/test_runner/web_test_proxy.h:119: void
CopyAtAndCapturePixels(const int x, const int y,
Please name this CopyImageAtAndCapturePixels.

[hj.r.chung, 2014-07-17 05:33:40]

Applied comments and added code needed for checking negative input test case.
PTAL!

https://codereview.chromium.org/396953002/diff/1/content/shell/renderer/test_...
File content/shell/renderer/test_runner/test_runner.cc (right):

https://codereview.chromium.org/396953002/diff/1/content/shell/renderer/test_...
content/shell/renderer/test_runner/test_runner.cc:532:
.SetMethod("copyAtAndCapturePixelsAsyncThen",
On 2014/07/16 20:40:50, Ken Russell wrote:
> Please name this copyImageAtAndCapturePixelsAsyncThen. Or perhaps
> copyImageAtAndReadbackAsyncThen.

Done.

https://codereview.chromium.org/396953002/diff/1/content/shell/renderer/test_...
File content/shell/renderer/test_runner/test_runner.h (right):

https://codereview.chromium.org/396953002/diff/1/content/shell/renderer/test_...
content/shell/renderer/test_runner/test_runner.h:546: void
CopyAtAndCapturePixelsAsyncThen(int x, int y, const v8::Handle<v8::Function>
callback);
On 2014/07/16 20:40:50, Ken Russell wrote:
> Please document this. "Similar to CapturePixelsAsyncThen(). Copies to the
> clipboard the image located at a particular point in the WebView (if there is
> such an image), reads back its pixels, and provides the snapshot to the
> callback. If there is no image at that point, calls the callback with (0, 0,
> null)."
> 
> Also, please rename to CopyImageAtAndCapturePixelsAsyncThen here and
throughout.

Done. Thanks for writing the description!

https://codereview.chromium.org/396953002/diff/1/content/shell/renderer/test_...
File content/shell/renderer/test_runner/web_test_proxy.cc (right):

https://codereview.chromium.org/396953002/diff/1/content/shell/renderer/test_...
content/shell/renderer/test_runner/web_test_proxy.cc:479: blink::WebData data =
blink::Platform::current()->clipboard()->readImage(blink::WebClipboard::Buffer());
On 2014/07/16 20:40:50, Ken Russell wrote:
> You'll need to handle the case where WebView::copyImageAt doesn't find an
image
> at the given (x, y) point. Please make sure to add a layout test for this.

I added a test in the Blink patch. Thanks!

https://codereview.chromium.org/396953002/diff/1/content/shell/renderer/test_...
File content/shell/renderer/test_runner/web_test_proxy.h (right):

https://codereview.chromium.org/396953002/diff/1/content/shell/renderer/test_...
content/shell/renderer/test_runner/web_test_proxy.h:119: void
CopyAtAndCapturePixels(const int x, const int y,
On 2014/07/16 20:40:50, Ken Russell wrote:
> Please name this CopyImageAtAndCapturePixels.

Done.

[Ken Russell (switch to Gerrit), 2014-07-17 21:05:36]

Looks great. LGTM

abarth/dpranke/pfeldman/tkent: content/shell/renderer/test_runner OWNERS review
please

sky/jochen/piman: content/test OWNERS review please

[Dirk Pranke, 2014-07-17 21:23:27]

rubber-stamp lgtm.

[sky, 2014-07-17 21:32:31]

https://codereview.chromium.org/396953002/diff/60001/content/test/mock_webcli...
File content/test/mock_webclipboard_impl.cc (right):

https://codereview.chromium.org/396953002/diff/60001/content/test/mock_webcli...
content/test/mock_webclipboard_impl.cc:202: m_sequenceNumber = 0;
Is there a reason why this and m_writeSmartPaste aren't set in the member
intializer list? It doesn't seem as though this is called from the constructor
either.

[hj.r.chung, 2014-07-18 04:00:27]

https://codereview.chromium.org/396953002/diff/60001/content/test/mock_webcli...
File content/test/mock_webclipboard_impl.cc (right):

https://codereview.chromium.org/396953002/diff/60001/content/test/mock_webcli...
content/test/mock_webclipboard_impl.cc:202: m_sequenceNumber = 0;
On 2014/07/17 21:32:31, sky wrote:
> Is there a reason why this and m_writeSmartPaste aren't set in the member
> intializer list? It doesn't seem as though this is called from the constructor
> either.


I'll upload another patch with both members in the initializer list.
And I think m_sequenceNumber shouldn't be set back to 0 in clear() since then it
will always be 1 after a write and won't be able to differentiate 2 consecutive
writes from no writes. The current code works for the test I wrote but will not
if we need more complex ones. I'll fix this.

[hj.r.chung, 2014-07-18 06:25:14]

On 2014/07/17 21:32:31, sky wrote:
>
https://codereview.chromium.org/396953002/diff/60001/content/test/mock_webcli...
> File content/test/mock_webclipboard_impl.cc (right):
> 
>
https://codereview.chromium.org/396953002/diff/60001/content/test/mock_webcli...
> content/test/mock_webclipboard_impl.cc:202: m_sequenceNumber = 0;
> Is there a reason why this and m_writeSmartPaste aren't set in the member
> intializer list? It doesn't seem as though this is called from the constructor
> either.

I updated the patch! PTAL. Thanks!

[jochen (gone - plz use gerrit), 2014-07-18 08:54:45]

lgtm

[hj.r.chung, 2014-07-18 11:09:13]

The CQ bit was checked by heejin.r.chung@samsung.com

[commit-bot: I haz the power, 2014-07-18 11:10:03]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/heejin.r.chung@samsung.com/396953002/8...

[commit-bot: I haz the power, 2014-07-18 15:42:33]

Change committed as 284098