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

[Ryosuke Niwa]

On Sun, Jun 3, 2012 at 6:01 PM, Adam Barth  wrote:

> On Sun, Jun 3, 2012 at 5:54 PM, Ryosuke Niwa  wrote:
>>
>> 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.
>

My current approach doesn't result in DumpRenderTree using private
interfaces because all binding code will be generated & compiled in
WebCoreTestSupport just like internals.

On Mon, Jun 4, 2012 at 10:51 AM, Sam Weinig  wrote:
>
> My objects were to adding V8 support to WebKit2, not WebKitTestRunner.
>

Okay, that's good to know. Thanks for the clarification.

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.
>

Yeah, that's the impression I've got as well. It doesn't seem like we can
share much code between DumpRenderTree and WebKitTestRunner other than IDLs.

On Mon, Jun 4, 2012 at 11:13 AM, Sam Weinig  wrote:
>
> I would not object to the IDL files being shared, as long it doesn't
> introduce additional mental overhead.  Right now, it is pretty simple, and
> I would like to keep it that way.
>

Would you prefer merging directories for DumpRenderTree and
WebKitTestRunner or create a new intermediary library/project to share IDLs
between the two over moving IDLs to WebCoreTestSupport like I was
originally proposing? (See my patches on
https://bugs.webkit.org/show_bug.cgi?id=88183 and
https://bugs.webkit.org/show_bug.cgi?id=88200)

- Ryosuke
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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

[Sam Weinig]

On Jun 4, 2012, at 11:07 AM, Adam Barth  wrote:

> On Mon, Jun 4, 2012 at 10:51 AM, Sam Weinig  wrote:
> On Jun 3, 2012, at 6:10 PM, Ryosuke Niwa  wrote:
>> On Sun, Jun 3, 2012 at 6:01 PM, Adam Barth  wrote:
>> On Sun, Jun 3, 2012 at 5:54 PM, Ryosuke Niwa  wrote:
>> On Sun, Jun 3, 2012 at 5:33 PM, Maciej Stachowiak  wrote:
>> On Jun 3, 2012, at 8:05 PM, Ryosuke Niwa  wrote:
>>> On Sun, Jun 3, 2012 at 3:55 PM, Maciej Stachowiak  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.

*objections* :(.

> 
>> 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. 
> 
> Even if we don't end up fully merging the two, sharing the IDL file seems 
> valuable since they're intended to run the same tests, which means the 
> interface between the tests and the test harness should be the same.
> 
> Adam

I would not object to the IDL files being shared, as long it doesn't introduce 
additional mental overhead.  Right now, it is pretty simple, and I would like 
to keep it that way.

-Sam
 

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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

[Adam Barth]

On Mon, Jun 4, 2012 at 10:51 AM, Sam Weinig  wrote:

> On Jun 3, 2012, at 6:10 PM, Ryosuke Niwa  wrote:
>
> On Sun, Jun 3, 2012 at 6:01 PM, Adam Barth  wrote:
>
>> On Sun, Jun 3, 2012 at 5:54 PM, Ryosuke Niwa  wrote:
>>
>>> On Sun, Jun 3, 2012 at 5:33 PM, Maciej Stachowiak  wrote:

 On Jun 3, 2012, at 8:05 PM, Ryosuke Niwa  wrote:

 On Sun, Jun 3, 2012 at 3:55 PM, Maciej Stachowiak 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.pmhasn'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.
>

Even if we don't end up fully merging the two, sharing the IDL file seems
valuable since they're intended to run the same tests, which means the
interface between the tests and the test harness should be the same.

Adam
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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

[Sam Weinig]

On Jun 3, 2012, at 6:10 PM, Ryosuke Niwa  wrote:

> On Sun, Jun 3, 2012 at 6:01 PM, Adam Barth  wrote:
> On Sun, Jun 3, 2012 at 5:54 PM, Ryosuke Niwa  wrote:
> On Sun, Jun 3, 2012 at 5:33 PM, Maciej Stachowiak  wrote:
> On Jun 3, 2012, at 8:05 PM, Ryosuke Niwa  wrote:
>> On Sun, Jun 3, 2012 at 3:55 PM, Maciej Stachowiak  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


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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

[Balazs Kelemen]

On 06/04/2012 03:10 AM, Ryosuke Niwa wrote:
On Sun, Jun 3, 2012 at 6:01 PM, Adam Barth > wrote:


On Sun, Jun 3, 2012 at 5:54 PM, Ryosuke Niwa mailto:rn...@webkit.org>> wrote:

On Sun, Jun 3, 2012 at 5:33 PM, Maciej Stachowiak
mailto:m...@apple.com>> wrote:

On Jun 3, 2012, at 8:05 PM, Ryosuke Niwa mailto:rn...@webkit.org>> wrote:

On Sun, Jun 3, 2012 at 3:55 PM, Maciej Stachowiak
mailto:m...@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.


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


- Ryosuke



I think you refer to the discussion started by me. To make it clear, it 
was about whether can we support v8 in WebKit2, not (just) 
WebKitTestRunner. The solution of mine - wrapping v8 with the jsc api 
for WTR - would make it unnecessary to change anything on WTR. On the 
other hand, using generated bindings in WTR and WebKit2 could also be a 
way to keep the mantainance overhead of supporting v8 low.



___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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

[Ryosuke Niwa]

On Sun, Jun 3, 2012 at 6:01 PM, Adam Barth  wrote:

> On Sun, Jun 3, 2012 at 5:54 PM, Ryosuke Niwa  wrote:
>
>> On Sun, Jun 3, 2012 at 5:33 PM, Maciej Stachowiak  wrote:
>>>
>>> On Jun 3, 2012, at 8:05 PM, Ryosuke Niwa  wrote:
>>>
>>> On Sun, Jun 3, 2012 at 3:55 PM, Maciej Stachowiak  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.pmhasn'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.

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

- Ryosuke
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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

[Adam Barth]

On Sun, Jun 3, 2012 at 5:54 PM, Ryosuke Niwa  wrote:

> On Sun, Jun 3, 2012 at 5:33 PM, Maciej Stachowiak  wrote:
>>
>> On Jun 3, 2012, at 8:05 PM, Ryosuke Niwa  wrote:
>>
>> On Sun, Jun 3, 2012 at 3:55 PM, Maciej Stachowiak  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.pmhasn't
changed in 17 months, so I wouldn't expect an
ongoing maintenance issue.

Adam
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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

[Ryosuke Niwa]

On Sun, Jun 3, 2012 at 5:33 PM, Maciej Stachowiak  wrote:
>
> On Jun 3, 2012, at 8:05 PM, Ryosuke Niwa  wrote:
>
> On Sun, Jun 3, 2012 at 3:55 PM, Maciej Stachowiak  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.

- Ryosuke
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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

[Maciej Stachowiak]

Sent from my iPad

On Jun 3, 2012, at 8:05 PM, Ryosuke Niwa  wrote:

> On Sun, Jun 3, 2012 at 3:55 PM, Maciej Stachowiak  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.

> 
> It also seems better to use the public interface to the JS engine rather than 
> the internal interface, even though this requires additional codegen back 
> ends (one of which is already written). But, I may be missing advantages to 
> your approach, since I only gave it a quick glance.
> 
> My intention is to contain all interactions with the JS engine within 
> WebCoreTestSupport so that you'd never see JSStringRef, JSContextRef, etc... 
> (or their V8 equivalents) in LayoutTestController code.
> 
> Now, we wouldn't have this problem if we were merging DumpRenderTree into 
> WebKitTestRunner, but that appears to be unrealistic at the moment as far 
> I've read the code.

I think that is actually a good longer term goal, though I can see that the way 
to get there is non-obvious. Even putting them in one directory to more easily 
share some files would be good IMO. The tricky bit would be to ensure that 
ports where WK1 and WK2 versions are both applicable can build both 
simultaneously.

(I also agree with your earlier comment that DumpRenderTree isn't the greatest 
name.)

Maybe Sam has thoughts, since I think he is the one who decided to make WKTR 
completely separate from DRT in the first place.

 - Maciej
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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

[Ryosuke Niwa]

On Sun, Jun 3, 2012 at 3:55 PM, Maciej Stachowiak  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.

It also seems better to use the public interface to the JS engine rather
> than the internal interface, even though this requires additional codegen
> back ends (one of which is already written). But, I may be missing
> advantages to your approach, since I only gave it a quick glance.
>

My intention is to contain all interactions with the JS engine within
WebCoreTestSupport so that you'd never see JSStringRef, JSContextRef,
etc... (or their V8 equivalents) in LayoutTestController code.

Now, we wouldn't have this problem if we were merging DumpRenderTree into
WebKitTestRunner, but that appears to be unrealistic at the moment as far
I've read the code.

- Ryosuke
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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

[Maciej Stachowiak]

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. It also seems 
better to use the public interface to the JS engine rather than the internal 
interface, even though this requires additional codegen back ends (one of which 
is already written). But, I may be missing advantages to your approach, since I 
only gave it a quick glance.

 - Maciej

On Jun 3, 2012, at 5:17 PM, Ryosuke Niwa  wrote:

> On Sun, Jun 3, 2012 at 1:45 PM, Maciej Stachowiak  wrote:
> Already done for the WebKit2 WebKitTestRunner. It should be straightforward 
> to adapt its IDL compiler.
> 
> I've considered adapting WebKitTestRunner's code generator, but it appeared 
> quite tricky because ports don't share much code in DumpRenderTree unlike 
> WebKitTestRunner so we end up having to significantly modify each port's 
> build configuration files for DumpRenderTree. Also, WebKitTestRunner's code 
> generator only supports JSC at the moment, and supporting V8 there will 
> require a significant work and might clutter the codebase.
> 
> Given that, the approach I'm taking in 
> https://bugs.webkit.org/show_bug.cgi?id=88183 at the moment is re-use 
> Internals object's infrastructure. So I'm defining 
> LayoutTestControllerBase.idl (interfaceName=LayoutTestController) and 
> LayoutTestControllerBase.h/cpp in WebCore, and letting LayoutTestController 
> in DumpRenderTree override it.
> 
> One big advantage of this approach is that code generator in WebCore already 
> supportsand V8, so we'll be able to share more code between JSC & V8 
> implementations of LayoutTestController. In fact, my current plan is to 
> replace all usage of JSStringRef, etc... (and their V8 equivalents) by 
> std::string so that they can be identical between two implementation
> Of course, one big disadvantage is that we'll be introducing a separate IDL 
> file for LayoutTestController. However, it appears that we can use the same 
> object injection mechanism for WebKitTestRunner as well. So I'm hopeful that 
> we can ultimately share IDLs between DumpRenderTree and WebKitTestRunner.
> 
> What do you think of this approach?
> 
> - Ryosuke
> 
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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

[Ryosuke Niwa]

On Sun, Jun 3, 2012 at 1:45 PM, Maciej Stachowiak  wrote:

> Already done for the WebKit2 WebKitTestRunner. It should be
> straightforward to adapt its IDL compiler.
>

I've considered adapting WebKitTestRunner's code generator, but it appeared
quite tricky because ports don't share much code in DumpRenderTree unlike
WebKitTestRunner so we end up having to significantly modify each port's
build configuration files for DumpRenderTree. Also, WebKitTestRunner's code
generator only supports JSC at the moment, and supporting V8 there will
require a significant work and might clutter the codebase.

Given that, the approach I'm taking in
https://bugs.webkit.org/show_bug.cgi?id=88183 at the moment is re-use
Internals object's infrastructure. So I'm defining
LayoutTestControllerBase.idl (interfaceName=LayoutTestController) and
LayoutTestControllerBase.h/cpp in WebCore, and letting LayoutTestController
in DumpRenderTree override it.

One big advantage of this approach is that code generator in WebCore
already supports JSC and V8, so we'll be able to share more code between
JSC & V8 implementations of LayoutTestController. In fact, my current plan
is to replace all usage of JSStringRef, etc... (and their V8 equivalents)
by std::string so that they can be identical between two implementations.

Of course, one big disadvantage is that we'll be introducing a separate IDL
file for LayoutTestController. However, it appears that we can use the same
object injection mechanism for WebKitTestRunner as well. So I'm hopeful
that we can ultimately share IDLs between DumpRenderTree and
WebKitTestRunner.

What do you think of this approach?

- Ryosuke
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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

[Maciej Stachowiak]

Already done for the WebKit2 WebKitTestRunner. It should be straightforward to 
adapt its IDL compiler.

 - Maciej

On Jun 2, 2012, at 11:03 PM, Adam Barth  wrote:

> We might also want to define the layoutTestController object with IDL
> so we don't have to hand-write bindings every time we change it.
> Using IDL for window.internals seems to have worked out well.  It
> would be nice to replicate that success with layoutTestController.
> 
> Adam
> 
> 
> On Sat, Jun 2, 2012 at 6:55 PM, Ryosuke Niwa  wrote:
>> Hi,
>> 
>> One thing that has annoyed me ever since I started working on WebKit is that
>> our test runner is called DumpRenderTree. We fixed this problem in WebKit2
>> by renaming it to WebKitTestRunner but we've still got DumpRenderTree in
>> WebKit1 land.
>> 
>> Can we either rename DumpRenderTree to WebKitTestRunner or merge those two
>> programs into one?
>> 
>> Best,
>> Ryosuke Niwa
>> Software Engineer
>> Google Inc.
>> 
>> 
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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

[Ryosuke Niwa]

On Sat, Jun 2, 2012 at 8:04 PM, Ryosuke Niwa  wrote:

> On Sat, Jun 2, 2012 at 8:03 PM, Adam Barth  wrote:
>
>> We might also want to define the layoutTestController object with IDL
>> so we don't have to hand-write bindings every time we change it.
>> Using IDL for window.internals seems to have worked out well.  It
>> would be nice to replicate that success with layoutTestController.
>>
>
> That sounds like a great idea.
>

Filed https://bugs.webkit.org/show_bug.cgi?id=88183
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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

[Ryosuke Niwa]

On Sat, Jun 2, 2012 at 8:03 PM, Adam Barth  wrote:

> We might also want to define the layoutTestController object with IDL
> so we don't have to hand-write bindings every time we change it.
> Using IDL for window.internals seems to have worked out well.  It
> would be nice to replicate that success with layoutTestController.
>

That sounds like a great idea.

- Ryosuke
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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

[Adam Barth]

We might also want to define the layoutTestController object with IDL
so we don't have to hand-write bindings every time we change it.
Using IDL for window.internals seems to have worked out well.  It
would be nice to replicate that success with layoutTestController.

Adam


On Sat, Jun 2, 2012 at 6:55 PM, Ryosuke Niwa  wrote:
> Hi,
>
> One thing that has annoyed me ever since I started working on WebKit is that
> our test runner is called DumpRenderTree. We fixed this problem in WebKit2
> by renaming it to WebKitTestRunner but we've still got DumpRenderTree in
> WebKit1 land.
>
> Can we either rename DumpRenderTree to WebKitTestRunner or merge those two
> programs into one?
>
> Best,
> Ryosuke Niwa
> Software Engineer
> Google Inc.
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

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

[Ryosuke Niwa]

Hi,

One thing that has annoyed me ever since I started working on WebKit is
that our test runner is called DumpRenderTree. We fixed this problem in
WebKit2 by renaming it to WebKitTestRunner but we've still got
DumpRenderTree in WebKit1 land.

Can we either rename DumpRenderTree to WebKitTestRunner or merge those two
programs into one?

Best,
Ryosuke Niwa
Software Engineer
Google Inc.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev