[webkit-dev] window.internals abuse

[Ryosuke Niwa]

On Wed, Mar 13, 2013 at 11:50 AM, Dana Jansens <danakj at chromium.org> wrote:

> On Wed, Mar 13, 2013 at 1:25 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:
>
>> On Wed, Mar 13, 2013 at 10:22 AM, Dana Jansens <danakj at chromium.org>wrote:
>>
>>> On Wed, Mar 13, 2013 at 1:21 PM, Simon Fraser <simon.fraser at apple.com>wrote:
>>>
>>>> On Mar 13, 2013, at 10:15 AM, Dana Jansens <danakj at chromium.org> wrote:
>>>>
>>>> On Wed, Mar 13, 2013 at 12:42 PM, Simon Fraser <simon.fraser at apple.com>wrote:
>>>>
>>>>> https://bugs.webkit.org/show_bug.cgi?id=80046 (landed in
>>>>> http://trac.webkit.org/changeset/114081) added
>>>>> internals. setBackgroundBlurOnNode().
>>>>>
>>>>> I consider this to be abuse of the Internals object. As shown by the
>>>>> location of the files (Source/WebCore/*testing*/Internals.*), the
>>>>> Internals interface is intended for testing, not adding API to toggle
>>>>> features in WebCore for non-test code.
>>>>>
>>>>
>>>> Sorry, but I think this is based on a misunderstanding. That function
>>>> is used for enabling the feature in layout tests in order to verify it is
>>>> working. It is not used in non-test code.
>>>>
>>>>
>>>> So why does the feature exist at all?
>>>>
>>>
>>> It can be enabled by UI code via the platform layer directly.
>>>
>>
>> That sounds like a feature to be tested via testRunner object, not
>> internals object.
>>
>> internals object should be reserved for testing cross-platform,
>> cross-port features. Otherwise, we'll end up zillion of if-defs for each
>> and every port specific feature we want to test :(
>>
>
> I don't think that testRunner existed in its current form a year ago when
> this was written. I can look into moving it there if it would be better
> suited.
>

Of course not in the current form but testRunner (or layoutTestController
before the rename) object have existed for years.  It had been the de facto
standard way of exposing new testing functions to layout tests until we've
introduced internals object two years ago.

- R. Niwa
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20130313/758d58d6/attachment.html>