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. Pointed at Bug 51013.
(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>