Bug 192669 – Web Inspector: Timelines: correctly label Intersection Observer callbacks

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

[Simon Fraser (smfr)]

Intersection Observer callbacks currently show as "Script evaluated", which is misleading (no eval is involved). They need to be correctly labeled as "Intersection Observer callbacks" or something.

[Radar WebKit Bug Importer]

<rdar://problem/46702490>

[Joseph Pecoraro]

Extra data, such as a name, could probably be provided at these points:

    bindings/js/JSCallbackData.cpp
    78:    InspectorInstrumentationCookie cookie = JSExecState::instrumentFunctionCall(context, callType, callData);

    bindings/js/JSEventListener.cpp
    171:    InspectorInstrumentationCookie cookie = JSExecState::instrumentFunctionCall(&scriptExecutionContext, callType, callData);

    bindings/js/JSErrorHandler.cpp
    102:        InspectorInstrumentationCookie cookie = JSExecState::instrumentFunctionCall(&scriptExecutionContext, callType, callData);

    bindings/js/ScheduledAction.cpp
    115:    InspectorInstrumentationCookie cookie = JSExecState::instrumentFunctionCall(&context, callType, callData);

[Simon Fraser (smfr)]

Created attachment 357638 [details]
Patch

[EWS Watchlist]

This patch modifies the inspector protocol. Please ensure that any frontend changes appropriately use feature checks for new protocol features.

[Joseph Pecoraro]

Comment on attachment 357638 [details]
Patch

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

r=me

We should consider writing a test for this in:

    LayoutTests/inspector/timeline

But it looks like we don't have any tests for generic timeline data. Feel free to file a bug on me to add inspector tests, and attach any test page content you have for observers. Or if you want I can guide you through a test that might not be too difficult.

> Source/WebCore/dom/MutationObserver.cpp:253
> +        InspectorInstrumentationCookie cookie = InspectorInstrumentation::willFireObserverCallback(*context, "Mutation Observer"_s);

Should we drop the space? The name of the class is `MutationObserver`.

I think that would read fine without having to localize it to something like "Foo Observer".

> Source/WebCore/inspector/InspectorInstrumentation.h:1433
> +    if (InstrumentingAgents* instrumentingAgents = instrumentingAgentsForContext(context))

Please insert a fast return to do as little work as possible if no inspector is open:

    FAST_RETURN_IF_NO_FRONTENDS(InspectorInstrumentationCookie());

> Source/WebCore/page/IntersectionObserver.cpp:264
> +    InspectorInstrumentationCookie cookie = InspectorInstrumentation::willFireObserverCallback(*context, "Intersection Observer"_s);

Ditto regarding space.

> Source/WebCore/page/PerformanceObserver.cpp:111
> +    InspectorInstrumentationCookie cookie = InspectorInstrumentation::willFireObserverCallback(*context, "Performance Observer"_s);

Ditto regarding space.

[Simon Fraser (smfr)]

https://trac.webkit.org/changeset/239397/webkit