Bug 65105 – Web Inspector: Add resource initiator column to network panel.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Vsevolod Vlasov]

Add resource initiator column to network panel.

[Vsevolod Vlasov]

Created attachment 101903 [details]
Patch

[Joseph Pecoraro]

Neat. Could you add a screen shot of what this looks like?

Making the timeline graph part of the networking panel is making the
networking panel less and less useful at a glance. Instead of getting
a feel for how the page loaded, I'm instead seeing a big table. Just
something to think about.

[Joseph Pecoraro]

(In reply to comment #2)
> Making the timeline graph part of the networking panel
> is making the networking panel less and less useful at a glance.

Should read "Making the timeline graph part of the panel narrower
is making the panel less useful at a glance..."

[Pavel Feldman]

(In reply to comment #3)
> (In reply to comment #2)
> > Making the timeline graph part of the networking panel
> > is making the networking panel less and less useful at a glance.
> 
> Should read "Making the timeline graph part of the panel narrower
> is making the panel less useful at a glance..."

Large timeline does not help in case of dynamic sites. XHRs quickly draw timeline part unusable (see the screenshot). We just need to make it zoom as the real timeline panel.

[Pavel Feldman]

Created attachment 101910 [details]
[IMAGE] Screenshot on why timeline size does not matter.

[Joseph Pecoraro]

> Large timeline does not help in case of dynamic sites. XHRs quickly draw timeline
> part unusable (see the screenshot). We just need to make it zoom as the real timeline panel.

Yep, I totally agree with you there.

[Pavel Feldman]

Comment on attachment 101903 [details]
Patch

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

> Source/WebCore/inspector/Inspector.json:466
> +                "description": "Information about the cached resource.",

cached resource load initiator?

> Source/WebCore/inspector/Inspector.json:468
> +                    { "name": "type", "$ref": "InitiatorType", "description": "Type of this initiator." },

You could inline enum definition here, no need for top-level class definition.

> Source/WebCore/inspector/Inspector.json:469
> +                    { "name": "url", "type": "string", "description": "Initiator URL." },

Please use existing Location type.

> Source/WebCore/inspector/Inspector.json:608
>                      { "name": "resource", "$ref": "CachedResource", "description": "Cached resource data." }

Drive-by comment: why does it miss stackTrace field?

> Source/WebCore/inspector/InspectorInstrumentation.cpp:392
> +        resourceAgent->willRecalculateStyle();

This sounds weird: theoretically, network agent should not care about styles.

> Source/WebCore/inspector/InspectorInstrumentation.cpp:410
> +        resourceAgent->didScheduleStyleRecalculation(document);

ditto

> Source/WebCore/inspector/InspectorResourceAgent.cpp:367
> +    m_scheduledStyleRecalculationInitiator = buildInitiatorObject(document);

You should either ASSERT or check for it being nullptr prior to assigning.

> Source/WebCore/inspector/InspectorResourceAgent.cpp:379
> +        lineNumber = stackTrace->at(0).lineNumber();

I'd suggest that you send the entire stack.

[Vsevolod Vlasov]

Created attachment 102117 [details]
Screenshot of proposed feature

[Vsevolod Vlasov]

(In reply to comment #7)
> (From update of attachment 101903 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=101903&action=review
> 
> > Source/WebCore/inspector/Inspector.json:466
> > +                "description": "Information about the cached resource.",
> 
> cached resource load initiator?
Done.

> > Source/WebCore/inspector/Inspector.json:468
> > +                    { "name": "type", "$ref": "InitiatorType", "description": "Type of this initiator." },
> 
> You could inline enum definition here, no need for top-level class definition.
Done. 

> > Source/WebCore/inspector/Inspector.json:469
> > +                    { "name": "url", "type": "string", "description": "Initiator URL." },
> 
> Please use existing Location type.
Location type uses sourceId, not url. Can not use it here.

> > Source/WebCore/inspector/Inspector.json:608
> >                      { "name": "resource", "$ref": "CachedResource", "description": "Cached resource data." }
> 
> Drive-by comment: why does it miss stackTrace field?
This stackTrace is only used for console messages and resources from memory cache are not likely to have network errors. 
 
> > Source/WebCore/inspector/InspectorInstrumentation.cpp:392
> > +        resourceAgent->willRecalculateStyle();
> 
> This sounds weird: theoretically, network agent should not care about styles.
I abstracted style concept out of resourceAgent, so that it thinks of it as of deferred resource initiation.
 
> > Source/WebCore/inspector/InspectorResourceAgent.cpp:367
> > +    m_scheduledStyleRecalculationInitiator = buildInitiatorObject(document);
> 
> You should either ASSERT or check for it being nullptr prior to assigning.
Done.
 
> > Source/WebCore/inspector/InspectorResourceAgent.cpp:379
> > +        lineNumber = stackTrace->at(0).lineNumber();
> 
> I'd suggest that you send the entire stack.
Done.

[Vsevolod Vlasov]

Created attachment 102135 [details]
Patch, comments addressed

[Pavel Feldman]

Comment on attachment 102135 [details]
Patch, comments addressed

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

> Source/WebCore/inspector/Inspector.json:467
> +                    { "name": "type", "type": "string", "enum": ["Parser", "Script", "Other"], "description": "Type of this initiator." },

we tend to keep enum values lower-case

> Source/WebCore/inspector/Inspector.json:468
> +                    { "name": "stackTrace", "type": "array", "items": { "$ref": "Console.CallFrame"}, "description": "Initiator JavaScript stack trace, set for Script only." },

Isn't there a stack trace structure?

> Source/WebCore/inspector/Inspector.json:469
> +                    { "name": "url", "type": "string", "description": "Initiator URL, set for Parser type only." },

"optional": true for all of them.

> Source/WebCore/inspector/InspectorInstrumentation.cpp:392
> +        resourceAgent->willStartDeferredResourcesInitiation();

willStartInitiation sounds weird. scheduleDeferredResourceLoad() ?

> Source/WebCore/inspector/InspectorResourceAgent.cpp:392
> +    initiatorObject->setString("url", url);

You should either put top stack frame values here or not send them at all (does not make much sense to send blank values here, they are optional).

[Vsevolod Vlasov]

Created attachment 102251 [details]
Patch

[Vsevolod Vlasov]

Committed r91928: <http://trac.webkit.org/changeset/91928>