Bug 106650 - Web Inspector: fix DRT to deliver simulated events to devtools front-end
Summary: Web Inspector: fix DRT to deliver simulated events to devtools front-end
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: Andrey Lushnikov
URL:
Keywords:
Depends on:
Blocks: 106705
  Show dependency treegraph
 
Reported: 2013-01-11 03:03 PST by Andrey Lushnikov
Modified: 2013-01-14 04:12 PST (History)
10 users (show)

See Also:


Attachments
Patch (14.42 KB, patch)
2013-01-11 03:15 PST, Andrey Lushnikov
no flags Details | Formatted Diff | Diff
Patch (6.18 KB, patch)
2013-01-11 06:19 PST, Andrey Lushnikov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Lushnikov 2013-01-11 03:03:10 PST
Events simulated by the means of window.eventSender and window.textInputController objects (which are available in test environment only) are always delivered to main window. That makes it impossible to 
exploit these objects for devtools front-end testing.
Comment 1 Andrey Lushnikov 2013-01-11 03:15:02 PST
Created attachment 182312 [details]
Patch
Comment 2 Pavel Feldman 2013-01-11 04:45:00 PST
Comment on attachment 182312 [details]
Patch

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

> Tools/DumpRenderTree/chromium/TestRunner/public/WebTestProxy.h:77
>      void setInterfaces(WebTestInterfaces*);

I don't think you need this, just make interfaces private to the host and make it own it.

> Tools/DumpRenderTree/chromium/TestRunner/public/WebTestProxy.h:78
> +    WebTestInterfaces *testInterfaces() const { return m_testInterfaces.get(); }

WebTestInterfaces*

> Tools/DumpRenderTree/chromium/TestShell.cpp:386
> +    m_testRunner->setTestIsRunning(false);

What if testRunner is missing? Can it be?

> Tools/DumpRenderTree/chromium/TestShell.cpp:764
> +    WebTestInterfaces * testInterfaces = new WebTestInterfaces();

Just move this into the host constructor.
Comment 3 jochen 2013-01-11 04:55:55 PST
Comment on attachment 182312 [details]
Patch

I don't see an easy way how to achieve what you're trying to achieve there.

Can you please post a design doc so we can discuss possible ways to fix this?


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

> Tools/DumpRenderTree/chromium/TestRunner/public/WebTestProxy.h:43
> +#include <wtf/OwnPtr.h>

you can't have wtf/ headers in a public webkit api

> Tools/DumpRenderTree/chromium/TestRunner/public/WebTestProxy.h:137
> +    OwnPtr<WebTestInterfaces> m_testInterfaces;

there can only be one WebTestInterfaces class by design

> Tools/DumpRenderTree/chromium/TestRunner/public/WebTestProxy.h:183
> +    virtual void reset()

WebTestProxy should only have methods from WebFrameClient or WebViewClient

> Tools/DumpRenderTree/chromium/TestRunner/src/WebTestInterfaces.cpp:-58
> -    void setTestIsRunning(bool);

why are you removing this? This is required for the frame load resource callback dumping et al

>> Tools/DumpRenderTree/chromium/TestShell.cpp:386
>> +    m_testRunner->setTestIsRunning(false);
> 
> What if testRunner is missing? Can it be?

we're going to delete DRTTestRunner soon, so please don't add new calls to it

> Tools/DumpRenderTree/chromium/WebViewHost.cpp:-1171
> -        proxy()->reset();

this won't work. WebTestProxy is also used by content::RenderViewImpl
Comment 4 Andrey Lushnikov 2013-01-11 06:19:37 PST
Created attachment 182330 [details]
Patch
Comment 5 jochen 2013-01-11 12:08:30 PST
Comment on attachment 182330 [details]
Patch

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

> Tools/DumpRenderTree/chromium/TestShell.cpp:751
> +        m_devToolsTestInterfaces->bindTo(frame);

ok, that might work.

in content_shell, the devtools will run in their own process, and so they also get their own test interfaces.

Keep in mind that this code (TestShell/DRT) is going to be deleted soon, so whatever you change here needs to be ported to content_shell
Comment 6 WebKit Review Bot 2013-01-14 04:12:21 PST
Comment on attachment 182330 [details]
Patch

Clearing flags on attachment: 182330

Committed r139606: <http://trac.webkit.org/changeset/139606>
Comment 7 WebKit Review Bot 2013-01-14 04:12:26 PST
All reviewed patches have been landed.  Closing bug.