[webkit-reviews] review denied: [Bug 64297] [Qt][WK2] Add the Web Inspector to WebKit2 : [Attachment 104947] Updated Web Inspector patch for one of latest WebKit2 rev. Fixed Alphabetical sorting problem

[bugzilla-daemon at webkit.org]

Alexis Menard <alexis.menard at openbossa.org> has denied Genisim
<genisim at yahoo.com>'s request for review:
Bug 64297: [Qt][WK2] Add the Web Inspector to WebKit2
https://bugs.webkit.org/show_bug.cgi?id=64297

Attachment 104947: Updated Web Inspector patch for one of latest WebKit2 rev.
Fixed Alphabetical sorting problem
https://bugs.webkit.org/attachment.cgi?id=104947&action=review

------- Additional Comments from Alexis Menard <alexis.menard at openbossa.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=104947&action=review


> Source/WebKit2/ChangeLog:17
> +	   (WebKit::WebInspectorProxy::didLoadInspectorPage):
RequestAttachWindow() not exist. Commented.

Regenerate your changelog after removing commented code.

> Source/WebKit2/UIProcess/WebInspectorProxy.cpp:223
> +    inspectorPage->loadURL(inspectorPageURL());

Why this change?

> Source/WebKit2/UIProcess/WebInspectorProxy.cpp:235
> +//	    
m_page->process()->send(Messages::WebInspector::RequestAttachWindow(),
m_page->pageID());

You can't commit that.

> Source/WebKit2/UIProcess/WebInspectorProxy.h:183
> +    QSGView* m_view;

I think you can use OwnPtr here.

> Source/WebKit2/UIProcess/WebInspectorProxy.h:184
> +    QDesktopWebViewPrivate* m_inspectorView;

I don't understand why it needs to be a QDesktopWebViewPrivate unless I miss
something.

> Source/WebKit2/UIProcess/qt/WebInspectorProxyQt.cpp:38
> +class QObject;

You don't forward declare class in a cpp object.

> Source/WebKit2/UIProcess/qt/WebInspectorProxyQt.cpp:58
> +    m_view = new QSGView();

This seems to leak.

> Source/WebKit2/UIProcess/qt/WebInspectorProxyQt.cpp:68
> +	   delete m_inspectorView->q;

What is that? This is very wrong programming methods.

> Tools/MiniBrowser/qt/BrowserView.cpp:35
> +#include <qdesktopwebview_p.h>

This is not acceptable. You can't use private API.

> Tools/MiniBrowser/qt/BrowserView.cpp:100
> +	   QDesktopWebViewPrivate* desktopWebViewPrivate = new
QDesktopWebViewPrivate(desktopWebView()); 

This is wrong. You can't create private object like this.

> Tools/MiniBrowser/qt/BrowserView.cpp:112
> +	   delete desktopWebViewPrivate;

This is very wrong to me. You shouldn't do that at all. Have you look how the
API looks like in WebKit1, we should aim for something easy, this is WAY too
complicated to use and requires private API and some hacks.

> Tools/MiniBrowser/qt/MiniBrowser.pro:23
> +include(../../../Source/WebKit2/WebKit2.pri)

Why this?