Summary: | Web Inspector: Add some fast returns in cases where we only call through to a NetworkAgent | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||
Component: | Web Inspector | Assignee: | Joseph Pecoraro <joepeck> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | hi, inspector-bugzilla-changes, joepeck, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Joseph Pecoraro
2017-11-06 19:36:39 PST
Created attachment 326184 [details]
[PATCH] Proposed Fix
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. (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. |