Summary: | save resource zeroing stacks in ScriptElement for debugging | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gavin Peters <gavinp> | ||||||||||
Component: | New Bugs | Assignee: | Gavin Peters <gavinp> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | abarth, ap, aroben, japhet, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | 69472 | ||||||||||||
Bug Blocks: | |||||||||||||
Attachments: |
|
Description
Gavin Peters
2011-10-05 12:26:33 PDT
Created attachment 109837 [details]
Patch
add more data for crashes in ScriptElement::notifyFinished double calls. Comment on attachment 109837 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109837&action=review > Source/WebCore/dom/ScriptElement.h:120 > + MaxBacktraceSize = 32 bad indent Created attachment 109838 [details]
Patch
Comment on attachment 109838 [details]
Patch
Fixed indent.
Comment on attachment 109838 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109838&action=review > Source/WebCore/ChangeLog:8 > + two notifyFinished() events in a ScriptElement, which can crash chrome (see chrome bug > + 75604). This patch will save a stack in the ScriptElement when this happens, so that if Please provide a link to the chromium bug. > Source/WebCore/dom/ScriptElement.h:122 > + > + // We grab a backtrace when we zero m_cachedScript, so that at later crashes > + // we'll have a debuggable stack. > + enum { > + MaxBacktraceSize = 32 > + }; > + int m_backtraceSize; Does this value ever change? Can we just make it a static const in ScriptElement.cpp? Created attachment 109842 [details]
Patch
Comment on attachment 109838 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109838&action=review >> Source/WebCore/ChangeLog:8 >> + 75604). This patch will save a stack in the ScriptElement when this happens, so that if > > Please provide a link to the chromium bug. Done. >> Source/WebCore/dom/ScriptElement.h:122 >> + int m_backtraceSize; > > Does this value ever change? Can we just make it a static const in ScriptElement.cpp? It does change, it contains the actual size of the stack in m_backtrace, it could be anything from 0 to 32. Comment on attachment 109842 [details]
Patch
Now with links to the chromium bug database. Boy, isn't the chromium bug database really unbearably slow?
Comment on attachment 109842 [details] Patch Clearing flags on attachment: 109842 Committed r96756: <http://trac.webkit.org/changeset/96756> All reviewed patches have been landed. Closing bug. This broke many builds due to WTFGetBacktrace not being exported from JavaScriptCore. Rolling it out in bug 69472. I just added a new patch to bug 69018 which exports these functions, after that this patch should be good to go, possibly after rerunning ews. Created attachment 109957 [details]
Patch
Comment on attachment 109957 [details]
Patch
This patch is unchanged from the version that got a rollout; however I've since updated the exports from JavaScriptCore so it should work now. Let's wait on the windows ews bot in particular, then land it?
Comment on attachment 109957 [details]
Patch
Looks like it's passed the bots this time, so r+ again.
Committed r96819: <http://trac.webkit.org/changeset/96819> |