Bug 25990 – [GTK] DumpRenderTree needs eventSender object and implementation

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Holger Freyther]

To pass fast/events and various other unit tests we will need an eventSender.

[Holger Freyther]

Created attachment 30630 [details]
WorkInProgress EventSender implementation

This is work in progress, it is based on the windows version.

Open issues:
  - Drag and Drop is not implemented at all
  - I don't know if replayEvents should enter the glib event loop
  - When to send a key up event?

[Zan Dobersek]

Created attachment 39295 [details]
Partial implementation of EventSender

Though partial, it implements a lot. What's still missing is support for DND tests (waiting for proper DND support in WebCore) and some leapForward love.
200+ tests now pass, all of them enabled in the Skipped.

[Zan Dobersek]

Created attachment 39297 [details]
Partial implementation of EventSender

Defines zoom multiplier ratio as a static constant float.

[Gustavo Noronha (kov)]

Comment on attachment 39297 [details]
Partial implementation of EventSender

> +#include "DumpRenderTree.h"
> +
> +#include <gdk/gdk.h>
> +#include <gdk/gdkkeysyms.h>
> +
> +#include <wtf/ASCIICType.h>
> +#include <wtf/Platform.h>
> +#include <JavaScriptCore/JSObjectRef.h>
> +#include <JavaScriptCore/JSRetainPtr.h>
> +#include <JavaScriptCore/JSStringRef.h>
> +#include <webkit/webkitwebframe.h>
> +#include <webkit/webkitwebview.h>
> +
> +#include <string.h>

The first two includes are OK, but the others look a bit weirdly organized. I
think we want to follow our standards, which we use elsewhere, even if
DumpRenderTree has it this way somewhere, so let's do DumpRenderTree.h, and all
the other webkit-related includes in one block, and the system inclues in a
second block.

> +#define ZoomMultiplierRatio     1.2f

like we discussed on IRC, make this all-caps, or turn it into a static const
float, and Z->z.

> +    WebKitWebView* view = webkit_web_frame_get_web_view(mainFrame);
> +    ASSERT(view);
> +
> +    webkit_web_frame_layout(mainFrame);
> +
> +    gboolean return_val;

Both view and return_val are declared many lines before they are actually used.
Please put their declaration close to where they are used for the first time.

> +    if (!msgQueue[endOfQueue].delay) {
> +        webkit_web_frame_layout(mainFrame);
> +        WebKitWebView* view = webkit_web_frame_get_web_view(mainFrame);
> +        ASSERT(view);

I suggest an empty line between the _layout call, and the declaration, but feel
free to ignore this, I just feel it feels better.

> +        if (modifiersArray) {
> +            int modifiersCount = JSValueToNumber(context, JSObjectGetProperty(context, modifiersArray, lengthProperty, 0), 0);
> +            for (int i = 0; i < modifiersCount; ++i) {

Since modifiersCount is not used anywhere else, I'd just add the
JSValueToNumber call inside the for, where the variable is now.

> +    JSStringRef character = JSValueToStringCopy(context, arguments[0], exception);
> +    ASSERT(!*exception);

I don't think ASSERT is what we want, in DRT. If this fails, we want the test
to fail in release builds, right? So I suggest returning with undefined in
here, if the asserted condition is not met.

> +    else if (JSStringIsEqualToUTF8CString(character, "delete"))
> +        gdkKeySym = GDK_BackSpace;

Interesting. I would expect delete here instead of backspace =). Sounds like
those compatibility options from gnome terminal would be required even for the
web, eh?

Thanks for working on this, r- for now because of the stuff mentioned above.

[Zan Dobersek]

Created attachment 39303 [details]
Partial implementation of EventSender

Fixes includes organisation, declaration positions, turns ASSERTs to release build-friendly check functions, uses GDK_Delete instead of GDK_BackSpace.

[Gustavo Noronha (kov)]

Comment on attachment 39303 [details]
Partial implementation of EventSender

Looks good to me, thanks!

[WebKit Commit Bot]

Comment on attachment 39303 [details]
Partial implementation of EventSender

Clearing flags on attachment: 39303

Committed r48228: <http://trac.webkit.org/changeset/48228>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.