[content shell] link against the TestRunner library

[content shell] link against the TestRunner library

BUG=111316
TEST=layout tests are more awesome


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

[jochen (gone - plz use gerrit), 2012-10-23 19:11:20]

depends on https://bugs.webkit.org/show_bug.cgi?id=100079

[jochen (gone - plz use gerrit), 2012-10-29 11:00:59]

Marja, mind having a look?

[jochen (gone - plz use gerrit), 2012-10-29 11:38:31]

Here's a description of the change:

I'm linking against the TestRunner library, and instantiate a WebTestInterfaces
object (in the ShellRenderProcessObserver). This replaces the test runners that
were previously mocked in the webkit_test_runner.js file

To use the WebTestInterfaces, I need a notion of the "main render view" and
something implementing the WebTestDelegate interface.

Before, the WebKitTestController (in webkit_test_runner_host.cc) had a main
window. I'm adding an IPC to tell the renderer which render view is the main
window.

Then, the WebKitTestRunner observing this main render view can implement the
WebTestDelegate interface.

I've also added implementations of some of the WebTestDelegate methods, for
which I had to add some methods to RV and RVO.

[marja, 2012-10-29 11:42:43]

lgtm (as in "can't notice anything wrong")

[jochen (gone - plz use gerrit), 2012-10-29 11:47:03]

Adam, please review how the WebTestInterfaces are hooked up in
ShellRenderProcessObserver.

John. please review the rest

[jochen (gone - plz use gerrit), 2012-10-29 12:30:02]

linux_clang compile error is fixed by
https://bugs.webkit.org/show_bug.cgi?id=100658

[jochen (gone - plz use gerrit), 2012-10-29 12:30:24]

https://codereview.chromium.org/11245004/diff/12001/DEPS
File DEPS (right):

https://codereview.chromium.org/11245004/diff/12001/DEPS#newcode11
DEPS:11: "webkit_revision": "132781",
this is just to make the trybots happy

[jam, 2012-10-29 15:28:19]

On 2012/10/29 11:47:03, jochen wrote:
> Adam, please review how the WebTestInterfaces are hooked up in
> ShellRenderProcessObserver.
> 
> John. please review the rest

you want me to review all the files? i.e. all the files that marja looked at?

[jochen (gone - plz use gerrit), 2012-10-29 15:31:26]

On 2012/10/29 15:28:19, John Abd-El-Malek wrote:
> On 2012/10/29 11:47:03, jochen wrote:
> > Adam, please review how the WebTestInterfaces are hooked up in
> > ShellRenderProcessObserver.
> > 
> > John. please review the rest
> 
> you want me to review all the files? i.e. all the files that marja looked at?

Technically, it would be enough to review content/renderer and content/public,
but these changes are pretty meaningless if you don't look at the rest, although
that's ok with me

In  https://codereview.chromium.org/11245004/#msg3 I gave an overview of what
the CL does. If you're ok with the overall change, then the stuff outside of
content/shell will do

[jam, 2012-10-29 15:32:47]

lgtm

https://codereview.chromium.org/11245004/diff/12001/content/public/renderer/r...
File content/public/renderer/render_view_observer.h (right):

https://codereview.chromium.org/11245004/diff/12001/content/public/renderer/r...
content/public/renderer/render_view_observer.h:22: struct WebContextMenuData;
nit: struct after class per style guide, so this should be in line 31

[abarth-chromium, 2012-10-29 22:00:42]

http://codereview.chromium.org/11245004/diff/12001/content/shell/webkit_test_...
File content/shell/webkit_test_runner.h (right):

http://codereview.chromium.org/11245004/diff/12001/content/shell/webkit_test_...
content/shell/webkit_test_runner.h:30: virtual void clearContextMenuData()
OVERRIDE;
We don't usually use OVERRIDE when implementing WebKit API delegates.  That ends
up making it harder to change the API.

http://codereview.chromium.org/11245004/diff/12001/content/shell/webkit_test_...
File content/shell/webkit_test_runner.js (left):

http://codereview.chromium.org/11245004/diff/12001/content/shell/webkit_test_...
content/shell/webkit_test_runner.js:9: var textInputController =
textInputController || {};
\o/

[abarth-chromium, 2012-10-29 22:02:55]

I didn't look at all the details here, but aside from the OVERRIDE thing, this
patch LGTM

[jochen (gone - plz use gerrit), 2012-10-31 16:09:00]

On 2012/10/29 22:02:55, abarth wrote:
> I didn't look at all the details here, but aside from the OVERRIDE thing, this
> patch LGTM

I'll land the patch with the OVERRIDES, since the chrome clang plugin doesn't
let me do otherwise.

I've landed a patch to the clang plugin that applies the usual webkit api rules
to the WebTestRunner namespace - after the next clang update, I will remove the
OVERRIDES

[commit-bot: I haz the power, 2012-10-31 16:09:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jochen@chromium.org/11245004/19001

[commit-bot: I haz the power, 2012-10-31 21:51:09]

Step "update" is always a major failure.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-11-01 09:28:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jochen@chromium.org/11245004/19001

[commit-bot: I haz the power, 2012-11-01 10:34:12]

Change committed as 165329