Bug 118678

<rdar://problem/14444476>

Created attachment 206676 [details] initial patch Added a WebSocket initialization from the old inspector code. Also defined a 'connect-src' directive into the Content-Security-Policy HTTP header to enable the connection. Added WorkerManager.js file from the old inspector because it defines the isDedicatedWorkerFrontend() to skip the websocket initialization.

Comment on attachment 206676 [details] initial patch View in context: https://bugs.webkit.org/attachment.cgi?id=206676&action=review > Source/WebInspectorUI/UserInterface/Main.js:41 > +{(function parseQueryParameters() > +{ > + WebInspector.queryParamsObject = {}; > + var queryParams = window.location.search; > + if (!queryParams) > + return; > + var params = queryParams.substring(1).split("&"); > + for (var i = 0; i < params.length; ++i) { > + var pair = params[i].split("="); > + WebInspector.queryParamsObject[pair[0]] = pair[1]; > + } > +})();} We have parseLocationQueryParameters in Utilities.js. And we use parseLocationQueryParameters once in Main.js already. We should only parse this once. We don't want WebInspector.queryParamsObject like we had before either. > Source/WebInspectorUI/UserInterface/Main.js:57 > + if (ws) { This can be a early return. > Source/WebInspectorUI/UserInterface/Main.js:66 > + if (!WebInspector.socket._detachReason) > + (new WebInspector.RemoteDebuggingTerminatedScreen("websocket_closed")).showModal(); WebInspector.RemoteDebuggingTerminatedScreen does not exist in our UI. Remove this for now. > Source/WebInspectorUI/UserInterface/Main.js:76 > + WebInspector.initializeWebSocketIfNeeded(); this.initializeWebSocketIfNeeded(); > Source/WebInspectorUI/UserInterface/WorkerManager.js:50 > +WebInspector.WorkerManager.isDedicatedWorkerFrontend = function() > +{ > + return !!WebInspector.queryParamsObject["dedicatedWorkerId"]; > +} If this is all you use WebInspector.WorkerManager for, then this can go in Main.js and WorkerManager.js can be removed. I don't see WorkerManager being called otherwise.

Comment on attachment 206676 [details] initial patch View in context: https://bugs.webkit.org/attachment.cgi?id=206676&action=review > Source/WebInspectorUI/UserInterface/Main.js:46 > + if (WebInspector.WorkerManager.isDedicatedWorkerFrontend()) > + return; We don't have dedicated worker frontends. So you can remove this check and WorkerManager. > Source/WebInspectorUI/UserInterface/Main.js:61 > + WebInspector.socket.onmessage = function(message) { InspectorBackend.dispatch(message.data); } > + WebInspector.socket.onerror = function(error) { console.error(error); } > + WebInspector.socket.onopen = function() { Can these use addEventListener instead? > Source/WebInspectorUI/UserInterface/Main.js:62 > + InspectorFrontendHost.sendMessageToBackend = WebInspector.socket.send.bind(WebInspector.socket); This code should move to InspectorFrontendHostStub.js and if WebInspector.socket exists then use WebInspector.socket.send. > Source/WebInspectorUI/UserInterface/Main.js:64 > + WebInspector.socket.onclose = function() { You can remove onclose. >> Source/WebInspectorUI/UserInterface/WorkerManager.js:50 >> +} > > If this is all you use WebInspector.WorkerManager for, then this can go in Main.js and WorkerManager.js can be removed. I don't see WorkerManager being called otherwise. It turns out we don't have dedicated worker frontends like the deprecated Inspector — and likely never will. You can remove this and all of WorkerManager.js.

Any update on this?

I will attach an updated patch soon. (In reply to comment #5) > Any update on this?

Created attachment 207307 [details] patch

Comment on attachment 207307 [details] patch Clearing flags on attachment: 207307 Committed r153044: <http://trac.webkit.org/changeset/153044>

All reviewed patches have been landed. Closing bug.

Comment on attachment 207307 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=207307&action=review Just some unimportant post thoughts. Nothing that needs to be changed. > Source/WebInspectorUI/UserInterface/Main.js:35 > + // Initialize WebSocket to communication Nits for the future, comments are sentences and should end in a period. > Source/WebInspectorUI/UserInterface/Main.js:1100 > + var host = "host" in queryParams ? queryParams.host : window.location.host; I think this could have been written: var host = queryParams.host || window.location.host; Unless the possibility of an empty string host ("") would be considered valid.