Bug 221871

Summary: Web Inspector: Add a way to wake up debuggables to the remote inspector protocol
Product: WebKit Reporter: Timothy Hatcher <timothy>
Component: Web InspectorAssignee: Timothy Hatcher <timothy>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, ews-watchlist, hi, inspector-bugzilla-changes, joepeck, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 221933    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Timothy Hatcher
Reported 2021-02-13 15:35:32 PST
We need a way to wake up debuggables on the target side for an application. rdar://70351644
Attachments
Patch (8.79 KB, patch)
2021-02-13 15:40 PST, Timothy Hatcher 		no flags
DetailsFormatted DiffDiff
Patch (8.96 KB, patch)
2021-02-15 15:18 PST, Timothy Hatcher 		no flags
DetailsFormatted DiffDiff
Patch (8.95 KB, patch)
2021-02-15 15:30 PST, Timothy Hatcher 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Timothy Hatcher
Comment 1 2021-02-13 15:40:45 PST
Created attachment 420228 [details] Patch
Devin Rousso
Comment 2 2021-02-15 14:56:09 PST
Comment on attachment 420228 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420228&action=review > Source/WebKit/UIProcess/Cocoa/AutomationClient.mm:96 > +void AutomationClient::requestedDebuggablesToWakeUp() This feels kinda wrong IMO. I don't think an Inspector feature should rely on anything `Automation*`. I think it would be better to rename `RemoteInspector::Client` to `RemoteInspector::AutomationClient` and create a new `RemoteInspector::Client` that is initialized/inherited elsewhere.
Timothy Hatcher
Comment 3 2021-02-15 15:05:08 PST
Comment on attachment 420228 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420228&action=review >> Source/WebKit/UIProcess/Cocoa/AutomationClient.mm:96 >> +void AutomationClient::requestedDebuggablesToWakeUp() > > This feels kinda wrong IMO. I don't think an Inspector feature should rely on anything `Automation*`. I think it would be better to rename `RemoteInspector::Client` to `RemoteInspector::AutomationClient` and create a new `RemoteInspector::Client` that is initialized/inherited elsewhere. It is only called `AutomationClient` in WebKit. It is technically the `RemoteInspectorClient`. It should be renamed at some point.
Timothy Hatcher
Comment 4 2021-02-15 15:18:12 PST
Created attachment 420385 [details] Patch
Devin Rousso
Comment 5 2021-02-15 15:23:47 PST
Comment on attachment 420385 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420385&action=review > Source/WebKit/UIProcess/API/Cocoa/_WKAutomationDelegate.h:39 > +// FIXME: Consider renaming WebKit::AutomationClient and _WKAutomationDelegate to _WKInspectorDelegate since it isn't only used for automation now. http://webkit.org/b/221933 NIT: I'd put this in `RemoteInspector::receivedWakeUpDebuggables` and `AutomationClient::requestedDebuggablesToWakeUp` too so that it's more visible and less likely to be missed/forgotten :)
Timothy Hatcher
Comment 6 2021-02-15 15:30:32 PST
Comment on attachment 420385 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420385&action=review >> Source/WebKit/UIProcess/API/Cocoa/_WKAutomationDelegate.h:39 >> +// FIXME: Consider renaming WebKit::AutomationClient and _WKAutomationDelegate to _WKInspectorDelegate since it isn't only used for automation now. http://webkit.org/b/221933 > > NIT: I'd put this in `RemoteInspector::receivedWakeUpDebuggables` and `AutomationClient::requestedDebuggablesToWakeUp` too so that it's more visible and less likely to be missed/forgotten :) I don't think `RemoteInspector::receivedWakeUpDebuggables` is a good place for that comment since RemoteInspector is the right place. The WebKit side is what needs changed.
Timothy Hatcher
Comment 7 2021-02-15 15:30:50 PST
Created attachment 420387 [details] Patch
EWS
Comment 8 2021-02-15 16:16:54 PST
Committed r272889: <https://commits.webkit.org/r272889> All reviewed patches have been landed. Closing bug and clearing flags on attachment 420387 [details].
Note You need to log in before you can comment on or make changes to this bug.