Add a const char* parameter to ProcessManager::Increment/DecrementKeepaliveCount() and track in Debug builds |
||||
Issue descriptionTo simplify tracking of the cause of lazy extension processes unexpectedly lingering, we could add a parameter to the Increment/Decrement APIs which describes the caller (e.g. the name of the async Extension API that wants to keep the Extension alive). Debug builds can maintain a table of ref-counts per description/name, so that we can expose the information e.g. via chrome://extensions, or Inspect. Alternatively, identifying a small set of "buckets" describing keepalive reasons (e.g. AsyncApiPending, PluginImpulse, PpapiPlugin, etc) could be used to present similar information in some abbreviated form in Task Manager even in Release builds.
,
Feb 24 2017
If we took the enum-based approach (i.e. a small number of buckets) then we could, e.g replace the keep-alive count column with some sort of coded indication of the reasons, e.g. if a page has one or more tabs using it it'd have a "T", if it is waiting on an API call it'd also have an "A", if it has a plugin active then "P" and so on. It wouldn't be pretty by any means, but useful for us and developers. The other option would be to expose the information via an extension-accessible API - when things are alive it should always be possible to go into Developer Tools, run a command to query the keepalive reasons - though we'd need a way to exclude the API itself from the reasons ;)
,
Feb 24 2017
So with the enum-based approach, we could expect to see, e.g. TTAAAAAP in the keepalive? I guess it's reasonably descriptive, if somewhat unorthodox. Re the extension API approach, that's also an interesting thought, and I could see something like a chrome.runtime.getKeepalive(), but I worry about whether there's valid use cases outside of debugging.
,
Feb 24 2017
No, I think I'd limit it to one instance of each "reason", so it doesn't become unweildy, e.g. you'd get a single T no matter how many tabs are using the extension. The most interesting cases we'll want to diagnose are, I suspect, at the 0->1 reference boundary. Re the API, yes, it'd be preferable to have an API that actually means something useful in a more general context. e.g. if there were a standard way to ask "what pages are currently relying on this ServiceWorker" then we could perhaps crowbar Extension keep-alive reasons into that?
,
Feb 24 2017
> The most interesting cases we'll want to diagnose are, I suspect, at the 0->1 reference boundary. I'm not sure I agree - it could be, e.g., that a number of API functions are never finishing, and thus the keepalive is never being decremented. The indication of that would be a ton of AAAAAAs that never go away.
,
Feb 24 2017
I'm thinking of any info in Task Manager as the indicator that there's an unexpected problem, with some other means (e.g. chrome://inspect, chrome://extensions, DevTools) as the means to find out the detail.
,
Feb 24 2017
Ah, okay. In that case, it might be best to start with just the detailed form, since the keepalive count in the task manager is a good indication of whether or not more investigation is warranted. This probably isn't something I'll be able to get to soon, so marking as available, but if you wanna jump on it, I'm happy to help review. :)
,
Feb 24 2017
If you have an idea of where to surface this then I'd be happy to - my suggestion would be in chrome://extensions, as a "Used-by:" line below the links to the active WebContents for each.
,
Mar 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/adhd/+/ab692f0b051c25a94e30d25c906df321b3fc443d commit ab692f0b051c25a94e30d25c906df321b3fc443d Author: Cheng-Yi Chiang <cychiang@chromium.org> Date: Thu Mar 02 02:42:15 2017 CRAS: iodev_list - Fix argument of cras_iodev_list_dev_is_enabled This function queries whether a device is enabled, so it should take a pointer to const iodev. BUG=chromium:695711 TEST=make check Change-Id: I14f3460089b66059fd0b9315bc0bf79672769827 Reviewed-on: https://chromium-review.googlesource.com/448241 Commit-Ready: Cheng-Yi Chiang <cychiang@chromium.org> Tested-by: Cheng-Yi Chiang <cychiang@chromium.org> Reviewed-by: Dylan Reid <dgreid@chromium.org> [modify] https://crrev.com/ab692f0b051c25a94e30d25c906df321b3fc443d/cras/src/server/cras_iodev_list.c [modify] https://crrev.com/ab692f0b051c25a94e30d25c906df321b3fc443d/cras/src/server/cras_iodev_list.h [modify] https://crrev.com/ab692f0b051c25a94e30d25c906df321b3fc443d/cras/src/tests/bt_device_unittest.cc
,
Mar 2 2017
Please ignore CL in #9, the bug number was a typo for issue 697511
,
Mar 6 2018
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 3
,
Aug 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c063c77087274c4f13b0694a42ebcf3f4627d62e commit c063c77087274c4f13b0694a42ebcf3f4627d62e Author: David Bertoni <dbertoni@chromium.org> Date: Mon Aug 27 16:12:56 2018 [Extensions] Rearranged code so we only ACK events that are expected. Bug: 695711 Change-Id: Ic286b58b0d9a9984428e1db62ce82bd6f9c0ec0e Reviewed-on: https://chromium-review.googlesource.com/1187826 Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Commit-Queue: David Bertoni <dbertoni@chromium.org> Cr-Commit-Position: refs/heads/master@{#586275} [modify] https://crrev.com/c063c77087274c4f13b0694a42ebcf3f4627d62e/extensions/browser/extension_host.cc
,
Aug 28
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/79661573b41734cdf7fcb2f2e637ef1023d9cb57 commit 79661573b41734cdf7fcb2f2e637ef1023d9cb57 Author: David Bertoni <dbertoni@chromium.org> Date: Tue Aug 28 23:51:14 2018 [Extensions] Initial implementation of the chrome://extensions-internals page. Design doc is here: https://docs.google.com/document/d/1wYrk_EVzWF_df9OXl2Th0eHlCFlX5CpNqK-8lHg6Z7A/edit?usp=sharing The extension fields included in the current implementation are: ID Keepalive count Location Manifest version Name Path Type Version Bug: 695711 Change-Id: I7b1d61383b2107024e4b56cbd5389ab8fe20bee7 Reviewed-on: https://chromium-review.googlesource.com/1168119 Commit-Queue: David Bertoni <dbertoni@chromium.org> Reviewed-by: Michael Giuffrida <michaelpg@chromium.org> Reviewed-by: Karan Bhatia <karandeepb@chromium.org> Cr-Commit-Position: refs/heads/master@{#586939} [modify] https://crrev.com/79661573b41734cdf7fcb2f2e637ef1023d9cb57/chrome/browser/extensions/extension_system_impl.cc [modify] https://crrev.com/79661573b41734cdf7fcb2f2e637ef1023d9cb57/chrome/browser/ui/BUILD.gn [add] https://crrev.com/79661573b41734cdf7fcb2f2e637ef1023d9cb57/chrome/browser/ui/webui/extensions/extensions_internals_browsertest.cc [add] https://crrev.com/79661573b41734cdf7fcb2f2e637ef1023d9cb57/chrome/browser/ui/webui/extensions/extensions_internals_source.cc [add] https://crrev.com/79661573b41734cdf7fcb2f2e637ef1023d9cb57/chrome/browser/ui/webui/extensions/extensions_internals_source.h [modify] https://crrev.com/79661573b41734cdf7fcb2f2e637ef1023d9cb57/chrome/common/webui_url_constants.cc [modify] https://crrev.com/79661573b41734cdf7fcb2f2e637ef1023d9cb57/chrome/common/webui_url_constants.h [modify] https://crrev.com/79661573b41734cdf7fcb2f2e637ef1023d9cb57/chrome/test/BUILD.gn
,
Aug 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f commit 3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f Author: David Bertoni <dbertoni@chromium.org> Date: Wed Aug 29 20:39:30 2018 [Extensions] Initial implementation of enhanced keepalive data. The CL adds additional data when incrementing and decrementing the keepalive count, to allow for easier debugging of the counts. Bug: 695711 Change-Id: Ibd078259c34cc11f786b3f895dc63c6452025604 Reviewed-on: https://chromium-review.googlesource.com/1168139 Commit-Queue: David Bertoni <dbertoni@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Cr-Commit-Position: refs/heads/master@{#587285} [modify] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/chrome/browser/extensions/app_background_page_apitest.cc [modify] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/chrome/browser/extensions/extension_loading_browsertest.cc [modify] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/chrome/browser/extensions/lazy_background_page_apitest.cc [modify] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/chrome/browser/extensions/process_manager_browsertest.cc [modify] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/chrome/browser/extensions/service_worker_apitest.cc [modify] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/extensions/browser/BUILD.gn [add] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/extensions/browser/activity.cc [add] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/extensions/browser/activity.h [modify] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/extensions/browser/api/messaging/extension_message_port.cc [modify] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/extensions/browser/event_router.cc [modify] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/extensions/browser/event_router.h [modify] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/extensions/browser/extension_function.cc [modify] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/extensions/browser/extension_function_dispatcher.cc [modify] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/extensions/browser/extension_function_dispatcher.h [modify] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/extensions/browser/extension_host.cc [modify] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/extensions/browser/extension_host.h [modify] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/extensions/browser/extension_registrar.cc [modify] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/extensions/browser/extension_web_contents_observer.cc [modify] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/extensions/browser/lazy_background_task_queue.cc [modify] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/extensions/browser/mojo/keep_alive_impl.cc [modify] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/extensions/browser/mojo/keep_alive_impl_unittest.cc [modify] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/extensions/browser/process_manager.cc [modify] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/extensions/browser/process_manager.h [modify] https://crrev.com/3e1e9fac1331b2dedab5d611e6cc9ed08fb0fc0f/extensions/components/javascript_dialog_extensions_client/javascript_dialog_extension_client_impl.cc
,
Aug 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2cb53ba1f4b64078b4394237ffd74ee09ff2c62c commit 2cb53ba1f4b64078b4394237ffd74ee09ff2c62c Author: David Bertoni <dbertoni@chromium.org> Date: Thu Aug 30 21:08:33 2018 [Extensions] Add the keepalive data to the extensions-internals page. This CL integrates the new enhanced keepalive data into the JSON output for the chrome://extensions-internals page. Bug: 695711 Change-Id: I7cc3b09dbccd2fef9379f2cb68939f4e029136cf Reviewed-on: https://chromium-review.googlesource.com/1196169 Commit-Queue: David Bertoni <dbertoni@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Cr-Commit-Position: refs/heads/master@{#587763} [modify] https://crrev.com/2cb53ba1f4b64078b4394237ffd74ee09ff2c62c/chrome/browser/ui/webui/extensions/extensions_internals_source.cc [modify] https://crrev.com/2cb53ba1f4b64078b4394237ffd74ee09ff2c62c/extensions/browser/activity.cc [modify] https://crrev.com/2cb53ba1f4b64078b4394237ffd74ee09ff2c62c/extensions/browser/activity.h |
||||
►
Sign in to add a comment |
||||
Comment 1 by rdevlin....@chromium.org
, Feb 24 2017