Make WebTestProxy handle printing + compositing mode

Make WebTestProxy handle printing + compositing mode

CapturePixels has a special branch for printing code, but
CapturePixelsAsync before this patch doesn't.  Because WebKitTestRunner
calls the async version in compositing mode, printing tests fail if it
doesn't also handle this case.

The stateful canvas() accessor that the paintPagesWithBoundaries
modifies is a bit of a code smell that previously existed, but it can't
be easily cleaned up until the CapturePixels function gets removed.

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

[danakj, 2014-05-09 21:17:21]

LGTM

https://codereview.chromium.org/274253002/diff/20001/content/shell/renderer/t...
File content/shell/renderer/test_runner/WebTestProxy.cpp (right):

https://codereview.chromium.org/274253002/diff/20001/content/shell/renderer/t...
content/shell/renderer/test_runner/WebTestProxy.cpp:510: callback.Run(bitmap);
do you wanna post-task this to keep it async? otherwise printing tests get
somewhat different call stacks?

[enne (OOO), 2014-05-09 21:25:05]

https://codereview.chromium.org/274253002/diff/20001/content/shell/renderer/t...
File content/shell/renderer/test_runner/WebTestProxy.cpp (right):

https://codereview.chromium.org/274253002/diff/20001/content/shell/renderer/t...
content/shell/renderer/test_runner/WebTestProxy.cpp:510: callback.Run(bitmap);
On 2014/05/09 21:17:21, danakj wrote:
> do you wanna post-task this to keep it async? otherwise printing tests get
> somewhat different call stacks?

It's a nice thought, but it seems awkward to do with the canvas() being stored
statefully since something else could clobber it in the meantime.  I could make
a copy of the bitmap and then pass that along with the callback, but that seems
wasteful.

Can you think of an ok way to do this?

[danakj, 2014-05-09 21:37:24]

https://codereview.chromium.org/274253002/diff/20001/content/shell/renderer/t...
File content/shell/renderer/test_runner/WebTestProxy.cpp (right):

https://codereview.chromium.org/274253002/diff/20001/content/shell/renderer/t...
content/shell/renderer/test_runner/WebTestProxy.cpp:510: callback.Run(bitmap);
On 2014/05/09 21:25:06, enne wrote:
> On 2014/05/09 21:17:21, danakj wrote:
> > do you wanna post-task this to keep it async? otherwise printing tests get
> > somewhat different call stacks?
> 
> It's a nice thought, but it seems awkward to do with the canvas() being stored
> statefully since something else could clobber it in the meantime.  I could
make
> a copy of the bitmap and then pass that along with the callback, but that
seems
> wasteful.
> 
> Can you think of an ok way to do this?

I figured the test harness shouldn't be doing anything else in the meantime so
it should "just work". If that's not true then things are indeed more awkward
until that happens. Alternatively you could post-task this whole block here to
keep it async until the m_canvas can go?

[enne (OOO), 2014-05-09 22:03:20]

https://codereview.chromium.org/274253002/diff/20001/content/shell/renderer/t...
File content/shell/renderer/test_runner/WebTestProxy.cpp (right):

https://codereview.chromium.org/274253002/diff/20001/content/shell/renderer/t...
content/shell/renderer/test_runner/WebTestProxy.cpp:510: callback.Run(bitmap);
On 2014/05/09 21:37:25, danakj wrote:
> On 2014/05/09 21:25:06, enne wrote:
> > On 2014/05/09 21:17:21, danakj wrote:
> > > do you wanna post-task this to keep it async? otherwise printing tests get
> > > somewhat different call stacks?
> > 
> > It's a nice thought, but it seems awkward to do with the canvas() being
stored
> > statefully since something else could clobber it in the meantime.  I could
> make
> > a copy of the bitmap and then pass that along with the callback, but that
> seems
> > wasteful.
> > 
> > Can you think of an ok way to do this?
> 
> I figured the test harness shouldn't be doing anything else in the meantime so
> it should "just work". If that's not true then things are indeed more awkward
> until that happens. Alternatively you could post-task this whole block here to
> keep it async until the m_canvas can go?

Done.

[danakj, 2014-05-09 22:04:28]

LGTM

[abarth-chromium, 2014-05-10 00:47:54]

rslgtm

[enne (OOO), 2014-05-12 17:48:56]

The CQ bit was checked by enne@chromium.org

[commit-bot: I haz the power, 2014-05-12 17:50:13]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/enne@chromium.org/274253002/50002

[commit-bot: I haz the power, 2014-05-12 19:13:52]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)

[commit-bot: I haz the power, 2014-05-12 20:40:00]

Change committed as 269867