Bug 221871 - Web Inspector: Add a way to wake up debuggables to the remote inspector protocol
Summary: Web Inspector: Add a way to wake up debuggables to the remote inspector protocol
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Local Build
Hardware: All All
: P2 Normal
Assignee: Timothy Hatcher
URL:
Keywords: InRadar
Depends on:
Blocks: 221933
  Show dependency treegraph
 
Reported: 2021-02-13 15:35 PST by Timothy Hatcher
Modified: 2021-02-15 16:16 PST (History)
11 users (show)

See Also:


Attachments
Patch (8.79 KB, patch)
2021-02-13 15:40 PST, Timothy Hatcher
no flags Details | Formatted Diff | Diff
Patch (8.96 KB, patch)
2021-02-15 15:18 PST, Timothy Hatcher
no flags Details | Formatted Diff | Diff
Patch (8.95 KB, patch)
2021-02-15 15:30 PST, Timothy Hatcher
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Timothy Hatcher 2021-02-13 15:35:32 PST
We need a way to wake up debuggables on the target side for an application.

rdar://70351644
Comment 1 Timothy Hatcher 2021-02-13 15:40:45 PST
Created attachment 420228 [details]
Patch
Comment 2 Devin Rousso 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.
Comment 3 Timothy Hatcher 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.
Comment 4 Timothy Hatcher 2021-02-15 15:18:12 PST
Created attachment 420385 [details]
Patch
Comment 5 Devin Rousso 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 :)
Comment 6 Timothy Hatcher 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.
Comment 7 Timothy Hatcher 2021-02-15 15:30:50 PST
Created attachment 420387 [details]
Patch
Comment 8 EWS 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].