[Webkit-unassigned] [Bug 33096] [Qt] DRT: Support evaluateInWebInspector(), setTimerProfilingEnabled() and display().

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=33096


Simon Hausmann <hausmann at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #46743|review?                     |review-
               Flag|                            |




--- Comment #36 from Simon Hausmann <hausmann at webkit.org>  2010-01-22 07:59:06 PST ---
(From update of attachment 46743)
In general this looks like a really good patch! Excellent work with improving
our test coverage.



> +void QWEBKIT_EXPORT qt_drt_webinspector_executeScript(QWebPage *page, long callId, const QString &script)
> +{
> +    if (!page->handle()->page->inspectorController())
> +        return;
> +    page->handle()->page->inspectorController()->evaluateForTestInFrontend(callId, script);
> +}
> +
> +void QWEBKIT_EXPORT qt_drt_webinspector_close(QWebPage *page)
> +{
> +    if (!page->handle()->page->inspectorController())
> +        return;
> +    page->handle()->page->inspectorController()->close();
> +}
> +
> +void QWEBKIT_EXPORT qt_drt_webinspector_show(QWebPage *page)
> +{
> +    if (!page->handle()->page->inspectorController())
> +        return;
> +    page->handle()->page->inspectorController()->show();
> +}

This approach is fine with me, but the '*' position is incorrect (coding style
nitpick :)


> diff --git a/WebKit/qt/WebCoreSupport/InspectorClientQt.cpp b/WebKit/qt/WebCoreSupport/InspectorClientQt.cpp
> index 1caf96d..6e3bd5c 100644
> --- a/WebKit/qt/WebCoreSupport/InspectorClientQt.cpp
> +++ b/WebKit/qt/WebCoreSupport/InspectorClientQt.cpp
> @@ -120,6 +120,9 @@ void InspectorClientQt::showWindow()
>  void InspectorClientQt::closeWindow()
>  {
>  #if ENABLE(INSPECTOR)
> +    // QWebInspector::frontEnd is a copy of inspectorView which will be left dangling
> +    // by m_frontend.set(0) in InspectorController::close() if we do not clear it here.
> +    m_inspectedWebPage->d->getOrCreateInspector()->d->setFrontend(0);
>      m_inspectedWebPage->d->inspectorController()->setWindowVisible(false);
>  #endif

Jocelyn, does that look okay to you? (you have a better idea of the inspector
workings than I do :)

>
> +static bool isWebInspectorTest(const QUrl& url)
> +{
> +    if (url.toString().contains("inspector/"))

Is there an easier way to do this? Maybe just url.path().contains() instead of
the expensive toString() conversion?

r- because of the style bit, and I'd like to hear Jocelyn's opinion on the
InspectorClientQt.cpp change.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.