* SUMMARY Put ScriptDebugServer into InspectorEnvironment and cleanup duplicate references. Currently InspectorDebuggerAgent subclasses create unique ScriptDebugServer subclasses and InspectorController's handle passing it around to other agents. This is not very clean and as more agents want to use JSC::Debugger/ScriptDebugServer this will be problematic. Since Inspectors will always have a ScriptDebugServer put it into the InspectorEnvironment (the Controllers). With ScriptDebugServer in the Environment: - JS Agents can access it if they need it in construction - All Web Agents have a convenience to access the environment Do the same type of clean up with VM access. RuntimeAgent's have globalVM() which essentially does the same thing but VM was already put into InspectorEnvironment. So clean things up there as well.
<rdar://problem/23385443>
Created attachment 264759 [details] [PATCH] Proposed Fix
Comment on attachment 264759 [details] [PATCH] Proposed Fix Attachment 264759 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/379243 Number of test failures exceeded the failure limit.
Created attachment 264762 [details] Archive of layout-test-results from ews116 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-yosemite Platform: Mac OS X 10.10.5
Reproduced the debug crash locally. shell> run-webkit-tests --debug fast/workers ASSERTION FAILED: isMainThread() /Users/pecoraro/Code/safari/OpenSource/Source/WebCore/bindings/js/JSDOMWindowBase.cpp(249) : static JSC::VM &WebCore::JSDOMWindowBase::commonVM() 1 0x10d9762d0 WTFCrash 2 0x110661609 WebCore::JSDOMWindowBase::commonVM() 3 0x1119d4a11 WebCore::WorkerInspectorController::vm() 4 0x10d4b7cd0 Inspector::InspectorRuntimeAgent::InspectorRuntimeAgent(Inspector::AgentContext&) 5 0x1119f2ad7 WebCore::WorkerRuntimeAgent::WorkerRuntimeAgent(WebCore::WorkerAgentContext&) 6 0x1119f2b7d WebCore::WorkerRuntimeAgent::WorkerRuntimeAgent(WebCore::WorkerAgentContext&) 7 0x1119d4d87 std::_Unique_if<WebCore::WorkerRuntimeAgent>::_Single_object std::make_unique<WebCore::WorkerRuntimeAgent, WebCore::WorkerAgentContext&>(WebCore::WorkerAgentContext&&&) 8 0x1119d28c6 WebCore::WorkerInspectorController::WorkerInspectorController(WebCore::WorkerGlobalScope&) 9 0x1119d3c8d WebCore::WorkerInspectorController::WorkerInspectorController(WebCore::WorkerGlobalScope&) 10 0x1119cc8d7 std::_Unique_if<WebCore::WorkerInspectorController>::_Single_object std::make_unique<WebCore::WorkerInspectorController, WebCore::WorkerGlobalScope&>(WebCore::WorkerGlobalScope&&&) 11 0x1119c8b0f WebCore::WorkerGlobalScope::WorkerGlobalScope(WebCore::URL const&, WTF::String const&, WebCore::WorkerThread&, WTF::PassRefPtr<WebCore::SecurityOrigin>) 12 0x10fb5e487 WebCore::DedicatedWorkerGlobalScope::DedicatedWorkerGlobalScope(WebCore::URL const&, WTF::String const&, WebCore::DedicatedWorkerThread&, WTF::PassRefPtr<WebCore::SecurityOrigin>) 13 0x10fb5e40d WebCore::DedicatedWorkerGlobalScope::DedicatedWorkerGlobalScope(WebCore::URL const&, WTF::String const&, WebCore::DedicatedWorkerThread&, WTF::PassRefPtr<WebCore::SecurityOrigin>) 14 0x10fb5e37a WebCore::DedicatedWorkerGlobalScope::create(WebCore::URL const&, WTF::String const&, WebCore::DedicatedWorkerThread&, WTF::String const&, WebCore::ContentSecurityPolicy::HeaderType, WTF::PassRefPtr<WebCore::SecurityOrigin>) 15 0x10fb6050a WebCore::DedicatedWorkerThread::createWorkerGlobalScope(WebCore::URL const&, WTF::String const&, WTF::String const&, WebCore::ContentSecurityPolicy::HeaderType, WTF::PassRefPtr<WebCore::SecurityOrigin>) 16 0x1119f6dd7 WebCore::WorkerThread::workerThread()
Note to self, WorkerInspectorController should use the WorkerGlobalObject's vm via ScriptExecutionContext::vm, not the JSDOMWindow::commonVM. This was a latent bug caught by this! That said, we don't have worker debugging enabled.
Comment on attachment 264759 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=264759&action=review It looks pretty good to me. Please fix the crash, though. > Source/JavaScriptCore/inspector/JSGlobalObjectInspectorController.cpp:87 > auto consoleAgent = std::make_unique<JSGlobalObjectConsoleAgent>(context); Can more of these be inlined below? > Source/WebCore/inspector/InspectorWebAgentBase.h:75 > + , m_environment(context.environment) Can we do something similar for JSC agents? It would be nice.
Created attachment 264806 [details] [PATCH] Proposed Fix
Comment on attachment 264806 [details] [PATCH] Proposed Fix r=me
Comment on attachment 264806 [details] [PATCH] Proposed Fix Clearing flags on attachment: 264806 Committed r192061: <http://trac.webkit.org/changeset/192061>
All reviewed patches have been landed. Closing bug.