Bug 179359 – Web Inspector: Add some fast returns in cases where we only call through to a NetworkAgent

• Component: Web Inspector
• Status: RESOLVED FIXED

[Joseph Pecoraro]

Add some fast returns in cases where we only call through to a NetworkAgent

The NetworkAgent is only enabled when a frontend connects, so we don't need to do work for these unless a frontend is connected.

[Joseph Pecoraro]

Created attachment 326184 [details]
[PATCH] Proposed Fix

[Devin Rousso]

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

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

r=me.

Should you also add this to these instrumentation points as well?
 - didReceiveThreadableLoaderResponse
 - didReceiveData

It seems like the logic we had could've theoretically supported saving network information when there was no frontend, but it would've required some changes to InspectorNetworkAgent::enable.  Is the main reason that we don't want to do this performance, or am I missing something?

> Source/WebCore/ChangeLog:14
> +        (WebCore::InspectorInstrumentation::continueAfterXFrameOptionsDenied):
> +        (WebCore::InspectorInstrumentation::continueWithPolicyDownload):

These are not actually modified.  Should they be?  They call into InspectorInstrumentation::didReceiveResourceResponseImpl, which does call to InspectorConsoleAgent.  I am unfamiliar with how that agent functions, so I can't say as to whether it relies on data from before a frontend exists or not.

[Joseph Pecoraro]

(In reply to Devin Rousso from comment #2)
> Comment on attachment 326184 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=326184&action=review
> 
> r=me.
> 
> Should you also add this to these instrumentation points as well?
>  - didReceiveThreadableLoaderResponse
>  - didReceiveData

Yes! Thanks for catching these, I had multiple changes in my tree and forgot to git-add these.

> It seems like the logic we had could've theoretically supported saving
> network information when there was no frontend, but it would've required
> some changes to InspectorNetworkAgent::enable.  Is the main reason that we
> don't want to do this performance, or am I missing something?

Yes, performance, since this code is inlined it should make instrumentation hooks more lightweight.

> > Source/WebCore/ChangeLog:14
> > +        (WebCore::InspectorInstrumentation::continueAfterXFrameOptionsDenied):
> > +        (WebCore::InspectorInstrumentation::continueWithPolicyDownload):
> 
> These are not actually modified.  Should they be?  They call into
> InspectorInstrumentation::didReceiveResourceResponseImpl, which does call to
> InspectorConsoleAgent.  I am unfamiliar with how that agent functions, so I
> can't say as to whether it relies on data from before a frontend exists or
> not.

Again, multiple changes in my tree probably.

[Joseph Pecoraro]

<https://trac.webkit.org/r224540>

[Radar WebKit Bug Importer]

<rdar://problem/35567075>