[webkit-dev] Should we move methods from layoutTestController/etc... to internals?

[Hajime Morrita]

Hi, Thanks for starting this discussion!

To summarize, there is a rough consensus which is like:

* A. LayoutTestController (LTC) is for control test harness.
dumpAsText() and waitUntilDone() is great example of this.

* B. EventSender and some other stuff are for emulating external
events like user input.
keyDown() is good example.

* C. Internals is for inspecting WebCore internal state which isn't
available from WebKit API.
Inspecting spellcheck markers is one typical example.

But I found that there are some test-specific APIs which don't fit
well in any of listed categories:

* D. Changing Settings and RuntimeFeatures values.
Inspecting flags in these objects is a good fit for Internals. But how
about changing these values? I feel that it's no longer "inspecting".
On the other hand, we know that adding new WebKit and LTC API for each
these flags is certainly tedious work especially the flag is just for
disabling/enabling half-baked features. Currently we have several flag
setters on Internals object but I worry a bit this as a start of
abusing Internals.

My feeling is that it's OK to use Internals family assuming that
getting/setting flags are "trivial" enough so that we don't need to
worry about unexpected side-effects. We already have InternalsSettings
for this purpose.

* E. Invoking some WebCore API which implements "portable" WebKit API.
This is one step forward from D. There are some WebKit APIs which just
call corresponding WebCore methods. A good example of WebCore side is
WebCore::Page::setDeviceScaleFactor(): There is a discussion to
whether we should add a wrapper API for this to Internals:
http://wkb.ug/74014.

This API is less trivial than changing settings since this isn't just
a parameter setting but has a side effect. On the other hand, some API
like this has no platform-specific part. WebKit API just calls this.
It could be felt as a waste of abstraction if we add LTC API for that
- it will just stack a few more call frames.

I think we could cover this usecase by some Internals-like object. But
it should never be Internals itself and ideally a wrapper of WebCore
object. For example, we could have something like PageWrapper for
WebCore::Page which could be implemented in the same way as Internals.

Or, we could employ the controller-client pattern to have IDL
generated class in shared WebCoreTestSupport and let each DRT to
implement the client. This could eliminate the need for hand-written
bindings even though we need to have some per-port stuff.

* F. Purely for testing, but "portable" mechanism.
Mocking some clients are primary example of this. Mock objects are
rarely platform specific and implementing it in each platform is
nothing more than coding practice IMHO. I understand that there are
some controversy around mock itself. But we have some features which
are hard to test in automated manner without mock.

For this, we could do same as E.

I think LTC isn't good place for neither D, E or F. But Internals also isn't.
What do you think?

--
morrita

On Wed, Feb 22, 2012 at 8:43 AM, Dimitri Glazkov <dglazkov at chromium.org> wrote:
> On Tue, Feb 21, 2012 at 3:39 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:
>> On Tue, Feb 21, 2012 at 3:14 PM, Maciej Stachowiak <mjs at apple.com> wrote:
>>>
>>> On Feb 21, 2012, at 1:21 PM, Ryosuke Niwa wrote:
>>>
>>> > I'd prefer getting rid of the entire interface when possible. We're
>>> > obviously removing layoutTestController, textInputController, etc... at the
>>> > moment though.
>>>
>>> I think it's helpful to organize the methods by functional area instead of
>>> dumping them all onto one giant interface. layoutTestController for example
>>> has far too many methods, in my opinion. We would not design a public API
>>> where every method is on one global interface, and our testing API should
>>> not look that way either. I would like to see our test API split up more,
>>> rather than combined into a single interface.
>>
>>
>> Yeah, that'll be nice. Maybe morrita and dglazkov have an idea on how to do
>> this nicely?
>
> I kind of like this plan: https://bugs.webkit.org/show_bug.cgi?id=76423
>
> :DG<
>
>>
>>> I'm also confused about your examples of interfaces you are "obviously"
>>> removing. Did you mean "obviously not removing"?
>>
>>
>> Sorry. Yes, it should read "obviously NOT removing".
>>
>> - Ryosuke
>>