Add basic implementation of automation for RemoteInspector socket server backend.
Created attachment 380774 [details] PATCH
Created attachment 380775 [details] PATCH
Basically following GTK implementation.
Comment on attachment 380775 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=380775&action=review > Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorSocket.cpp:200 > + if (!m_client) > + return; > + > + if (!m_clientCapabilities || !m_clientCapabilities->remoteAutomationAllowed) > + return; > + > + if (sessionID.isNull()) > + return; Not sure that we need three separate conditionals here. > Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorSocket.cpp:359 > + String sessionID = *event.message; > + requestAutomationSession(WTFMove(sessionID), { }); This style is a bit confusing. At the least, I think `event.message.value()` would be more readable, but I also wonder if it's worth making the new requestAutomationSession take const String& instead, so that we wouldn't have to explicitly copy.
Comment on attachment 380775 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=380775&action=review >> Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorSocket.cpp:200 >> + return; > > Not sure that we need three separate conditionals here. Latter two are runtime check which caused by configuration so I'll add different LOG_ERROR to them. First one is the correct check which caused by timing and required check. >> Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorSocket.cpp:359 >> + requestAutomationSession(WTFMove(sessionID), { }); > > This style is a bit confusing. At the least, I think `event.message.value()` would be more readable, but I also wonder if it's worth making the new requestAutomationSession take const String& instead, so that we wouldn't have to explicitly copy. Right. I'll change this to const String& because last step of this function calls may ended up with const String& and using `move` here is meaningless.
Created attachment 381013 [details] PATCH
Comment on attachment 381013 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=381013&action=review > Source/JavaScriptCore/inspector/remote/RemoteInspector.h:152 > + void requestAutomationSession(const String& sessionID, const Client::SessionCapabilities&); Final nit: I guess the string name can be omitted here.
Comment on attachment 381013 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=381013&action=review Thanks! >> Source/JavaScriptCore/inspector/remote/RemoteInspector.h:152 >> + void requestAutomationSession(const String& sessionID, const Client::SessionCapabilities&); > > Final nit: I guess the string name can be omitted here. No I don't think so. General types such as bool, int or String should have its name to make it meaningful.
Comment on attachment 381013 [details] PATCH Clearing flags on attachment: 381013 Committed r251373: <https://trac.webkit.org/changeset/251373>
All reviewed patches have been landed. Closing bug.
<rdar://problem/56472064>