| Summary: | Web Inspector: Debugger: lazily create the agent | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Devin Rousso <hi> | ||||||||||
| Component: | Web Inspector | Assignee: | 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
Devin Rousso
2019-03-19 15:57:02 PDT
Created attachment 365264 [details]
Patch
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? 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`. Created attachment 365278 [details]
Patch
Comment on attachment 365278 [details] Patch Attachment 365278 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11575533 New failing tests: http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable.html Created attachment 365310 [details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 365278 [details] Patch Attachment 365278 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/11575413 New failing tests: http/tests/security/cross-origin-worker-indexeddb.html Created attachment 365313 [details]
Archive of layout-test-results from ews117 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 365278 [details] Patch Clearing flags on attachment: 365278 Committed r243192: <https://trac.webkit.org/changeset/243192> All reviewed patches have been landed. Closing bug. |