Bug 196725 – Web Inspector: REGRESSION: lazy agents used outside of frontend/instrumentation can be accessed before being created

• Component: Web Inspector
• Status: RESOLVED FIXED

[Devin Rousso]

`WebCore::InspectorFrontendClientLocal::showMainResourceForFrame` and `WebKit::WebInspector::showMainResourceForFrame` both access `WebCore::InspectorController::pageAgent`, even though the underlying `m_pageAgent` isn't instantiated until the first frontend connects.

[Devin Rousso]

<rdar://problem/49669810>

[Devin Rousso]

Created attachment 367025 [details]
Patch

[Devin Rousso]

Created attachment 367064 [details]
Patch

[Timothy Hatcher]

Comment on attachment 367064 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=367064&action=review

> Source/WebCore/inspector/InspectorController.h:110
> -    InspectorPageAgent* pageAgent() const { return m_pageAgent; }
> +
> +    InspectorDOMAgent& ensureDOMAgent();
> +    WEBCORE_EXPORT InspectorPageAgent& ensurePageAgent();

Why not just have pageAgent() and a domAgent() getter create the agents if needed and drop the need for the ensure functions?

[Devin Rousso]

Comment on attachment 367064 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=367064&action=review

>> Source/WebCore/inspector/InspectorController.h:110
>> +    WEBCORE_EXPORT InspectorPageAgent& ensurePageAgent();
> 
> Why not just have pageAgent() and a domAgent() getter create the agents if needed and drop the need for the ensure functions?

I vaguely remember there being a discussion about this style on one of the webkit-* mailing lists.  I prefer the "ensure" as it makes it clearer to callers that this function might create an object, rather than just return whatever currently exists.

[WebKit Commit Bot]

Comment on attachment 367064 [details]
Patch

Clearing flags on attachment: 367064

Committed r244167: <https://trac.webkit.org/changeset/244167>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.