tabs.executeScript calls are failing validation; killing renderers |
|||
Issue descriptionUMA [1] seems to indicate that tabs.executeScript calls are accounting for far too many process kills. This happens when something on the browser fails validation in such a way that the *only* possible explanation would be a compromised (either fully or even just through our JS bindings) renderer, but it having a single API account for 98% of function kills seems suspicious. We should investigate. [1] https://uma.googleplex.com/histograms?endDate=08-30-2016&dayCount=7&histograms=Extensions.BadMessageFunctionName&fixupData=true&showMax=true&filters=channel%2Ceq%2C4%2Cisofficial%2Ceq%2CTrue&implicitFilters=isofficial
,
Jan 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0605ec9d2876c3c58b3f91ba5e82ca04c6ad5828 commit 0605ec9d2876c3c58b3f91ba5e82ca04c6ad5828 Author: lazyboy <lazyboy@chromium.org> Date: Tue Jan 17 23:28:49 2017 Upload crash reports when ExtensionFunctions generate bad messages. ExtensionFunctionDispatcher used to have its own implementation of bad message handling, which missed calling DumpWithoutCrashing(). Use extensions::bad_message namespace's functions instead, with little unfortunate deviation in logging to be consistent with the existing UMA. Note that this is not a fix for issue 642794. This CL will help tracking 642794. BUG=642794 Test=Note that we should start seeing crash reports for bad message termination of ExtensionFunctions. This should give us a clue narrow down reasons that cause crbug 642794. Review-Url: https://codereview.chromium.org/2629783002 Cr-Commit-Position: refs/heads/master@{#444183} [modify] https://crrev.com/0605ec9d2876c3c58b3f91ba5e82ca04c6ad5828/extensions/browser/bad_message.cc [modify] https://crrev.com/0605ec9d2876c3c58b3f91ba5e82ca04c6ad5828/extensions/browser/bad_message.h [modify] https://crrev.com/0605ec9d2876c3c58b3f91ba5e82ca04c6ad5828/extensions/browser/extension_function_dispatcher.cc
,
Jan 23 2017
Issue 492702 has been merged into this issue.
,
Jan 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c93597557d1bee873c2e47ad18639d1f1b34d2b2 commit c93597557d1bee873c2e47ad18639d1f1b34d2b2 Author: lazyboy <lazyboy@chromium.org> Date: Wed Jan 25 01:01:14 2017 Separate validation failures from other failures in ExecuteCodeFunction. ExecuteCodeFunction::Init() used to treat all failures as fatal, leading to validation failures, resulting in renderer kills. This is likely the cause of high number of tabs.executeScript/tabs.insertCSS failures we see in UMA. During shutdown, the browser window can be null or it might not have any active tabs. Make ExecuteCodeInTabFunction return an error instead of blowing up the renderer with bad_message. This CL treats these kind of of errors non-fatal and returns error from ExtensionFunction instead. Add unit test to cover one case of non-fatal failure. BUG=642794 Test=Have an extension keep the browser busy with running scripts [1] and shutdown the browser (Ctrl-c) while it is running those scripts. Renderer kill shouldn't happen anymore. [1] example: var currentIter = 0; var runScript = function() { ++currentIter; if (currentIter > 1000) return; chrome.tabs.executeScript(undefined, {file:"content.js", runScript); }; chrome.browserAction.onClicked.addListener(runScript); Review-Url: https://codereview.chromium.org/2630753003 Cr-Commit-Position: refs/heads/master@{#445887} [modify] https://crrev.com/c93597557d1bee873c2e47ad18639d1f1b34d2b2/chrome/browser/extensions/api/tabs/tabs_api.cc [modify] https://crrev.com/c93597557d1bee873c2e47ad18639d1f1b34d2b2/chrome/browser/extensions/api/tabs/tabs_api.h [modify] https://crrev.com/c93597557d1bee873c2e47ad18639d1f1b34d2b2/chrome/browser/extensions/api/tabs/tabs_api_unittest.cc [modify] https://crrev.com/c93597557d1bee873c2e47ad18639d1f1b34d2b2/chrome/browser/extensions/api/tabs/tabs_constants.cc [modify] https://crrev.com/c93597557d1bee873c2e47ad18639d1f1b34d2b2/chrome/browser/extensions/api/tabs/tabs_constants.h [modify] https://crrev.com/c93597557d1bee873c2e47ad18639d1f1b34d2b2/extensions/browser/api/execute_code_function.cc [modify] https://crrev.com/c93597557d1bee873c2e47ad18639d1f1b34d2b2/extensions/browser/api/execute_code_function.h [modify] https://crrev.com/c93597557d1bee873c2e47ad18639d1f1b34d2b2/extensions/browser/api/guest_view/web_view/web_view_internal_api.cc [modify] https://crrev.com/c93597557d1bee873c2e47ad18639d1f1b34d2b2/extensions/browser/api/guest_view/web_view/web_view_internal_api.h
,
Jan 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5e465202abc260c9aa8319f8e77bf721e0453ed2 commit 5e465202abc260c9aa8319f8e77bf721e0453ed2 Author: lazyboy <lazyboy@chromium.org> Date: Sat Jan 28 01:13:47 2017 Better crash stacktraces for ExtensionFunction bad messages. Record crash trace right where we set bad_message in ExtensionFunction This will give us clue about which particular extension functions are crashing. Previously, we used to record the trace from common response callback where we already lost track of which function actually set a bad_message. Turn ExtensionFunction::set_bad_message(bool) to EF::SetBadMessage(void) since one should only use this function to set bad_message state, not clear it. BUG=642794 Test=No visible change. Beware though, similar to https://codereview.chromium.org/2629783002, this might show up as a blame CL for extension function crashes. These will not really be new crashes, so file a (non-release-blocking) bug. Review-Url: https://codereview.chromium.org/2656013005 Cr-Commit-Position: refs/heads/master@{#446865} [modify] https://crrev.com/5e465202abc260c9aa8319f8e77bf721e0453ed2/extensions/browser/extension_function.cc [modify] https://crrev.com/5e465202abc260c9aa8319f8e77bf721e0453ed2/extensions/browser/extension_function.h [modify] https://crrev.com/5e465202abc260c9aa8319f8e77bf721e0453ed2/extensions/browser/extension_function_dispatcher.cc
,
Feb 8 2018
,
May 15 2018
|
|||
►
Sign in to add a comment |
|||
Comment 1 by lazyboy@chromium.org
, Jan 13 2017Status: Started (was: Untriaged)