<rdar://78237764>
<rdar://problem/78768633>
Created attachment 430392 [details] Patch v1.0
Correct radar is <rdar://78237764>
Comment on attachment 430392 [details] Patch v1.0 View in context: https://bugs.webkit.org/attachment.cgi?id=430392&action=review r=me, nice fix with a great explanation :) > Source/JavaScriptCore/inspector/remote/RemoteInspector.h:263 > + bool m_allowRetryRelayConnection { false }; NIT: How about something a bit more specific/explanatory like `m_shouldReconnectToRelayOnFailure`? > Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm:281 > + m_relayConnection->sendMessage(@"syn", nil); NIT: I wonder if maybe we should give this a different name so it shows up differently in logging. If we do keep the same name, perhaps we should create a static constant instead of hardcoding in both spots? > Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm:376 > + if (m_allowRetryRelayConnection) { Style: I'd make this into an early return > Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm:377 > + WTFLogAlways("RemoteInspector XPC connection to relay failed, reattempting connection..."); NIT: s/reattempting connection/reconnecting/ > Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm:380 > + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ Should we maybe wait a second or two before attempting to reconnect? > Source/JavaScriptCore/inspector/remote/cocoa/RemoteInspectorCocoa.mm:384 > + m_allowRetryRelayConnection = false; NIT: I'd put this as the first thing in the `if` so that if someone adds an early return in the future it's less likely to be missed/forgotten
Created attachment 430484 [details] Patch v1.1 - Review nits
Created attachment 430491 [details] Patch v1.1.1 - Add reviewer to changelog
Committed r278413 (238438@main): <https://commits.webkit.org/238438@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 430491 [details].