On Wed, Feb 22, 2012 at 10:48 AM, Maciej Stachowiak <[email protected]> wrote: > > On Feb 21, 2012, at 5:40 PM, Hajime Morrita wrote: > >> 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? > > For D, E and F, I think the methods should be split up into object by purpose > rather than by how they'd be implemented. I agree that these methods > shouldn't be piled onto a single LayoutTestController object.
That's right. > > One concern I have about settings: poking through the WebKit API layer to set > settings directly in WebCore may not necessarily work as expected in all > WebKit ports, or at least it will violate implementation invariants. For > example, in WebKit2 (currently using the WebKitTestRunner tool which is > separate from DumpRenderTree at present), both the WebProcess and the UI > process need to know about settings. This makes me think that settings (at > least ones surfaced at the API level) should actually go through the WebKit > API rather than poking directly into WebCore via the internals object. > Good point. I wasn't aware of this. In my understanding, the problem here is that these states can be out-of-sync. Considering so, we can move a part of Settings parameters to RuntimeFeatures, which doesn't need to be updated dynamically. I guess there is a certain numer of flags in Settings which are't changed from the default outside the test. Thanks for pointing this out. Bests, -- morrita > Regards, > Maciej > > > > > _______________________________________________ webkit-dev mailing list [email protected] http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
