200225 2019-07-29 08:24:57 -0700 Possible use-after-move under NetworkConnectionToWebProcess::resourceLoadStatisticsUpdated() 2019-07-29 09:43:18 -0700 1 1 1 Unclassified WebKit WebKit2 WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 196407 1 cdumez cdumez bfulgham commit-queue ggaren rniwa webkit-bug-importer wilander youennf oldest_to_newest 1556563 0 cdumez 2019-07-29 08:24:57 -0700 Possible use-after-move under NetworkConnectionToWebProcess::resourceLoadStatisticsUpdated(). Was WTFMove()-ing the method parameter inside of a loop. 1556564 1 375076 cdumez 2019-07-29 08:27:52 -0700 Created attachment 375076 Patch 1556570 2 375076 youennf 2019-07-29 08:52:48 -0700 Comment on attachment 375076 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=375076&action=review > Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:699 > + auto* networkSession = networkProcess().networkSessionByConnection(connection()); This seems a bit strange. I hope we can remove networkSessionByConnection in the future since a process can have multiple pages with different sessions. It seems also strange that the statistics are not related to some sessionIDs. Would it not be safer to keep passing a copy of the statistics to all sessions? Or pass sessionID with the statistics? 1556571 3 375076 bfulgham 2019-07-29 09:07:38 -0700 Comment on attachment 375076 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=375076&action=review >> Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:699 >> + auto* networkSession = networkProcess().networkSessionByConnection(connection()); > > This seems a bit strange. > I hope we can remove networkSessionByConnection in the future since a process can have multiple pages with different sessions. > It seems also strange that the statistics are not related to some sessionIDs. > Would it not be safer to keep passing a copy of the statistics to all sessions? Or pass sessionID with the statistics? We should probably not pass the statistics to all sessions, as that could be viewed as a privacy violation if you were servicing multiple network clients (e.g. an Ephemeral session versus a default session versus a non-default session). We know that this can happen in use cases like Safari that may have multiple session to support different tasks unrelated to the main browsing session. We should be sending the sessionID with each of these messages. I started a patch doing this earlier this year but got sidetracked. I should revive it. I would support doing this in the short term to avoid the issue, and remove this use of 'networkSessionByConnection' with my proposed update later this cycle. 1556592 4 375076 commit-queue 2019-07-29 09:42:29 -0700 Comment on attachment 375076 Patch Clearing flags on attachment: 375076 Committed r247905: <https://trac.webkit.org/changeset/247905> 1556593 5 commit-queue 2019-07-29 09:42:31 -0700 All reviewed patches have been landed. Closing bug. 1556594 6 webkit-bug-importer 2019-07-29 09:43:18 -0700 <rdar://problem/53665272> 375076 2019-07-29 08:27:52 -0700 2019-07-29 09:42:29 -0700 Patch bug-200225-20190729082752.patch text/plain 2560 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMjQ3OTA0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IGUyZmY3Yjg5YzJkYmIxZjUw MWQ0MDc3OTViMjJhODNiNzg0ODNkODIuLmRiOWM5MTA5ZTU5MmUyOWU5MTM3YTA4MjVkNTY3MzU3 MjQ1OGFlMDMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTktMDctMjkgIENocmlzIER1 bWV6ICA8Y2R1bWV6QGFwcGxlLmNvbT4KKworICAgICAgICBQb3NzaWJsZSB1c2UtYWZ0ZXItbW92 ZSB1bmRlciBOZXR3b3JrQ29ubmVjdGlvblRvV2ViUHJvY2Vzczo6cmVzb3VyY2VMb2FkU3RhdGlz dGljc1VwZGF0ZWQoKQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9MjAwMjI1CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAg ICAgICAgVGhlIGNvZGUgd2FzIFdURk1vdmUoKS1pbmcgdGhlIG1ldGhvZCBwYXJhbWV0ZXIgaW5z aWRlIG9mIGEgbG9vcCwgd2hpY2ggbWVhbnMgdGhhdCBpdCBjb3VsZAorICAgICAgICBtb3ZlIGl0 IHNldmVyYWwgdGltZXMuIEluc3RlYWQgb2YgY29weWluZyB0aGUgcGFyYW1ldGVycywgSSBvcHRl ZCBpbnRvIHNlbmRpbmcgdGhlIHN0YXRpc3RpY3MKKyAgICAgICAgb25seSB0byB0aGUgbmV0d29y ayBzZXNzaW9uIHRoYXQgbWF0Y2hlcyB0aGlzIFdlYlByb2Nlc3MgY29ubmVjdGlvbi4KKworICAg ICAgICAqIE5ldHdvcmtQcm9jZXNzL05ldHdvcmtDb25uZWN0aW9uVG9XZWJQcm9jZXNzLmNwcDoK KyAgICAgICAgKFdlYktpdDo6TmV0d29ya0Nvbm5lY3Rpb25Ub1dlYlByb2Nlc3M6OnJlc291cmNl TG9hZFN0YXRpc3RpY3NVcGRhdGVkKToKKwogMjAxOS0wNy0yNyAgQ2hyaXMgRHVtZXogIDxjZHVt ZXpAYXBwbGUuY29tPgogCiAgICAgICAgIEFsbG93IG1vcmUgc3lzY2FsbHMgaW4gdGhlIFdlYkNv bnRlbnQgcHJvY2Vzcycgc2FuZGJveCBwcm9maWxlCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0 L05ldHdvcmtQcm9jZXNzL05ldHdvcmtDb25uZWN0aW9uVG9XZWJQcm9jZXNzLmNwcCBiL1NvdXJj ZS9XZWJLaXQvTmV0d29ya1Byb2Nlc3MvTmV0d29ya0Nvbm5lY3Rpb25Ub1dlYlByb2Nlc3MuY3Bw CmluZGV4IDkzM2MzYWViMmUyZmM5MzRkNmJjYTk1NjYxNTkzMWMyYTBiNTE4ZjQuLjliMWI5Yjg2 MDQzOGQ5YjZhM2YxMjY4OTE0NjJiMzM2YmM1OWZjNGQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJL aXQvTmV0d29ya1Byb2Nlc3MvTmV0d29ya0Nvbm5lY3Rpb25Ub1dlYlByb2Nlc3MuY3BwCisrKyBi L1NvdXJjZS9XZWJLaXQvTmV0d29ya1Byb2Nlc3MvTmV0d29ya0Nvbm5lY3Rpb25Ub1dlYlByb2Nl c3MuY3BwCkBAIC02OTYsMTMgKzY5NiwxMiBAQCB2b2lkIE5ldHdvcmtDb25uZWN0aW9uVG9XZWJQ cm9jZXNzOjpsb2dTdWJyZXNvdXJjZVJlZGlyZWN0KFBBTDo6U2Vzc2lvbklEIHNlc3NpbwogCiB2 b2lkIE5ldHdvcmtDb25uZWN0aW9uVG9XZWJQcm9jZXNzOjpyZXNvdXJjZUxvYWRTdGF0aXN0aWNz VXBkYXRlZChWZWN0b3I8V2ViQ29yZTo6UmVzb3VyY2VMb2FkU3RhdGlzdGljcz4mJiBzdGF0aXN0 aWNzKQogewotICAgIGZvciAoYXV0byYgbmV0d29ya1Nlc3Npb24gOiBuZXR3b3JrUHJvY2Vzcygp Lm5ldHdvcmtTZXNzaW9ucygpLnZhbHVlcygpKSB7Ci0gICAgICAgIGlmIChuZXR3b3JrU2Vzc2lv bi0+c2Vzc2lvbklEKCkuaXNFcGhlbWVyYWwoKSkKLSAgICAgICAgICAgIGNvbnRpbnVlOworICAg IGF1dG8qIG5ldHdvcmtTZXNzaW9uID0gbmV0d29ya1Byb2Nlc3MoKS5uZXR3b3JrU2Vzc2lvbkJ5 Q29ubmVjdGlvbihjb25uZWN0aW9uKCkpOworICAgIGlmICghbmV0d29ya1Nlc3Npb24pCisgICAg ICAgIHJldHVybjsKIAotICAgICAgICBpZiAoYXV0byogcmVzb3VyY2VMb2FkU3RhdGlzdGljcyA9 IG5ldHdvcmtTZXNzaW9uLT5yZXNvdXJjZUxvYWRTdGF0aXN0aWNzKCkpCi0gICAgICAgICAgICBy ZXNvdXJjZUxvYWRTdGF0aXN0aWNzLT5yZXNvdXJjZUxvYWRTdGF0aXN0aWNzVXBkYXRlZChXVEZN b3ZlKHN0YXRpc3RpY3MpKTsKLSAgICB9CisgICAgaWYgKGF1dG8qIHJlc291cmNlTG9hZFN0YXRp c3RpY3MgPSBuZXR3b3JrU2Vzc2lvbi0+cmVzb3VyY2VMb2FkU3RhdGlzdGljcygpKQorICAgICAg ICByZXNvdXJjZUxvYWRTdGF0aXN0aWNzLT5yZXNvdXJjZUxvYWRTdGF0aXN0aWNzVXBkYXRlZChX VEZNb3ZlKHN0YXRpc3RpY3MpKTsKIH0KIAogdm9pZCBOZXR3b3JrQ29ubmVjdGlvblRvV2ViUHJv Y2Vzczo6aGFzU3RvcmFnZUFjY2VzcyhQQUw6OlNlc3Npb25JRCBzZXNzaW9uSUQsIGNvbnN0IFJl Z2lzdHJhYmxlRG9tYWluJiBzdWJGcmFtZURvbWFpbiwgY29uc3QgUmVnaXN0cmFibGVEb21haW4m IHRvcEZyYW1lRG9tYWluLCB1aW50NjRfdCBmcmFtZUlELCBQYWdlSWRlbnRpZmllciBwYWdlSUQs IENvbXBsZXRpb25IYW5kbGVyPHZvaWQoYm9vbCk+JiYgY29tcGxldGlvbkhhbmRsZXIpCg==