Bug 195973

Summary: Web Inspector: Debugger: lazily create the agent
Product: WebKit Reporter: Devin Rousso <hi>
Component: Web InspectorAssignee: Devin Rousso <hi>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ews-watchlist, hi, inspector-bugzilla-changes, joepeck, keith_miller, mark.lam, msaboff, rniwa, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Archive of layout-test-results from ews106 for mac-highsierra-wk2
none
Archive of layout-test-results from ews117 for mac-highsierra none

Devin Rousso
Reported 2019-03-19 15:57:02 PDT
.
Attachments
Patch (20.75 KB, patch)
2019-03-19 16:53 PDT, Devin Rousso
no flags
Patch (20.56 KB, patch)
2019-03-19 18:14 PDT, Devin Rousso
no flags
Archive of layout-test-results from ews106 for mac-highsierra-wk2 (2.91 MB, application/zip)
2019-03-19 22:18 PDT, EWS Watchlist
no flags
Archive of layout-test-results from ews117 for mac-highsierra (2.57 MB, application/zip)
2019-03-19 22:30 PDT, EWS Watchlist
no flags
Radar WebKit Bug Importer
Comment 1 2019-03-19 15:57:18 PDT
Devin Rousso
Comment 2 2019-03-19 16:53:46 PDT
Joseph Pecoraro
Comment 3 2019-03-19 17:32:35 PDT
Comment on attachment 365264 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=365264&action=review > Source/JavaScriptCore/inspector/JSGlobalObjectInspectorController.cpp:245 > + if (m_debuggerAgent) { This should always be true. I'd drop the if check. The only path to get here (that we expect) is through InspectorAgent::initialized meaning the agents are alive and a frontend sent a message. > Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp:107 > + for (auto* listener : copyToVector(m_listeners)) > + listener->debuggerWasDisabled(); Why upgrade this to a list. Do you expect to add more listeners?
Devin Rousso
Comment 4 2019-03-19 18:13:40 PDT
Comment on attachment 365264 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=365264&action=review >> Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp:107 >> + listener->debuggerWasDisabled(); > > Why upgrade this to a list. Do you expect to add more listeners? No "good" reason other than that it seemed "wrong" to only allow for one listener. This way, it also matches `JavaScriptCore::ScriptDebugServer`.
Devin Rousso
Comment 5 2019-03-19 18:14:06 PDT
EWS Watchlist
Comment 6 2019-03-19 22:18:50 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 7 2019-03-19 22:18:52 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 8 2019-03-19 22:30:13 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 9 2019-03-19 22:30:15 PDT Comment hidden (obsolete)
WebKit Commit Bot
Comment 10 2019-03-19 22:40:17 PDT
Comment on attachment 365278 [details] Patch Clearing flags on attachment: 365278 Committed r243192: <https://trac.webkit.org/changeset/243192>
WebKit Commit Bot
Comment 11 2019-03-19 22:40:19 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.