Bug 103530 - Web Inspector: Introduce new "Input" domain for emitting keyboard / mouse events.
Summary: Web Inspector: Introduce new "Input" domain for emitting keyboard / mouse eve...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Ken Kania
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-28 09:09 PST by Pavel Feldman
Modified: 2012-12-04 10:05 PST (History)
14 users (show)

See Also:


Attachments
Patch (21.47 KB, patch)
2012-11-28 15:43 PST, Ken Kania
no flags Details | Formatted Diff | Diff
Patch (23.98 KB, patch)
2012-11-29 09:16 PST, Ken Kania
no flags Details | Formatted Diff | Diff
Patch (29.33 KB, patch)
2012-11-30 11:52 PST, Ken Kania
no flags Details | Formatted Diff | Diff
Patch (29.44 KB, patch)
2012-11-30 16:30 PST, Ken Kania
no flags Details | Formatted Diff | Diff
Patch (30.41 KB, patch)
2012-11-30 16:59 PST, Ken Kania
no flags Details | Formatted Diff | Diff
Patch (30.42 KB, patch)
2012-12-03 15:56 PST, Ken Kania
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2012-11-28 09:09:55 PST
We'd like to be able to emit events via remote debugging protocol. Lets add new (hidden for now) domain and put initial implementation there.
Comment 1 Ken Kania 2012-11-28 15:43:00 PST
Created attachment 176591 [details]
Patch
Comment 2 Build Bot 2012-11-28 15:52:00 PST
Comment on attachment 176591 [details]
Patch

Attachment 176591 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15013645
Comment 3 Early Warning System Bot 2012-11-28 15:52:54 PST
Comment on attachment 176591 [details]
Patch

Attachment 176591 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15010692
Comment 4 Early Warning System Bot 2012-11-28 15:55:58 PST
Comment on attachment 176591 [details]
Patch

Attachment 176591 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15018629
Comment 5 Build Bot 2012-11-28 16:01:51 PST
Comment on attachment 176591 [details]
Patch

Attachment 176591 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15015646
Comment 6 EFL EWS Bot 2012-11-28 16:36:33 PST
Comment on attachment 176591 [details]
Patch

Attachment 176591 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15029554
Comment 7 Ken Kania 2012-11-29 09:16:35 PST
Created attachment 176745 [details]
Patch
Comment 8 Yury Semikhatsky 2012-11-29 10:24:27 PST
Comment on attachment 176745 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=176745&action=review

> Source/WebCore/inspector/Inspector.json:1942
> +                    { "name": "nodeId", "$ref": "DOM.NodeId" }

Please provide description for the parameter.

> Source/WebCore/inspector/Inspector.json:3261
> +                    { "name": "type", "type": "string", "enum": ["keyDown", "keyUp", "rawKeyDown", "char"] },

Description is missing for this ans a few other parameters.

> Source/WebCore/inspector/InspectorInputAgent.h:17
> + * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY

This is a wrong copyright header. Google contributors use a bit different one. See InspectorMemoryAgent.cpp for example.

> Source/WebCore/inspector/InspectorInputAgent.h:40
> +class InstrumentingAgents;

No need to declare it here as it is already provided by InspectorBaseAgent.

> LayoutTests/inspector-protocol/dom-focus.html:1
> +<html>

Let's move "Input" domain tests under LayoutTests/inspector-protocol/input. We should eventually do the same for other tests under LayoutTests/inspector-protocol/.

> LayoutTests/inspector-protocol/dom-focus.html:7
> +    document.querySelector("#second").addEventListener("focus", function() {

We usually use named functions even if they are used as callbacks. Same thing with other anonymous functions in this patch.
Comment 9 Yury Semikhatsky 2012-11-29 10:27:07 PST
(In reply to comment #8)
> Let's move "Input" domain tests under LayoutTests/inspector-protocol/input. We should eventually do the same for other tests under LayoutTests/inspector-protocol/.
> 

Ok, this particular one seems to be from DOM domain.
Comment 10 Build Bot 2012-11-29 14:24:47 PST
Comment on attachment 176745 [details]
Patch

Attachment 176745 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15044447
Comment 11 Ken Kania 2012-11-30 11:52:05 PST
Created attachment 177003 [details]
Patch
Comment 12 Build Bot 2012-11-30 12:48:40 PST
Comment on attachment 177003 [details]
Patch

Attachment 177003 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15083080
Comment 13 Yury Semikhatsky 2012-11-30 13:42:18 PST
(In reply to comment #12)
> (From update of attachment 177003 [details])
> Attachment 177003 [details] did not pass mac-ews (mac):
> Output: http://queues.webkit.org/results/15083080

/Volumes/Data/EWS/WebKit/Source/WebCore/inspector/InspectorInputAgent.cpp:58:57: error: unused parameter 'error' [-Werror,-Wunused-parameter]

Just omit the name of the parameter if it is unused.
Comment 14 Ken Kania 2012-11-30 16:30:52 PST
Created attachment 177048 [details]
Patch
Comment 15 Build Bot 2012-11-30 16:57:27 PST
Comment on attachment 177048 [details]
Patch

Attachment 177048 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15061550

New failing tests:
inspector-protocol/input/dispatchKeyEvent.html
Comment 16 Ken Kania 2012-11-30 16:59:18 PST
Created attachment 177056 [details]
Patch
Comment 17 Build Bot 2012-11-30 20:44:26 PST
Comment on attachment 177056 [details]
Patch

Attachment 177056 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15086085

New failing tests:
inspector-protocol/input/dispatchKeyEvent.html
Comment 18 Ken Kania 2012-12-03 15:56:38 PST
Created attachment 177360 [details]
Patch
Comment 19 Yury Semikhatsky 2012-12-03 17:48:39 PST
I'm wondering why it crashed on Mac and passed on other platforms?
Comment 20 Nat Duca 2012-12-03 18:03:33 PST
Comment on attachment 177360 [details]
Patch

Should this be formulated such that it can  asynchronou? E.g. what's the execution order of

page.dispatchKeyEvent('x')
runtime.Evaluate('window.didGetX');

for some page that sets didGetX in a keyHandler on the main page...

If I push them onto the websocket right away?

I think this is less important for keys, but for mouse events, its definitely important that we think through the ordering of things.
Comment 21 WebKit Review Bot 2012-12-03 22:08:43 PST
Comment on attachment 177360 [details]
Patch

Attachment 177360 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15121591

New failing tests:
inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html
Comment 22 Ken Kania 2012-12-04 09:37:11 PST
Nat,

The dispatchKeyEvent command processes the key event synchronously, although some effects may be asynchronous (such as if the key stroke submitted a form which later causes a navigation). For your example, as long as the commands are sent in order, things will behave as you expect.
Comment 23 Yury Semikhatsky 2012-12-04 09:43:28 PST
(In reply to comment #21)
> (From update of attachment 177360 [details])
> Attachment 177360 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/15121591
> 
> New failing tests:
> inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html

Looks like a flakiness unrelated to the change.
Comment 24 WebKit Review Bot 2012-12-04 10:05:17 PST
Comment on attachment 177360 [details]
Patch

Clearing flags on attachment: 177360

Committed r136521: <http://trac.webkit.org/changeset/136521>
Comment 25 WebKit Review Bot 2012-12-04 10:05:24 PST
All reviewed patches have been landed.  Closing bug.