Bug 150869 – Web Inspector: Put ScriptDebugServer into InspectorEnvironment and cleanup duplicate references

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

* 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.

[Radar WebKit Bug Importer]

<rdar://problem/23385443>

[Joseph Pecoraro]

Created attachment 264759 [details]
[PATCH] Proposed Fix

[Build Bot]

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.

[Build Bot]

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

[Joseph Pecoraro]

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()

[Joseph Pecoraro]

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.

[Blaze Burg]

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.

[Joseph Pecoraro]

Created attachment 264806 [details]
[PATCH] Proposed Fix

[Blaze Burg]

Comment on attachment 264806 [details]
[PATCH] Proposed Fix

r=me

[WebKit Commit Bot]

Comment on attachment 264806 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 264806

Committed r192061: <http://trac.webkit.org/changeset/192061>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.