Bug 88902

Summary: Web Inspector: add a way to get the remote inspector url for a given page.
Product: WebKit Reporter: Alexis Menard (darktears) <menard>
Component: Web Inspector (Deprecated)Assignee: Alexis Menard (darktears) <menard>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, jturcotte, pfeldman, webkit.review.bot, zoltan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: Windows 2000   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description Alexis Menard (darktears) 2012-06-12 13:17:10 PDT
Web Inspector: add a way to get the remote inspector url for a given page.
Comment 1 Alexis Menard (darktears) 2012-06-12 13:20:15 PDT
Created attachment 147138 [details]
Patch
Comment 2 Jocelyn Turcotte 2012-06-13 05:24:09 PDT
Comment on attachment 147138 [details]
Patch

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

> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:1302
> +    return QUrl::fromUserInput(WebInspectorServer::shared().inspectorUrlForPageID(d_ptr->webPageProxy->inspector()->remoteInspectionPageId()));

It would be cleaner to build a proper URL in WebInspectorServer::inspectorUrlForPageID and use QUrl::QUrl(QString&) instead.

> Source/WebKit2/UIProcess/API/qt/qquickwebview_p.h:274
> +    Q_PROPERTY(QUrl remoteInspectorUrl READ remoteInspectorUrl() CONSTANT FINAL)

In some case it might be useful to have it non-constant. e.g. if the developerExtraEnabled preference is set to true/false or if the inspector server is closed. If we can't change the signature of the property later we should have a notify signal already.

> Source/WebKit2/UIProcess/InspectorServer/WebInspectorServer.cpp:98
> +String WebInspectorServer::inspectorUrlForPageID(int pageId)

pageId is unused.

> Source/WebKit2/UIProcess/InspectorServer/WebSocketServer.cpp:67
> +    m_bindAddress = bindAddress;
> +    m_port = port;

This should be set only if (isNowListening).
Comment 3 Alexis Menard (darktears) 2012-06-13 05:38:01 PDT
Created attachment 147299 [details]
Patch
Comment 4 Jocelyn Turcotte 2012-06-13 05:52:14 PDT
Comment on attachment 147299 [details]
Patch

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

Only one detail, looks awesome otherwise.

> Source/WebKit2/UIProcess/InspectorServer/WebSocketServer.cpp:71
> +    } else
> +        m_state = Closed;

Because we return early if m_state == Listening you can omit that part. That wasn't needed in the original code either :P
Comment 5 Alexis Menard (darktears) 2012-06-13 05:59:25 PDT
Created attachment 147304 [details]
Patch for landing
Comment 6 WebKit Review Bot 2012-06-13 08:14:50 PDT
Comment on attachment 147304 [details]
Patch for landing

Clearing flags on attachment: 147304

Committed r120211: <http://trac.webkit.org/changeset/120211>
Comment 7 WebKit Review Bot 2012-06-13 08:14:56 PDT
All reviewed patches have been landed.  Closing bug.