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
Alexis Menard (darktears)
2012-06-12 13:17:10 PDT
Created attachment 147138 [details]
Patch
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). Created attachment 147299 [details]
Patch
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 Created attachment 147304 [details]
Patch for landing
Comment on attachment 147304 [details] Patch for landing Clearing flags on attachment: 147304 Committed r120211: <http://trac.webkit.org/changeset/120211> All reviewed patches have been landed. Closing bug. |