TaskManager should be able to track network traffic with Network Service enabled |
||||
Issue descriptionCurrently TaskManager relies on |net::NetworkDelegate| to track raw network bytes sent/received, however |net::NetworkDelegate| is not supported with Network Service enabled. The proposed solution is to get the data through the following path: Network Service process -> (mojom::URLLoaderClient) -> Renderer process -> (content::WebContentsObserver) -> Browser process -> (chrome/browser/task_manager) Potential Drawbacks: 1. Additional process hop (e.g. Extra delay and logic). 2. Less frequent refresh rate. As discussed in network-service-dev@ we will accept the drawbacks, and we can always increase the frequency by adding hooks to |mojom::URLLoaderClient|. https://groups.google.com/a/chromium.org/d/msg/network-service-dev/-11bMY5sJRc/FNang7wnBwAJ
,
Mar 15 2018
Thanks for the suggestion! Are you referring to the codepath: |content::ResourceHandler::OnDataDownloaded()| -> |mojom ::URLLoaderClient::OnDataDownloaded(...)|? Not sure if it works for requests other than 'download_to_file' though (I don't see how they get information from Network Service), but will look into it. Thanks!
,
Mar 15 2018
I don't know - when you load a page on Android, there's a little blue bar under the omnibox that progresses as the page loads. I'm not sure what it's based on, how often we send a signal, etc. I assume it's a pipe from the renderer to the browser based on resource load progress, not from the network service.
,
Mar 15 2018
Oh I see, yes I've seen that. Will check how it works then, thanks!
,
Mar 15 2018
I suggest using the notification at the end of the request, and only if we see that we need more frequent updates we can add more complexity.
,
Mar 16 2018
Thanks for the suggestion! Use the notification at the end of the request for now sounds good. My plan is to add |int64_t encoded_data_sent_length| to |mojom::URLLoaderClient::OnComplete(URLLoaderCompletionStatus status)| which already has data received, and forward them through |content::WebContentsObserver|, probably together with navigation commit and subresource load complete notifications.
,
May 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/072c4714c83c81d6a1fe92c5805d383122063499 commit 072c4714c83c81d6a1fe92c5805d383122063499 Author: Chong Zhang <chongz@chromium.org> Date: Tue May 01 02:01:01 2018 Introduce NetworkService::GetTotalNetworkUsages() for TaskManager Background: * Currently |TaskManager| tracks network usage through |net::NetworkDelegate|, which is not supported with Network Service. * https://crrev.com/c/978607 Tried to get the info from |content::WebContentsObserver| but failed due to too many corner cases. This CL: 1. Introduced |NetworkService::GetTotalNetworkUsages() => (array<NetworkUsage> total_network_usages);| 2. Updated |TaskManagerImpl| to query the above API 1/sec when active. (e.g. While showing the UI or queried by extensions.) 3. Still use the old codepath when Network Service was disabled. Note that |NetworkService| receives network usage reports from |URLLoader::NotifyCompleted()|, and we could improve the frequency when necessary. The API may be useful for |data_reduction_proxy| as well, see: https://docs.google.com/a/google.com/document/d/1qJYdvt0USWAHbJ9hvCkfauZ9ItMoyYCY2w0Fodvse0M/edit?disco=AAAAB203Cv8 Bug: 822415 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo Change-Id: I5c4035c8944f75e1484afdeaf121c9059a7b8c71 Reviewed-on: https://chromium-review.googlesource.com/1008969 Commit-Queue: Chong Zhang <chongz@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org> Reviewed-by: John Abd-El-Malek <jam@chromium.org> Reviewed-by: Nick Carter <nick@chromium.org> Cr-Commit-Position: refs/heads/master@{#554967} [modify] https://crrev.com/072c4714c83c81d6a1fe92c5805d383122063499/chrome/browser/task_manager/sampling/task_manager_impl.cc [modify] https://crrev.com/072c4714c83c81d6a1fe92c5805d383122063499/chrome/browser/task_manager/sampling/task_manager_impl.h [modify] https://crrev.com/072c4714c83c81d6a1fe92c5805d383122063499/chrome/browser/task_manager/task_manager_browsertest.cc [modify] https://crrev.com/072c4714c83c81d6a1fe92c5805d383122063499/content/browser/loader/navigation_url_loader_network_service_unittest.cc [modify] https://crrev.com/072c4714c83c81d6a1fe92c5805d383122063499/content/browser/network_service_restart_browsertest.cc [modify] https://crrev.com/072c4714c83c81d6a1fe92c5805d383122063499/services/network/BUILD.gn [modify] https://crrev.com/072c4714c83c81d6a1fe92c5805d383122063499/services/network/keepalive_statistics_recorder.cc [modify] https://crrev.com/072c4714c83c81d6a1fe92c5805d383122063499/services/network/keepalive_statistics_recorder.h [modify] https://crrev.com/072c4714c83c81d6a1fe92c5805d383122063499/services/network/network_service.cc [modify] https://crrev.com/072c4714c83c81d6a1fe92c5805d383122063499/services/network/network_service.h [add] https://crrev.com/072c4714c83c81d6a1fe92c5805d383122063499/services/network/network_usage_accumulator.cc [add] https://crrev.com/072c4714c83c81d6a1fe92c5805d383122063499/services/network/network_usage_accumulator.h [add] https://crrev.com/072c4714c83c81d6a1fe92c5805d383122063499/services/network/network_usage_accumulator_unittest.cc [modify] https://crrev.com/072c4714c83c81d6a1fe92c5805d383122063499/services/network/public/mojom/network_service.mojom [modify] https://crrev.com/072c4714c83c81d6a1fe92c5805d383122063499/services/network/url_loader.cc [modify] https://crrev.com/072c4714c83c81d6a1fe92c5805d383122063499/services/network/url_loader.h [modify] https://crrev.com/072c4714c83c81d6a1fe92c5805d383122063499/services/network/url_loader_factory.cc [modify] https://crrev.com/072c4714c83c81d6a1fe92c5805d383122063499/services/network/url_loader_unittest.cc [modify] https://crrev.com/072c4714c83c81d6a1fe92c5805d383122063499/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
,
May 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/da76f5004aae54562c7f6996fdd1e6274a5a7aab commit da76f5004aae54562c7f6996fdd1e6274a5a7aab Author: Findit <findit-for-me@appspot.gserviceaccount.com> Date: Tue May 01 03:49:54 2018 Revert "Introduce NetworkService::GetTotalNetworkUsages() for TaskManager" This reverts commit 072c4714c83c81d6a1fe92c5805d383122063499. Reason for revert: Findit (https://goo.gl/kROfz5) identified CL at revision 554967 as the culprit for failures in the build cycles as shown on: https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzA3MmM0NzE0YzgzYzgxZDZhMWZlOTJjNTgwNWQzODMxMjIwNjM0OTkM Sample Failed Build: https://ci.chromium.org/buildbot/chromium.memory/Linux%20TSan%20Tests/20975 Sample Failed Step: content_browsertests Original change's description: > Introduce NetworkService::GetTotalNetworkUsages() for TaskManager > > Background: > * Currently |TaskManager| tracks network usage through > |net::NetworkDelegate|, which is not supported with Network Service. > * https://crrev.com/c/978607 Tried to get the info from > |content::WebContentsObserver| but failed due to too many corner > cases. > > This CL: > 1. Introduced |NetworkService::GetTotalNetworkUsages() > => (array<NetworkUsage> total_network_usages);| > 2. Updated |TaskManagerImpl| to query the above API 1/sec when active. > (e.g. While showing the UI or queried by extensions.) > 3. Still use the old codepath when Network Service was disabled. > > Note that |NetworkService| receives network usage reports from > |URLLoader::NotifyCompleted()|, and we could improve the frequency when > necessary. > > The API may be useful for |data_reduction_proxy| as well, see: > https://docs.google.com/a/google.com/document/d/1qJYdvt0USWAHbJ9hvCkfauZ9ItMoyYCY2w0Fodvse0M/edit?disco=AAAAB203Cv8 > > Bug: 822415 > Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo > Change-Id: I5c4035c8944f75e1484afdeaf121c9059a7b8c71 > Reviewed-on: https://chromium-review.googlesource.com/1008969 > Commit-Queue: Chong Zhang <chongz@chromium.org> > Reviewed-by: Tom Sepez <tsepez@chromium.org> > Reviewed-by: John Abd-El-Malek <jam@chromium.org> > Reviewed-by: Nick Carter <nick@chromium.org> > Cr-Commit-Position: refs/heads/master@{#554967} Change-Id: Ib26e008dc07d66b5ad0ae9cced9fdcfa78d0ff6c No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 822415 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo Reviewed-on: https://chromium-review.googlesource.com/1036490 Cr-Commit-Position: refs/heads/master@{#554988} [modify] https://crrev.com/da76f5004aae54562c7f6996fdd1e6274a5a7aab/chrome/browser/task_manager/sampling/task_manager_impl.cc [modify] https://crrev.com/da76f5004aae54562c7f6996fdd1e6274a5a7aab/chrome/browser/task_manager/sampling/task_manager_impl.h [modify] https://crrev.com/da76f5004aae54562c7f6996fdd1e6274a5a7aab/chrome/browser/task_manager/task_manager_browsertest.cc [modify] https://crrev.com/da76f5004aae54562c7f6996fdd1e6274a5a7aab/content/browser/loader/navigation_url_loader_network_service_unittest.cc [modify] https://crrev.com/da76f5004aae54562c7f6996fdd1e6274a5a7aab/content/browser/network_service_restart_browsertest.cc [modify] https://crrev.com/da76f5004aae54562c7f6996fdd1e6274a5a7aab/services/network/BUILD.gn [modify] https://crrev.com/da76f5004aae54562c7f6996fdd1e6274a5a7aab/services/network/keepalive_statistics_recorder.cc [modify] https://crrev.com/da76f5004aae54562c7f6996fdd1e6274a5a7aab/services/network/keepalive_statistics_recorder.h [modify] https://crrev.com/da76f5004aae54562c7f6996fdd1e6274a5a7aab/services/network/network_service.cc [modify] https://crrev.com/da76f5004aae54562c7f6996fdd1e6274a5a7aab/services/network/network_service.h [delete] https://crrev.com/f4168d99006da476aa0aef04472bb463aa19b925/services/network/network_usage_accumulator.cc [delete] https://crrev.com/f4168d99006da476aa0aef04472bb463aa19b925/services/network/network_usage_accumulator.h [delete] https://crrev.com/f4168d99006da476aa0aef04472bb463aa19b925/services/network/network_usage_accumulator_unittest.cc [modify] https://crrev.com/da76f5004aae54562c7f6996fdd1e6274a5a7aab/services/network/public/mojom/network_service.mojom [modify] https://crrev.com/da76f5004aae54562c7f6996fdd1e6274a5a7aab/services/network/url_loader.cc [modify] https://crrev.com/da76f5004aae54562c7f6996fdd1e6274a5a7aab/services/network/url_loader.h [modify] https://crrev.com/da76f5004aae54562c7f6996fdd1e6274a5a7aab/services/network/url_loader_factory.cc [modify] https://crrev.com/da76f5004aae54562c7f6996fdd1e6274a5a7aab/services/network/url_loader_unittest.cc [modify] https://crrev.com/da76f5004aae54562c7f6996fdd1e6274a5a7aab/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
,
May 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5304776c9886a583278ebea8bb59418368773365 commit 5304776c9886a583278ebea8bb59418368773365 Author: Chong Zhang <chongz@chromium.org> Date: Wed May 02 21:24:08 2018 Reland "Introduce NetworkService::GetTotalNetworkUsages() for TaskManager" The original patch (Patchset 3) was reverted due to flaky tests under Linux TSan: https://crrev.com/c/1036490 This patch fixed the issue by using a wait condition runloop. --- Original Description --- Background: * Currently |TaskManager| tracks network usage through |net::NetworkDelegate|, which is not supported with Network Service. * https://crrev.com/c/978607 Tried to get the info from |content::WebContentsObserver| but failed due to too many corner cases. This CL: 1. Introduced |NetworkService::GetTotalNetworkUsages() => (array<NetworkUsage> total_network_usages);| 2. Updated |TaskManagerImpl| to query the above API 1/sec when active. (e.g. While showing the UI or queried by extensions.) 3. Still use the old codepath when Network Service was disabled. Note that |NetworkService| receives network usage reports from |URLLoader::NotifyCompleted()|, and we could improve the frequency when necessary. The API may be useful for |data_reduction_proxy| as well, see: https://docs.google.com/a/google.com/document/d/1qJYdvt0USWAHbJ9hvCkfauZ9ItMoyYCY2w0Fodvse0M/edit?disco=AAAAB203Cv8 Bug: 822415 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo Change-Id: I96f4b3456b4fa8f599b76fbe614d1c724a9c9007 Reviewed-on: https://chromium-review.googlesource.com/1038803 Reviewed-by: John Abd-El-Malek <jam@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org> Commit-Queue: Chong Zhang <chongz@chromium.org> Cr-Commit-Position: refs/heads/master@{#555553} [modify] https://crrev.com/5304776c9886a583278ebea8bb59418368773365/chrome/browser/task_manager/sampling/task_manager_impl.cc [modify] https://crrev.com/5304776c9886a583278ebea8bb59418368773365/chrome/browser/task_manager/sampling/task_manager_impl.h [modify] https://crrev.com/5304776c9886a583278ebea8bb59418368773365/chrome/browser/task_manager/task_manager_browsertest.cc [modify] https://crrev.com/5304776c9886a583278ebea8bb59418368773365/content/browser/loader/navigation_url_loader_network_service_unittest.cc [modify] https://crrev.com/5304776c9886a583278ebea8bb59418368773365/content/browser/network_service_restart_browsertest.cc [modify] https://crrev.com/5304776c9886a583278ebea8bb59418368773365/services/network/BUILD.gn [modify] https://crrev.com/5304776c9886a583278ebea8bb59418368773365/services/network/keepalive_statistics_recorder.cc [modify] https://crrev.com/5304776c9886a583278ebea8bb59418368773365/services/network/keepalive_statistics_recorder.h [modify] https://crrev.com/5304776c9886a583278ebea8bb59418368773365/services/network/network_service.cc [modify] https://crrev.com/5304776c9886a583278ebea8bb59418368773365/services/network/network_service.h [add] https://crrev.com/5304776c9886a583278ebea8bb59418368773365/services/network/network_usage_accumulator.cc [add] https://crrev.com/5304776c9886a583278ebea8bb59418368773365/services/network/network_usage_accumulator.h [add] https://crrev.com/5304776c9886a583278ebea8bb59418368773365/services/network/network_usage_accumulator_unittest.cc [modify] https://crrev.com/5304776c9886a583278ebea8bb59418368773365/services/network/public/mojom/network_service.mojom [modify] https://crrev.com/5304776c9886a583278ebea8bb59418368773365/services/network/url_loader.cc [modify] https://crrev.com/5304776c9886a583278ebea8bb59418368773365/services/network/url_loader.h [modify] https://crrev.com/5304776c9886a583278ebea8bb59418368773365/services/network/url_loader_factory.cc [modify] https://crrev.com/5304776c9886a583278ebea8bb59418368773365/services/network/url_loader_unittest.cc [modify] https://crrev.com/5304776c9886a583278ebea8bb59418368773365/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
,
May 2 2018
,
May 14 2018
Issue 841316 has been merged into this issue.
,
May 17 2018
Hey Chong, reopening this since it looks like TaskManagerUtilityProcessBrowserTest.UtilityJSHeapMemory is related.
,
May 18 2018
Actually `TaskManagerUtilityProcessBrowserTest.UtilityJSHeapMemory` is not caused by network traffic, but will just reuse this bug since it's a trivial fix.
,
May 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0500c353ce7d314aa9acb3029a6c06a16666d4c1 commit 0500c353ce7d314aa9acb3029a6c06a16666d4c1 Author: Chong Zhang <chongz@chromium.org> Date: Tue May 22 00:05:47 2018 Fix TaskManagerUtilityProcessBrowserTest.UtilityJSHeapMemory for Network Service Remove the expected count for utility processes since the Network Service process might showup as well. Bug: 822415 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo Change-Id: I1a5b74c412568e12fb2994e8fcb23d3660e40e3a Reviewed-on: https://chromium-review.googlesource.com/1066689 Reviewed-by: John Abd-El-Malek <jam@chromium.org> Commit-Queue: Chong Zhang <chongz@chromium.org> Cr-Commit-Position: refs/heads/master@{#560422} [modify] https://crrev.com/0500c353ce7d314aa9acb3029a6c06a16666d4c1/chrome/browser/task_manager/task_manager_browsertest.cc [modify] https://crrev.com/0500c353ce7d314aa9acb3029a6c06a16666d4c1/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
,
May 22 2018
|
||||
►
Sign in to add a comment |
||||
Comment 1 by mmenke@chromium.org
, Mar 15 2018