NEW 200854
Web Inspector: implement a consistent style and lifecycle for all agents
https://bugs.webkit.org/show_bug.cgi?id=200854
Summary Web Inspector: implement a consistent style and lifecycle for all agents
Devin Rousso
Reported 2019-08-16 23:59:16 PDT
After speaking at length offline with Joe about what `enable`/`disable` represent, we've come to the following conclusion: An agent's `enable`/`disable` controls whether that agent is "allowed" to send events to the connected frontend. For now, an agent isn't "required" to be `enable`d in order for commands being sent from the frontend to the backend to have any effect (some agents already work this way). Agents that need other agents to be `enable`d shouldn't ever assume anything about the state of the other agent, and therefore should properly handle the case where that other agent isn't enabled (often this is just returning an error from the command or not sending an event). As such, we should: - add `enable`/`disable` to all domains that have events - re-add them to the `Page` domain (partial "undo" of r248454) - this does NOT include the `DOM` domain, as that's going to be handled by <https://webkit.org/b/197025> - add error messages when `enable` is called if the domain is already enabled - add proper checks when agents are referenced by other agents I think we should move towards a world where `enable`/`disable` is the single entry/exit point for ALL agents, but that can come later.
Attachments
Patch (220.02 KB, patch)
2019-08-17 00:11 PDT, Devin Rousso
no flags
Patch (219.95 KB, patch)
2019-08-17 00:18 PDT, Devin Rousso
no flags
Archive of layout-test-results from ews113 for mac-highsierra (545.36 KB, application/zip)
2019-08-17 01:22 PDT, EWS Watchlist
no flags
Archive of layout-test-results from ews103 for mac-highsierra (3.23 MB, application/zip)
2019-08-17 01:24 PDT, EWS Watchlist
no flags
Patch (224.07 KB, patch)
2019-08-17 02:15 PDT, Devin Rousso
no flags
Patch (224.08 KB, patch)
2019-08-17 02:19 PDT, Devin Rousso
no flags
Archive of layout-test-results from ews100 for mac-highsierra (3.31 MB, application/zip)
2019-08-17 03:30 PDT, EWS Watchlist
no flags
Archive of layout-test-results from ews114 for mac-highsierra (1.50 MB, application/zip)
2019-08-17 03:41 PDT, EWS Watchlist
no flags
Patch (224.14 KB, patch)
2019-08-17 17:29 PDT, Devin Rousso
no flags
Archive of layout-test-results from ews100 for mac-highsierra (3.42 MB, application/zip)
2019-08-17 18:21 PDT, EWS Watchlist
no flags
Archive of layout-test-results from ews113 for mac-highsierra (1.64 MB, application/zip)
2019-08-17 18:35 PDT, EWS Watchlist
no flags
Patch (225.92 KB, patch)
2019-08-18 03:39 PDT, Devin Rousso
no flags
Archive of layout-test-results from ews117 for mac-highsierra (3.08 MB, application/zip)
2019-08-18 05:28 PDT, EWS Watchlist
no flags
Patch (232.43 KB, patch)
2019-08-18 10:25 PDT, Devin Rousso
no flags
Devin Rousso
Comment 1 2019-08-17 00:11:09 PDT
Created attachment 376594 [details] Patch This _could_ be two or three separate patches, but I'd like to see how EWS feels about the entire thing all at once :)
Devin Rousso
Comment 2 2019-08-17 00:18:21 PDT
EWS Watchlist
Comment 3 2019-08-17 01:22:33 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 4 2019-08-17 01:22:35 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 5 2019-08-17 01:24:02 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 6 2019-08-17 01:24:03 PDT Comment hidden (obsolete)
Devin Rousso
Comment 7 2019-08-17 02:15:19 PDT
Devin Rousso
Comment 8 2019-08-17 02:19:26 PDT
EWS Watchlist
Comment 9 2019-08-17 03:30:31 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 10 2019-08-17 03:30:33 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 11 2019-08-17 03:41:38 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 12 2019-08-17 03:41:40 PDT Comment hidden (obsolete)
Devin Rousso
Comment 13 2019-08-17 17:29:48 PDT
EWS Watchlist
Comment 14 2019-08-17 18:21:07 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 15 2019-08-17 18:21:09 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 16 2019-08-17 18:35:31 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 17 2019-08-17 18:35:33 PDT Comment hidden (obsolete)
Devin Rousso
Comment 18 2019-08-18 03:39:51 PDT
EWS Watchlist
Comment 19 2019-08-18 05:28:34 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 20 2019-08-18 05:28:36 PDT Comment hidden (obsolete)
Devin Rousso
Comment 21 2019-08-18 10:25:12 PDT
Created attachment 376642 [details] Patch Rather than have subclasses implement the command variant of `enable`/`disable`, create a separate protected member function that is called by the command variant so that the `errorString` doesn't need to be propagated
Radar WebKit Bug Importer
Comment 22 2019-08-20 09:21:27 PDT
Timothy Hatcher
Comment 23 2020-02-14 14:23:36 PST
Comment on attachment 376642 [details] Patch Clearing review. I doubt this applies anymore.
Note You need to log in before you can comment on or make changes to this bug.