ENABLE(REMOTE_INSPECTOR) is an alternative to ENABLE(INSPECTOR_SERVER) for remotely debugging via the inspector protocol. REMOTE_INSPECTOR on Mac platforms exposes an XPC interface to receive page listings, setup/disconnect a remote debugging session, and send/receive inspector protocol socket messages. This is an alternative to the Web Socket approach enabled by INSPECTOR_SERVER.
Created attachment 214782 [details] [PATCH] Add ENABLE(REMOTE_INSPECTOR) Implementation This is a working WebKit1 Mac / iOS implementation. There are extra iOS bits that I'll put in a separate patch for node highlighting.
Attachment 214782 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/FeatureDefines.h', u'Source/WebCore/ChangeLog', u'Source/WebCore/Configurations/FeatureDefines.xcconfig', u'Source/WebCore/WebCore.exp.in', u'Source/WebKit/ChangeLog', u'Source/WebKit/WebKit.xcodeproj/project.pbxproj', u'Source/WebKit/cf/ChangeLog', u'Source/WebKit/cf/WebCoreSupport/WebInspectorClientCF.cpp', u'Source/WebKit/ios/ChangeLog', u'Source/WebKit/ios/WebCoreSupport/WebInspectorClientIOS.mm', u'Source/WebKit/mac/ChangeLog', u'Source/WebKit/mac/Configurations/FeatureDefines.xcconfig', u'Source/WebKit/mac/Misc/WebKitLogging.h', u'Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm', u'Source/WebKit/mac/WebCoreSupport/WebInspectorClient.h', u'Source/WebKit/mac/WebCoreSupport/WebInspectorClient.mm', u'Source/WebKit/mac/WebInspector/remote/WebInspectorClientRegistry.h', u'Source/WebKit/mac/WebInspector/remote/WebInspectorClientRegistry.mm', u'Source/WebKit/mac/WebInspector/remote/WebInspectorRelayDefinitions.h', u'Source/WebKit/mac/WebInspector/remote/WebInspectorRemoteChannel.h', u'Source/WebKit/mac/WebInspector/remote/WebInspectorRemoteChannel.mm', u'Source/WebKit/mac/WebInspector/remote/WebInspectorServer.h', u'Source/WebKit/mac/WebInspector/remote/WebInspectorServer.mm', u'Source/WebKit/mac/WebInspector/remote/WebInspectorServerWebViewConnection.h', u'Source/WebKit/mac/WebInspector/remote/WebInspectorServerWebViewConnection.mm', u'Source/WebKit/mac/WebInspector/remote/WebInspectorServerWebViewConnectionController.h', u'Source/WebKit/mac/WebInspector/remote/WebInspectorServerWebViewConnectionController.mm', u'Source/WebKit/mac/WebInspector/remote/WebInspectorXPCWrapper.h', u'Source/WebKit/mac/WebInspector/remote/WebInspectorXPCWrapper.m', u'Source/WebKit/mac/WebKit.exp', u'Source/WebKit/mac/WebView/WebView.mm', u'Source/WebKit/mac/WebView/WebViewData.h', u'Source/WebKit/mac/WebView/WebViewData.mm', u'Source/WebKit/mac/WebView/WebViewInternal.h', u'Source/WebKit/mac/WebView/WebViewPrivate.h', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/Configurations/FeatureDefines.xcconfig']" exit_code: 1 Source/WebKit/mac/WebCoreSupport/WebInspectorClient.h:89: The parameter name "remoteChannel" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebKit/mac/WebInspector/remote/WebInspectorXPCWrapper.h:39: This { should be at the end of the previous line [whitespace/braces] [4] Source/WebKit/mac/WebInspector/remote/WebInspectorXPCWrapper.h:49: Extra space before ( in function call [whitespace/parens] [4] Source/WebKit/mac/WebInspector/remote/WebInspectorXPCWrapper.h:50: Extra space before ( in function call [whitespace/parens] [4] Source/WebKit/mac/WebInspector/remote/WebInspectorXPCWrapper.h:51: Extra space before ( in function call [whitespace/parens] [4] Source/WebKit/mac/WebInspector/remote/WebInspectorXPCWrapper.h:52: Extra space before ( in function call [whitespace/parens] [4] Source/WebKit/mac/WebInspector/remote/WebInspectorClientRegistry.h:42: Extra space before ( in function call [whitespace/parens] [4] Source/WebKit/mac/ChangeLog:83: ChangeLog entry has no bug number [changelog/bugnumber] [5] Source/WTF/wtf/FeatureDefines.h:873: Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] Total errors found: 9 in 40 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 214782 [details] [PATCH] Add ENABLE(REMOTE_INSPECTOR) Implementation Attachment 214782 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/8848137
Comment on attachment 214782 [details] [PATCH] Add ENABLE(REMOTE_INSPECTOR) Implementation Attachment 214782 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/8828113
Comment on attachment 214782 [details] [PATCH] Add ENABLE(REMOTE_INSPECTOR) Implementation View in context: https://bugs.webkit.org/attachment.cgi?id=214782&action=review >> Source/WTF/wtf/FeatureDefines.h:873 >> +#error "ENABLE(REMOTE_INSPECTOR) requires ENABLE(INSPECTOR) > > Multi-line string ("...") found. This lint script doesn't do well with such strings, and may give bogus warnings. They're ugly and unnecessary, and you should use concatenation instead". [readability/multiline_string] [5] lol, yes it looks like I'm missing an ending quote.
Comment on attachment 214782 [details] [PATCH] Add ENABLE(REMOTE_INSPECTOR) Implementation View in context: https://bugs.webkit.org/attachment.cgi?id=214782&action=review > Source/WebKit/mac/WebCoreSupport/WebInspectorClient.mm:256 > + Page* page = core(m_webView); > + if (page) { > + page->inspectorController()->disconnectFrontend(); > + } Could put Page* page in the if() and drop the { }. > Source/WebKit/mac/WebInspector/remote/WebInspectorRelayDefinitions.h:38 > +#define WIRSimulatorTCPPortNumber 27753 > +#define WIRXPCMachPortName "com.apple.webinspector" > +#define WIRServiceAvailableNotification "com.apple.webinspectord.available" > +#define WIRServiceAvailabilityCheckNotification "com.apple.webinspectord.availability_check" > +#define WIRServiceEnabledNotification "com.apple.webinspectord.enabled" > +#define WIRServiceDisabledNotification "com.apple.webinspectord.disabled" These could be const NSString * instead. Also there isn't any reason for the symbol to keep the "WIR" prefix, it could differ even if the string content needs to keep "WIR". > Source/WebKit/mac/WebInspector/remote/WebInspectorServerWebViewConnectionController.mm:281 > + NSString *connectionIdentifier = [dictionary objectForKey:WIRConnectionIdentifierKey]; > + [self pushListing:connectionIdentifier]; One line?
Comment on attachment 214782 [details] [PATCH] Add ENABLE(REMOTE_INSPECTOR) Implementation View in context: https://bugs.webkit.org/attachment.cgi?id=214782&action=review >> Source/WebKit/mac/WebInspector/remote/WebInspectorRelayDefinitions.h:38 >> +#define WIRServiceDisabledNotification "com.apple.webinspectord.disabled" > > These could be const NSString * instead. Also there isn't any reason for the symbol to keep the "WIR" prefix, it could differ even if the string content needs to keep "WIR". These are used by XPC/notify APIs which take C strings, so I just kept them C String constants. Would there be an advantage to making them NSStrings? Just seems like more work unless they are actually used as NSStrings. I agree that there is no need to keep the defines WIR prefixed. The consistency, and prefix makes it obvious to me that these are shared constants. If I changed them, I'd like to change them to "RWI", but of course the values need to stay "WIR" for backwards compatibility.
(In reply to comment #7) > (From update of attachment 214782 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=214782&action=review > > >> Source/WebKit/mac/WebInspector/remote/WebInspectorRelayDefinitions.h:38 > >> +#define WIRServiceDisabledNotification "com.apple.webinspectord.disabled" > > > > These could be const NSString * instead. Also there isn't any reason for the symbol to keep the "WIR" prefix, it could differ even if the string content needs to keep "WIR". > > These are used by XPC/notify APIs which take C strings, so I just kept them C String constants. Would there be an advantage to making them NSStrings? Just seems like more work unless they are actually used as NSStrings. > > I agree that there is no need to keep the defines WIR prefixed. The consistency, and prefix makes it obvious to me that these are shared constants. If I changed them, I'd like to change them to "RWI", but of course the values need to stay "WIR" for backwards compatibility. You mentioned this could be "const char*" types. I'll address this in a follow-up.
Landed in: <http://trac.webkit.org/changeset/158050>