189130 2018-08-29 16:49:21 -0700 Avoid code duplication in ResourceLoadStatisticsMemoryStore::processStatisticsAndDataRecords() 2018-08-30 08:52:07 -0700 1 1 1 Unclassified WebKit WebKit2 WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 cdumez cdumez bfulgham commit-queue webkit-bug-importer wilander youennf oldest_to_newest 1455018 0 cdumez 2018-08-29 16:49:21 -0700 Avoid code duplication in ResourceLoadStatisticsMemoryStore::processStatisticsAndDataRecords(). 1455020 1 348449 cdumez 2018-08-29 16:50:37 -0700 Created attachment 348449 Patch 1455041 2 348449 youennf 2018-08-29 17:10:04 -0700 Comment on attachment 348449 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=348449&action=review > Source/WebKit/UIProcess/ResourceLoadStatisticsMemoryStore.cpp:358 > + return; I guess there is a small behavior change if m_parameters.shouldNotifyPagesWhenDataRecordsWereScanned value is updated between the time removeDataRecords is called and the lambda is executed. 1455134 3 348449 commit-queue 2018-08-29 20:40:06 -0700 Comment on attachment 348449 Patch Clearing flags on attachment: 348449 Committed r235495: <https://trac.webkit.org/changeset/235495> 1455135 4 commit-queue 2018-08-29 20:40:08 -0700 All reviewed patches have been landed. Closing bug. 1455138 5 webkit-bug-importer 2018-08-29 20:41:19 -0700 <rdar://problem/43871000> 1455228 6 348449 wilander 2018-08-30 08:48:53 -0700 Comment on attachment 348449 Patch If I recall correctly, this didn't use to be a pure duplication. There was something about the order in which things were done. But now it is totally a duplication and the patch looks good to me. (The code has been moved and changed too much for me to easily find out what the difference used to be.) 1455232 7 cdumez 2018-08-30 08:52:07 -0700 (In reply to John Wilander from comment #6) > Comment on attachment 348449 [details] > Patch > > If I recall correctly, this didn't use to be a pure duplication. There was > something about the order in which things were done. But now it is totally a > duplication and the patch looks good to me. > > (The code has been moved and changed too much for me to easily find out what > the difference used to be.) Ok, that makes sense. This code has been refactored a lot. Thanks for checking. 348449 2018-08-29 16:50:37 -0700 2018-08-29 20:40:06 -0700 Patch bug-189130-20180829165036.patch text/plain 3258 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMjM1NDg3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDQzOWVkMzhhNjFiZTRkODU1 YWJiMWQ3N2U3NGYxYzZkNmRiZmY2MmQuLjQ2NDgwNDc2YzkyYjJmYWNkMjZjYTVhYTI2MmY4OWM4 MWVlNzk4NDkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTgtMDgtMjkgIENocmlzIER1 bWV6ICA8Y2R1bWV6QGFwcGxlLmNvbT4KKworICAgICAgICBBdm9pZCBjb2RlIGR1cGxpY2F0aW9u IGluIFJlc291cmNlTG9hZFN0YXRpc3RpY3NNZW1vcnlTdG9yZTo6cHJvY2Vzc1N0YXRpc3RpY3NB bmREYXRhUmVjb3JkcygpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVn LmNnaT9pZD0xODkxMzAKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKwor ICAgICAgICBNb3ZlIGlmIGNoZWNrIGluc2lkZSB0aGUgbGFtYmRhIGluc3RlYWQgb2Ygb3V0c2lk ZSB0byBhdm9pZCBjb2RlIGR1cGxpY2F0aW9uLgorCisgICAgICAgICogVUlQcm9jZXNzL1Jlc291 cmNlTG9hZFN0YXRpc3RpY3NNZW1vcnlTdG9yZS5jcHA6CisgICAgICAgIChXZWJLaXQ6OlJlc291 cmNlTG9hZFN0YXRpc3RpY3NNZW1vcnlTdG9yZTo6cHJvY2Vzc1N0YXRpc3RpY3NBbmREYXRhUmVj b3Jkcyk6CisKIDIwMTgtMDgtMjkgIENocmlzIER1bWV6ICA8Y2R1bWV6QGFwcGxlLmNvbT4KIAog ICAgICAgICBDcmFzaCB1bmRlciBXZWJLaXQ6IFdURjo6RnVuY3Rpb248dm9pZCAoKT46OkNhbGxh YmxlV3JhcHBlcjxXZWJLaXQ6OlJlc291cmNlTG9hZFN0YXRpc3RpY3NNZW1vcnlTdG9yZTo6cmVt b3ZlRGF0YVJlY29yZHMoV1RGOjpDb21wbGV0aW9uSGFuZGxlcjx2b2lkICgpPiYmKTo6JF8xPjo6 Y2FsbCgpCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9SZXNvdXJjZUxvYWRT dGF0aXN0aWNzTWVtb3J5U3RvcmUuY3BwIGIvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvUmVzb3Vy Y2VMb2FkU3RhdGlzdGljc01lbW9yeVN0b3JlLmNwcAppbmRleCBlNTg5Zjg1YmM3NTBiMjZjNzM0 NWYxZjI3ZmJmMGE3NWJjMWNiZGYyLi4xN2NlN2JkMDVjOTQ1Mzk4NTkzMGNlNGUzNjJlM2E1NDJm NTM5YjJiIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9SZXNvdXJjZUxvYWRT dGF0aXN0aWNzTWVtb3J5U3RvcmUuY3BwCisrKyBiL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL1Jl c291cmNlTG9hZFN0YXRpc3RpY3NNZW1vcnlTdG9yZS5jcHAKQEAgLTM0NSwzMSArMzQ1LDIyIEBA IHZvaWQgUmVzb3VyY2VMb2FkU3RhdGlzdGljc01lbW9yeVN0b3JlOjpwcm9jZXNzU3RhdGlzdGlj c0FuZERhdGFSZWNvcmRzKCkKICAgICAgICAgfQogICAgIH0KIAotICAgIGlmIChtX3BhcmFtZXRl cnMuc2hvdWxkTm90aWZ5UGFnZXNXaGVuRGF0YVJlY29yZHNXZXJlU2Nhbm5lZCkgewotICAgICAg ICByZW1vdmVEYXRhUmVjb3JkcyhbdGhpcywgd2Vha1RoaXMgPSBtYWtlV2Vha1B0cigqdGhpcyld IHsKLSAgICAgICAgICAgIEFTU0VSVCghUnVuTG9vcDo6aXNNYWluKCkpOwotICAgICAgICAgICAg aWYgKCF3ZWFrVGhpcykKLSAgICAgICAgICAgICAgICByZXR1cm47Ci0KLSAgICAgICAgICAgIHBy dW5lU3RhdGlzdGljc0lmTmVlZGVkKCk7Ci0gICAgICAgICAgICBpZiAobV9wZXJzaXN0ZW50U3Rv cmFnZSkKLSAgICAgICAgICAgICAgICBtX3BlcnNpc3RlbnRTdG9yYWdlLT5zY2hlZHVsZU9yV3Jp dGVNZW1vcnlTdG9yZShSZXNvdXJjZUxvYWRTdGF0aXN0aWNzUGVyc2lzdGVudFN0b3JhZ2U6OkZv cmNlSW1tZWRpYXRlV3JpdGU6Ok5vKTsKLQotICAgICAgICAgICAgUnVuTG9vcDo6bWFpbigpLmRp c3BhdGNoKFtdIHsKLSAgICAgICAgICAgICAgICBXZWJQcm9jZXNzUHJveHk6Om5vdGlmeVBhZ2VT dGF0aXN0aWNzQW5kRGF0YVJlY29yZHNQcm9jZXNzZWQoKTsKLSAgICAgICAgICAgIH0pOwotICAg ICAgICB9KTsKLSAgICB9IGVsc2UgewotICAgICAgICByZW1vdmVEYXRhUmVjb3JkcyhbdGhpcywg d2Vha1RoaXMgPSBtYWtlV2Vha1B0cigqdGhpcyldIHsKLSAgICAgICAgICAgIEFTU0VSVCghUnVu TG9vcDo6aXNNYWluKCkpOwotICAgICAgICAgICAgaWYgKCF3ZWFrVGhpcykKLSAgICAgICAgICAg ICAgICByZXR1cm47Ci0KLSAgICAgICAgICAgIHBydW5lU3RhdGlzdGljc0lmTmVlZGVkKCk7Ci0g ICAgICAgICAgICBpZiAobV9wZXJzaXN0ZW50U3RvcmFnZSkKLSAgICAgICAgICAgICAgICBtX3Bl cnNpc3RlbnRTdG9yYWdlLT5zY2hlZHVsZU9yV3JpdGVNZW1vcnlTdG9yZShSZXNvdXJjZUxvYWRT dGF0aXN0aWNzUGVyc2lzdGVudFN0b3JhZ2U6OkZvcmNlSW1tZWRpYXRlV3JpdGU6Ok5vKTsKKyAg ICByZW1vdmVEYXRhUmVjb3JkcyhbdGhpcywgd2Vha1RoaXMgPSBtYWtlV2Vha1B0cigqdGhpcyld IHsKKyAgICAgICAgQVNTRVJUKCFSdW5Mb29wOjppc01haW4oKSk7CisgICAgICAgIGlmICghd2Vh a1RoaXMpCisgICAgICAgICAgICByZXR1cm47CisKKyAgICAgICAgcHJ1bmVTdGF0aXN0aWNzSWZO ZWVkZWQoKTsKKyAgICAgICAgaWYgKG1fcGVyc2lzdGVudFN0b3JhZ2UpCisgICAgICAgICAgICBt X3BlcnNpc3RlbnRTdG9yYWdlLT5zY2hlZHVsZU9yV3JpdGVNZW1vcnlTdG9yZShSZXNvdXJjZUxv YWRTdGF0aXN0aWNzUGVyc2lzdGVudFN0b3JhZ2U6OkZvcmNlSW1tZWRpYXRlV3JpdGU6Ok5vKTsK KworICAgICAgICBpZiAoIW1fcGFyYW1ldGVycy5zaG91bGROb3RpZnlQYWdlc1doZW5EYXRhUmVj b3Jkc1dlcmVTY2FubmVkKQorICAgICAgICAgICAgcmV0dXJuOworCisgICAgICAgIFJ1bkxvb3A6 Om1haW4oKS5kaXNwYXRjaChbXSB7CisgICAgICAgICAgICBXZWJQcm9jZXNzUHJveHk6Om5vdGlm eVBhZ2VTdGF0aXN0aWNzQW5kRGF0YVJlY29yZHNQcm9jZXNzZWQoKTsKICAgICAgICAgfSk7Ci0g ICAgfQorICAgIH0pOwogfQogCiB2b2lkIFJlc291cmNlTG9hZFN0YXRpc3RpY3NNZW1vcnlTdG9y ZTo6aGFzU3RvcmFnZUFjY2Vzcyhjb25zdCBTdHJpbmcmIHN1YkZyYW1lUHJpbWFyeURvbWFpbiwg Y29uc3QgU3RyaW5nJiB0b3BGcmFtZVByaW1hcnlEb21haW4sIHVpbnQ2NF90IGZyYW1lSUQsIHVp bnQ2NF90IHBhZ2VJRCwgQ29tcGxldGlvbkhhbmRsZXI8dm9pZChib29sKT4mJiBjb21wbGV0aW9u SGFuZGxlcikK