[webkit-reviews] review granted: [Bug 123111] Upstream ENABLE(REMOTE_INSPECTOR) and enable on iOS and Mac : [Attachment 214782] [PATCH] Add ENABLE(REMOTE_INSPECTOR) Implementation

[bugzilla-daemon at webkit.org]

Timothy Hatcher <timothy at apple.com> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 123111: Upstream ENABLE(REMOTE_INSPECTOR) and enable on iOS and Mac
https://bugs.webkit.org/show_bug.cgi?id=123111

Attachment 214782: [PATCH] Add ENABLE(REMOTE_INSPECTOR) Implementation
https://bugs.webkit.org/attachment.cgi?id=214782&action=review

------- Additional Comments from Timothy Hatcher <timothy at apple.com>
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/WebInspectorServerWebViewConnectionContro
ller.mm:281
> +    NSString *connectionIdentifier = [dictionary
objectForKey:WIRConnectionIdentifierKey];
> +    [self pushListing:connectionIdentifier];

One line?