Loading Predictor: Preconnect requests started and stopped immediately may always remain tracked |
||
Issue descriptionDue to some racing issues, it's possible that some of the preload requests that are canceled (possibly due to navigation getting completed), are never cleaned up from the preconnect_manager. This results in: (i) Increased memory usage since the corresponding PreresolveInfo structure is never evicted from memory (ii) Prevents subsequent preconnect/preresolve for the same webpage to be dropped since preconnect_manager believes that the corresponding requests are still in-flight. I think there are 2 cases where this can happen: (i) LoadingPredictor::PrepareForPageLoad is called for |webpage_1| and |webpage_2|. Then, CancelPageLoadHint() is called for |webpage_2| before any of the preconnect requests corresponding to |webpage_2| get a chance to go on the network. In this case, preconnect_manager will mark |preresolve_info_it->second->was_canceled| for |webpage_2| as true. However, AllPreresolvesForUrlFinished() will never be called for |webpage_2|. This would keep pre-resolve info corresponding to |webpage_2| in |preresolve_info_| forever. (ii) LoadingPredictor::PrepareForPageLoad is called for webpage_2. Then, CancelPageLoadHint() is called for webpage_2 after one of the preconnect requests corresponding to webpage_2 get a chance to go on the network. Again, in this case, pre-resolve info for |webpage_2| is not cleared immediately after the in-flight request finishes.
,
Jan 17
(6 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6462ff0595cd0f178657cf2fe72ab9bd614eea49 commit 6462ff0595cd0f178657cf2fe72ab9bd614eea49 Author: Tarun Bansal <tbansal@chromium.org> Date: Thu Jan 17 00:51:41 2019 Loading predictor: Add couple of more unit tests This CL adds a couple of unit tests to better test the behavior of preconnect manager when the request to preconnect for the same webpage arrives more than once. This CL does not change anything functionally. Subsequent CLs will fix the bug, and add regression tests for the issues described in the associated crbug. Bug: 921720 Change-Id: Ia9690b05df34c4e535a9f221596993b32197e23b Reviewed-on: https://chromium-review.googlesource.com/c/1410003 Commit-Queue: Tarun Bansal <tbansal@chromium.org> Reviewed-by: Alex Ilin <alexilin@chromium.org> Cr-Commit-Position: refs/heads/master@{#623475} [modify] https://crrev.com/6462ff0595cd0f178657cf2fe72ab9bd614eea49/chrome/browser/predictors/preconnect_manager.cc [modify] https://crrev.com/6462ff0595cd0f178657cf2fe72ab9bd614eea49/chrome/browser/predictors/preconnect_manager_unittest.cc
,
Jan 17
(5 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/31634da63e35a4d5067d994523c2b065cafa6c1d commit 31634da63e35a4d5067d994523c2b065cafa6c1d Author: Tarun Bansal <tbansal@chromium.org> Date: Thu Jan 17 18:45:47 2019 Loading predictor: Clean up preconnect requests that are done. PreresolveInfo that are done earlier because their associated navigation got cancelled may remain tracked in preconnect manager forever. In general, this is not a big problem and only causes very small memory leaks. However, with navigation predictor, a request for same URL may arrive again after some time. If the corresponding PreresolveInfo is not cleared, this newly arrived request would not be honored by the PreconnectManager. This CL ensures that if a PreresolveInfo is done, then it's eventually removed from the list of tracked objects. This increases the chances that a later-arriving preconnect request with an identical URL would be dispatched to the network. Change-Id: I887e367d8d8301e2db0b729ec0f0670368a63d77 Bug: 921720 Reviewed-on: https://chromium-review.googlesource.com/c/1416558 Reviewed-by: Alex Ilin <alexilin@chromium.org> Commit-Queue: Tarun Bansal <tbansal@chromium.org> Cr-Commit-Position: refs/heads/master@{#623768} [modify] https://crrev.com/31634da63e35a4d5067d994523c2b065cafa6c1d/chrome/browser/predictors/preconnect_manager.cc [modify] https://crrev.com/31634da63e35a4d5067d994523c2b065cafa6c1d/chrome/browser/predictors/preconnect_manager_unittest.cc
,
Jan 18
(4 days ago)
|
||
►
Sign in to add a comment |
||
Comment 1 by tbansal@chromium.org
, Jan 14