[webkit-dev] Renaming DumpRenderTree to WebKitTestRunner or merging those two

Sam Weinig weinig at apple.com
Mon Jun 4 10:51:37 PDT 2012


On Jun 3, 2012, at 6:10 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:

> On Sun, Jun 3, 2012 at 6:01 PM, Adam Barth <abarth at webkit.org> wrote:
> On Sun, Jun 3, 2012 at 5:54 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:
> On Sun, Jun 3, 2012 at 5:33 PM, Maciej Stachowiak <mjs at apple.com> wrote:
> On Jun 3, 2012, at 8:05 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:
>> On Sun, Jun 3, 2012 at 3:55 PM, Maciej Stachowiak <mjs at apple.com> wrote:
>> I am on vacation so I won't be able to review your patch in detail, but from your description it sounds less appealing to me than the WKTR approach. It seems like bad layering to me to define the IDL interface in WebCore for something actually implemented completely outside of WebCore.
>> 
>> While you're right that it's somewhat of a layer violation to define the IDL for layoutTestController, WebCoreTestSupport appears to be the most logical place to share files between DumpRenderTree and WebKitTestRunner at the moment unless we're going to create another project/library in Tools.
> 
> The downside is that they would be using internal WebCore interfaces instead of the public interface as originally intended. I do not think that is a good change, nor does it seem required just to share more code.
> 
> Are you referring to things like JSValueRef? If JS* functions are supposed to be tested in DumpRenderTree, then that's a good argument against this approach.
> 
> JavaScriptCore has a bunch of tests for its API independent of DumpRenderTree.  I suspect Maciej's point is more about having DumpRenderTree use public rather than private interfaces so that it's easier for JavaScriptCore to change its private interfaces.
> 
> Have you looked at adding a V8 backend to the code generator that WebKitTestRunner is using currently?  My guess is that it will be much simpler than CodeGeneratorV8.pm because WebKitTestRunner doesn't have deal with all the exciting variety was have in the web platform.  http://trac.webkit.org/browser/trunk/Tools/WebKitTestRunner/InjectedBundle/Bindings/CodeGeneratorTestRunner.pm hasn't changed in 17 months, so I wouldn't expect an ongoing maintenance issue.
> 
> Yes. But there was a recent discussion about supporting V8 in WebKitTestRunner where Sam objected to it citing that doing so will clutter the WTR's codebase. I have no problem with creating a V8 backend if Sam doesn't mind introducing V8 equivalent there.
> 

My objects were to adding V8 support to WebKit2, not WebKitTestRunner.

> However, the real difficulty is in sharing files between WebKitTestRunner and DumpRenderTree, and modifying 11? build configurations we have for DumpRenderTree.
> 

Just to give a bit of history here, the reason I didn't use DumpRenderTree for writing a test harness for WebKit2 was that there was no real benefit in me doing so.  The requirements of the test harness were quite different than they were for a test harness for WebKit1, specifically, we had to split functionality between the UIProcess and injected bundle.  In addition, DumpRenderTree had become a weird hodge-podge of code, where sharing code between projects seemed to be the exception, rather than the rule, for instance, I am not sure if the chromium port shares any code in DumpRenderTree with other ports (I could be wrong there), and since I was binding to a new API, a fresh start seemed the way to go.  All in all, I feel it has worked quite well, and has allowed better sharing of code between the Qt, Mac, Gtk and Windows WebKit2 implementations.

That said, I am not sure the current approach taken in WebKitTestRunner is fully suited to being merged with DumpRenderTree.  It is quite tied to the idea of having two processes and specifically the WebKit2 API. 

-Sam


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20120604/4a2814d0/attachment.html>


More information about the webkit-dev mailing list