Summary: | [Chromium][DRT] some PostDelayedTask() accesses possibly dangling this pointer. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Hajime Morrita <morrita> | ||||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | fishd, tkent | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Hajime Morrita
2011-02-06 18:43:14 PST
(In reply to comment #0) > In WebViewHost, there are several calls like: > webkit_support::PostDelayedTask(invokeFinishLastTextCheck, static_cast<void*>(this), 0); > But this is not safe when |this| object is deleted before the task callback is invoked. Yes. In that case, we had better use DumpRenderTree/chromium/Taks.h instead of the bare PostDelayedTask(). Created attachment 82766 [details]
Patch
Comment on attachment 82766 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=82766&action=review > Tools/DumpRenderTree/chromium/WebViewHost.cpp:419 > + postDelayedTask(new WebViewHost::HostMethodTask(this, &WebViewHost::finishLastTextCheck), 0); "WebViewHost::" looks redundant. > Tools/DumpRenderTree/chromium/WebViewHost.cpp:662 > + postDelayedTask(new WebViewHost::HostMethodTask(this, &WebViewHost::scheduleComposite), 0); ditto. > Tools/DumpRenderTree/chromium/WebViewHost.cpp:709 > + postDelayedTask(new WebViewHost::HostMethodTask(this, &WebViewHost::closeWidget), 0); ditto. > Tools/DumpRenderTree/chromium/WebViewHost.h:70 > + class HostMethodTask : public MethodTask<WebViewHost> { Do we need to make this class public? > Tools/DumpRenderTree/chromium/WebViewHost.h:76 > + {} nit: We usually use "{ }" (a blank between {}). Created attachment 82938 [details]
Patch
Hi Kent-san, thank you for reviewing! I updated the patch. (In reply to comment #3) > (From update of attachment 82766 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=82766&action=review > > > Tools/DumpRenderTree/chromium/WebViewHost.cpp:419 > > + postDelayedTask(new WebViewHost::HostMethodTask(this, &WebViewHost::finishLastTextCheck), 0); > > "WebViewHost::" looks redundant. Sure. removed. > > > Tools/DumpRenderTree/chromium/WebViewHost.cpp:662 > > + postDelayedTask(new WebViewHost::HostMethodTask(this, &WebViewHost::scheduleComposite), 0); > > ditto. Done. > > > Tools/DumpRenderTree/chromium/WebViewHost.cpp:709 > > + postDelayedTask(new WebViewHost::HostMethodTask(this, &WebViewHost::closeWidget), 0); > > ditto. Done. > > > Tools/DumpRenderTree/chromium/WebViewHost.h:70 > > + class HostMethodTask : public MethodTask<WebViewHost> { > > Do we need to make this class public? Good point. Moved to private. > > > Tools/DumpRenderTree/chromium/WebViewHost.h:76 > > + {} > > nit: We usually use "{ }" (a blank between {}). Done. Committed r78984: <http://trac.webkit.org/changeset/78984> |