179359 2017-11-06 19:36:39 -0800 Web Inspector: Add some fast returns in cases where we only call through to a NetworkAgent 2017-11-15 12:10:36 -0800 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 joepeck joepeck hi inspector-bugzilla-changes joepeck webkit-bug-importer oldest_to_newest 1368901 0 joepeck 2017-11-06 19:36:39 -0800 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. 1368902 1 326184 joepeck 2017-11-06 19:37:56 -0800 Created attachment 326184 [PATCH] Proposed Fix 1368959 2 326184 hi 2017-11-07 01:52:18 -0800 Comment on attachment 326184 [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. 1369074 3 joepeck 2017-11-07 11:06:57 -0800 (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. 1369116 4 joepeck 2017-11-07 12:01:43 -0800 <https://trac.webkit.org/r224540> 1372093 5 webkit-bug-importer 2017-11-15 12:10:36 -0800 <rdar://problem/35567075> 326184 2017-11-06 19:37:56 -0800 2017-11-07 11:07:02 -0800 [PATCH] Proposed Fix fast-return-1.patch text/plain 5965 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCBjMWUyOTdjNjYxYi4uMGU4MjNmNTdkNjkgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwyMCBAQAorMjAxNy0xMS0wNiAgSm9zZXBoIFBlY29yYXJvICA8cGVjb3Jhcm9AYXBwbGUu Y29tPgorCisgICAgICAgIFdlYiBJbnNwZWN0b3I6IEFkZCBzb21lIGZhc3QgcmV0dXJucyBpbiBj YXNlcyB3aGVyZSB3ZSBvbmx5IGNhbGwgdGhyb3VnaCB0byBhIE5ldHdvcmtBZ2VudAorICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTc5MzU5CisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBpbnNwZWN0b3IvSW5z cGVjdG9ySW5zdHJ1bWVudGF0aW9uLmg6CisgICAgICAgIChXZWJDb3JlOjpJbnNwZWN0b3JJbnN0 cnVtZW50YXRpb246OmFwcGx5RW11bGF0ZWRNZWRpYSk6CisgICAgICAgIChXZWJDb3JlOjpJbnNw ZWN0b3JJbnN0cnVtZW50YXRpb246OndpbGxTZW5kUmVxdWVzdCk6CisgICAgICAgIChXZWJDb3Jl OjpJbnNwZWN0b3JJbnN0cnVtZW50YXRpb246OmRpZEZpbmlzaExvYWRpbmcpOgorICAgICAgICAo V2ViQ29yZTo6SW5zcGVjdG9ySW5zdHJ1bWVudGF0aW9uOjpkaWRGYWlsTG9hZGluZyk6CisgICAg ICAgIChXZWJDb3JlOjpJbnNwZWN0b3JJbnN0cnVtZW50YXRpb246OmNvbnRpbnVlQWZ0ZXJYRnJh bWVPcHRpb25zRGVuaWVkKToKKyAgICAgICAgKFdlYkNvcmU6Okluc3BlY3Rvckluc3RydW1lbnRh dGlvbjo6Y29udGludWVXaXRoUG9saWN5RG93bmxvYWQpOgorICAgICAgICBGYXN0IHJldHVybiBp ZiBubyBmcm9udGVuZCBpbiBjYXNlcyB0aGF0IG9ubHkgY2FsbCBpbnRvIE5ldHdvcmtBZ2VudAor ICAgICAgICBiZWNhdXNlIHRoZSBOZXR3b3JrQWdlbnQgaXMgb25seSBhdmFpbGFibGUgaWYgdGhl cmUgaXMgYSBmcm9udGVuZC4KKwogMjAxNy0xMS0wNiAgQ2hyaXMgRHVtZXogIDxjZHVtZXpAYXBw bGUuY29tPgogCiAgICAgICAgIFtTZXJ2aWNlIFdvcmtlcnNdIEFkZCBwcm9wZXIgaW1wbGVtZW50 YXRpb24gZm9yICd1cGRhdGVmb3VuZCcgZXZlbnQKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3Jl L2luc3BlY3Rvci9JbnNwZWN0b3JJbnN0cnVtZW50YXRpb24uaCBiL1NvdXJjZS9XZWJDb3JlL2lu c3BlY3Rvci9JbnNwZWN0b3JJbnN0cnVtZW50YXRpb24uaAppbmRleCA1MzUwNDg1NDhhNC4uMDJj YjhjZDliZjEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2luc3BlY3Rvci9JbnNwZWN0b3JJ bnN0cnVtZW50YXRpb24uaAorKysgYi9Tb3VyY2UvV2ViQ29yZS9pbnNwZWN0b3IvSW5zcGVjdG9y SW5zdHJ1bWVudGF0aW9uLmgKQEAgLTE2OCw4ICsxNjgsOCBAQCBwdWJsaWM6CiAgICAgc3RhdGlj IEluc3BlY3Rvckluc3RydW1lbnRhdGlvbkNvb2tpZSB3aWxsUmVjYWxjdWxhdGVTdHlsZShEb2N1 bWVudCYpOwogICAgIHN0YXRpYyB2b2lkIGRpZFJlY2FsY3VsYXRlU3R5bGUoY29uc3QgSW5zcGVj dG9ySW5zdHJ1bWVudGF0aW9uQ29va2llJik7CiAgICAgc3RhdGljIHZvaWQgZGlkU2NoZWR1bGVT dHlsZVJlY2FsY3VsYXRpb24oRG9jdW1lbnQmKTsKLQogICAgIHN0YXRpYyB2b2lkIGFwcGx5RW11 bGF0ZWRNZWRpYShGcmFtZSYsIFN0cmluZyYpOworCiAgICAgc3RhdGljIHZvaWQgd2lsbFNlbmRS ZXF1ZXN0KEZyYW1lKiwgdW5zaWduZWQgbG9uZyBpZGVudGlmaWVyLCBEb2N1bWVudExvYWRlcios IFJlc291cmNlUmVxdWVzdCYsIGNvbnN0IFJlc291cmNlUmVzcG9uc2UmIHJlZGlyZWN0UmVzcG9u c2UpOwogICAgIHN0YXRpYyB2b2lkIGRpZExvYWRSZXNvdXJjZUZyb21NZW1vcnlDYWNoZShQYWdl JiwgRG9jdW1lbnRMb2FkZXIqLCBDYWNoZWRSZXNvdXJjZSopOwogICAgIHN0YXRpYyB2b2lkIGRp ZFJlY2VpdmVSZXNvdXJjZVJlc3BvbnNlKEZyYW1lJiwgdW5zaWduZWQgbG9uZyBpZGVudGlmaWVy LCBEb2N1bWVudExvYWRlciosIGNvbnN0IFJlc291cmNlUmVzcG9uc2UmLCBSZXNvdXJjZUxvYWRl ciopOwpAQCAtMzQ2LDggKzM0Niw4IEBAIHByaXZhdGU6CiAgICAgc3RhdGljIEluc3BlY3Rvcklu c3RydW1lbnRhdGlvbkNvb2tpZSB3aWxsUmVjYWxjdWxhdGVTdHlsZUltcGwoSW5zdHJ1bWVudGlu Z0FnZW50cyYsIERvY3VtZW50Jik7CiAgICAgc3RhdGljIHZvaWQgZGlkUmVjYWxjdWxhdGVTdHls ZUltcGwoY29uc3QgSW5zcGVjdG9ySW5zdHJ1bWVudGF0aW9uQ29va2llJik7CiAgICAgc3RhdGlj IHZvaWQgZGlkU2NoZWR1bGVTdHlsZVJlY2FsY3VsYXRpb25JbXBsKEluc3RydW1lbnRpbmdBZ2Vu dHMmLCBEb2N1bWVudCYpOwotCiAgICAgc3RhdGljIHZvaWQgYXBwbHlFbXVsYXRlZE1lZGlhSW1w bChJbnN0cnVtZW50aW5nQWdlbnRzJiwgU3RyaW5nJik7CisKICAgICBzdGF0aWMgdm9pZCB3aWxs U2VuZFJlcXVlc3RJbXBsKEluc3RydW1lbnRpbmdBZ2VudHMmLCB1bnNpZ25lZCBsb25nIGlkZW50 aWZpZXIsIERvY3VtZW50TG9hZGVyKiwgUmVzb3VyY2VSZXF1ZXN0JiwgY29uc3QgUmVzb3VyY2VS ZXNwb25zZSYgcmVkaXJlY3RSZXNwb25zZSk7CiAgICAgc3RhdGljIHZvaWQgd2lsbFNlbmRSZXF1 ZXN0T2ZUeXBlSW1wbChJbnN0cnVtZW50aW5nQWdlbnRzJiwgdW5zaWduZWQgbG9uZyBpZGVudGlm aWVyLCBEb2N1bWVudExvYWRlciosIFJlc291cmNlUmVxdWVzdCYsIExvYWRUeXBlKTsKICAgICBz dGF0aWMgdm9pZCBtYXJrUmVzb3VyY2VBc0NhY2hlZEltcGwoSW5zdHJ1bWVudGluZ0FnZW50cyYs IHVuc2lnbmVkIGxvbmcgaWRlbnRpZmllcik7CkBAIC05MDIsMTggKzkwMiwyMSBAQCBpbmxpbmUg dm9pZCBJbnNwZWN0b3JJbnN0cnVtZW50YXRpb246OmFwcGx5RW11bGF0ZWRNZWRpYShGcmFtZSYg ZnJhbWUsIFN0cmluZyYgbQogCiBpbmxpbmUgdm9pZCBJbnNwZWN0b3JJbnN0cnVtZW50YXRpb246 OndpbGxTZW5kUmVxdWVzdChGcmFtZSogZnJhbWUsIHVuc2lnbmVkIGxvbmcgaWRlbnRpZmllciwg RG9jdW1lbnRMb2FkZXIqIGxvYWRlciwgUmVzb3VyY2VSZXF1ZXN0JiByZXF1ZXN0LCBjb25zdCBS ZXNvdXJjZVJlc3BvbnNlJiByZWRpcmVjdFJlc3BvbnNlKQogeworICAgIEZBU1RfUkVUVVJOX0lG X05PX0ZST05URU5EUyh2b2lkKCkpOwogICAgIGlmIChJbnN0cnVtZW50aW5nQWdlbnRzKiBpbnN0 cnVtZW50aW5nQWdlbnRzID0gaW5zdHJ1bWVudGluZ0FnZW50c0ZvckZyYW1lKGZyYW1lKSkKICAg ICAgICAgd2lsbFNlbmRSZXF1ZXN0SW1wbCgqaW5zdHJ1bWVudGluZ0FnZW50cywgaWRlbnRpZmll ciwgbG9hZGVyLCByZXF1ZXN0LCByZWRpcmVjdFJlc3BvbnNlKTsKIH0KIAogaW5saW5lIHZvaWQg SW5zcGVjdG9ySW5zdHJ1bWVudGF0aW9uOjp3aWxsU2VuZFJlcXVlc3RPZlR5cGUoRnJhbWUqIGZy YW1lLCB1bnNpZ25lZCBsb25nIGlkZW50aWZpZXIsIERvY3VtZW50TG9hZGVyKiBsb2FkZXIsIFJl c291cmNlUmVxdWVzdCYgcmVxdWVzdCwgTG9hZFR5cGUgbG9hZFR5cGUpCiB7CisgICAgRkFTVF9S RVRVUk5fSUZfTk9fRlJPTlRFTkRTKHZvaWQoKSk7CiAgICAgaWYgKEluc3RydW1lbnRpbmdBZ2Vu dHMqIGluc3RydW1lbnRpbmdBZ2VudHMgPSBpbnN0cnVtZW50aW5nQWdlbnRzRm9yRnJhbWUoZnJh bWUpKQogICAgICAgICB3aWxsU2VuZFJlcXVlc3RPZlR5cGVJbXBsKCppbnN0cnVtZW50aW5nQWdl bnRzLCBpZGVudGlmaWVyLCBsb2FkZXIsIHJlcXVlc3QsIGxvYWRUeXBlKTsKIH0KIAogaW5saW5l IHZvaWQgSW5zcGVjdG9ySW5zdHJ1bWVudGF0aW9uOjpkaWRMb2FkUmVzb3VyY2VGcm9tTWVtb3J5 Q2FjaGUoUGFnZSYgcGFnZSwgRG9jdW1lbnRMb2FkZXIqIGxvYWRlciwgQ2FjaGVkUmVzb3VyY2Uq IHJlc291cmNlKQogeworICAgIEZBU1RfUkVUVVJOX0lGX05PX0ZST05URU5EUyh2b2lkKCkpOwog ICAgIGRpZExvYWRSZXNvdXJjZUZyb21NZW1vcnlDYWNoZUltcGwoaW5zdHJ1bWVudGluZ0FnZW50 c0ZvclBhZ2UocGFnZSksIGxvYWRlciwgcmVzb3VyY2UpOwogfQogCkBAIC05NzAsMjQgKzk3Mywy OCBAQCBpbmxpbmUgdm9pZCBJbnNwZWN0b3JJbnN0cnVtZW50YXRpb246OmNvbnRpbnVlV2l0aFBv bGljeUlnbm9yZShGcmFtZSYgZnJhbWUsIHVucwogCiBpbmxpbmUgdm9pZCBJbnNwZWN0b3JJbnN0 cnVtZW50YXRpb246OmRpZEZpbmlzaFhIUkxvYWRpbmcoU2NyaXB0RXhlY3V0aW9uQ29udGV4dCog Y29udGV4dCwgdW5zaWduZWQgbG9uZyBpZGVudGlmaWVyLCBzdGQ6Om9wdGlvbmFsPFN0cmluZz4g ZGVjb2RlZFRleHQpCiB7CisgICAgRkFTVF9SRVRVUk5fSUZfTk9fRlJPTlRFTkRTKHZvaWQoKSk7 CiAgICAgaWYgKEluc3RydW1lbnRpbmdBZ2VudHMqIGluc3RydW1lbnRpbmdBZ2VudHMgPSBpbnN0 cnVtZW50aW5nQWdlbnRzRm9yQ29udGV4dChjb250ZXh0KSkKICAgICAgICAgZGlkRmluaXNoWEhS TG9hZGluZ0ltcGwoKmluc3RydW1lbnRpbmdBZ2VudHMsIGlkZW50aWZpZXIsIGRlY29kZWRUZXh0 KTsKIH0KIAogaW5saW5lIHZvaWQgSW5zcGVjdG9ySW5zdHJ1bWVudGF0aW9uOjp3aWxsTG9hZFhI UlN5bmNocm9ub3VzbHkoU2NyaXB0RXhlY3V0aW9uQ29udGV4dCogY29udGV4dCkKIHsKKyAgICBG QVNUX1JFVFVSTl9JRl9OT19GUk9OVEVORFModm9pZCgpKTsKICAgICBpZiAoSW5zdHJ1bWVudGlu Z0FnZW50cyogaW5zdHJ1bWVudGluZ0FnZW50cyA9IGluc3RydW1lbnRpbmdBZ2VudHNGb3JDb250 ZXh0KGNvbnRleHQpKQogICAgICAgICB3aWxsTG9hZFhIUlN5bmNocm9ub3VzbHlJbXBsKCppbnN0 cnVtZW50aW5nQWdlbnRzKTsKIH0KIAogaW5saW5lIHZvaWQgSW5zcGVjdG9ySW5zdHJ1bWVudGF0 aW9uOjpkaWRMb2FkWEhSU3luY2hyb25vdXNseShTY3JpcHRFeGVjdXRpb25Db250ZXh0KiBjb250 ZXh0KQogeworICAgIEZBU1RfUkVUVVJOX0lGX05PX0ZST05URU5EUyh2b2lkKCkpOwogICAgIGlm IChJbnN0cnVtZW50aW5nQWdlbnRzKiBpbnN0cnVtZW50aW5nQWdlbnRzID0gaW5zdHJ1bWVudGlu Z0FnZW50c0ZvckNvbnRleHQoY29udGV4dCkpCiAgICAgICAgIGRpZExvYWRYSFJTeW5jaHJvbm91 c2x5SW1wbCgqaW5zdHJ1bWVudGluZ0FnZW50cyk7CiB9CiAKIGlubGluZSB2b2lkIEluc3BlY3Rv ckluc3RydW1lbnRhdGlvbjo6c2NyaXB0SW1wb3J0ZWQoU2NyaXB0RXhlY3V0aW9uQ29udGV4dCYg Y29udGV4dCwgdW5zaWduZWQgbG9uZyBpZGVudGlmaWVyLCBjb25zdCBTdHJpbmcmIHNvdXJjZVN0 cmluZykKIHsKKyAgICBGQVNUX1JFVFVSTl9JRl9OT19GUk9OVEVORFModm9pZCgpKTsKICAgICBp ZiAoSW5zdHJ1bWVudGluZ0FnZW50cyogaW5zdHJ1bWVudGluZ0FnZW50cyA9IGluc3RydW1lbnRp bmdBZ2VudHNGb3JDb250ZXh0KGNvbnRleHQpKQogICAgICAgICBzY3JpcHRJbXBvcnRlZEltcGwo Kmluc3RydW1lbnRpbmdBZ2VudHMsIGlkZW50aWZpZXIsIHNvdXJjZVN0cmluZyk7CiB9CkBAIC0x MDAwLDYgKzEwMDcsNyBAQCBpbmxpbmUgdm9pZCBJbnNwZWN0b3JJbnN0cnVtZW50YXRpb246OnNj cmlwdEV4ZWN1dGlvbkJsb2NrZWRCeUNTUChTY3JpcHRFeGVjdXRpbwogCiBpbmxpbmUgdm9pZCBJ bnNwZWN0b3JJbnN0cnVtZW50YXRpb246OmRpZFJlY2VpdmVTY3JpcHRSZXNwb25zZShTY3JpcHRF eGVjdXRpb25Db250ZXh0KiBjb250ZXh0LCB1bnNpZ25lZCBsb25nIGlkZW50aWZpZXIpCiB7Cisg ICAgRkFTVF9SRVRVUk5fSUZfTk9fRlJPTlRFTkRTKHZvaWQoKSk7CiAgICAgaWYgKEluc3RydW1l bnRpbmdBZ2VudHMqIGluc3RydW1lbnRpbmdBZ2VudHMgPSBpbnN0cnVtZW50aW5nQWdlbnRzRm9y Q29udGV4dChjb250ZXh0KSkKICAgICAgICAgZGlkUmVjZWl2ZVNjcmlwdFJlc3BvbnNlSW1wbCgq aW5zdHJ1bWVudGluZ0FnZW50cywgaWRlbnRpZmllcik7CiB9Cg==