[WinCairo] Start RemoteInspectorServer
Created attachment 374454 [details] WIP patch
Created attachment 374455 [details] WIP patch
Comment on attachment 374455 [details] WIP patch View in context: https://bugs.webkit.org/attachment.cgi?id=374455&action=review > Source/WebKit/Shared/WebProcessCreationParameters.cpp:154 > +#if ENABLE(REMOTE_INSPECTOR) && PLATFORM(WIN) Shouldn’t this be USE(INSPECTOR_SOCKET_SERVER) not PLATFORM(WIN)?
Comment on attachment 374455 [details] WIP patch View in context: https://bugs.webkit.org/attachment.cgi?id=374455&action=review LGTM. Actually this bug is duplicate of mine :) https://bugs.webkit.org/show_bug.cgi?id=197435 > Source/WebKit/Shared/WebProcessCreationParameters.h:193 > + uint16_t m_inspectorServerPort { 0 }; Should be inspectorServerPort in POD.
<rdar://problem/53323048>
Comment on attachment 374455 [details] WIP patch View in context: https://bugs.webkit.org/attachment.cgi?id=374455&action=review >> Source/WebKit/Shared/WebProcessCreationParameters.cpp:154 >> +#if ENABLE(REMOTE_INSPECTOR) && PLATFORM(WIN) > > Shouldn’t this be USE(INSPECTOR_SOCKET_SERVER) not PLATFORM(WIN)? Umm, in Unix, it uses unix domain sockets instead of BSD sockets. >> Source/WebKit/Shared/WebProcessCreationParameters.h:193 >> + uint16_t m_inspectorServerPort { 0 }; > > Should be inspectorServerPort in POD. Oops. Thanks.
Created attachment 374586 [details] WIP patch
Created attachment 374587 [details] WIP patch
Created attachment 374668 [details] WIP patch
Created attachment 375279 [details] Updated WIP patch Hey Fujii, what do you think about these changes? I added some socket fixes to your patch to fix issues with the server disconnecting and replaced the target list page to do DOM manipulation for updating the target list table. The windows remote inspector is much more stable with these changes.
Comment on attachment 375279 [details] Updated WIP patch View in context: https://bugs.webkit.org/attachment.cgi?id=375279&action=review Looks a great improvement. Good job! The issue of racy initial empty target list should be filed as separate bug ticket. > Source/WebKit/UIProcess/socket/RemoteInspectorClient.h:88 > + URL m_url; This is not used anymore. > Source/WebKit/UIProcess/socket/RemoteInspectorProtocolHandler.cpp:100 > + // Re-create the connection to avoid holding on to bad connections If you re-create RemoteInspectorClient every time inspector:// is load, we don't need to manage RemoteInspectorClient instances by using a HashMap m_inspectorClients anymore. > Source/WebKit/UIProcess/socket/RemoteInspectorProtocolHandler.cpp:165 > + "}" I think we should have just only one JS function. "function updateTargets(str) {" "let targetDiv = document.getElementById('targetlist');" "targetDiv.innerHTML = str" "}"
Created attachment 379488 [details] Patch The client should be working now that the weakptr assert issue was fixed in r249158.
(In reply to Fujii Hironori from comment #11) > Comment on attachment 375279 [details] > Updated WIP patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=375279&action=review > > Looks a great improvement. Good job! > > The issue of racy initial empty target list should be filed as separate bug > ticket. Created https://bugs.webkit.org/show_bug.cgi?id=202169
Created attachment 379491 [details] Patch Add changelog and fix style issues
Comment on attachment 379491 [details] Patch Clearing flags on attachment: 379491 Committed r250328: <https://trac.webkit.org/changeset/250328>
All reviewed patches have been landed. Closing bug.