Bug 48872

Summary: [chromium] Fix LayoutTestController UMRs.
Product: WebKit Reporter: Stephen White <senorblanco>
Component: New BugsAssignee: Stephen White <senorblanco>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, dpranke, tkent, tony
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch tony: review+

Description Stephen White 2010-11-02 14:29:39 PDT
[chromium] Fix LayoutTestController UMRs.
Comment 1 Stephen White 2010-11-02 14:32:03 PDT
While trying to track down some test flakiness, I ran valgrind on DumpRenderTree svg/custom/marker-child-changes.svg.  This exposed the following:

==23643== Conditional jump or move depends on uninitialised value(s)
==23643==    at 0x45A979: WebViewHost::didCreateDataSource(WebKit::WebFrame*, WebKit::WebDataSource*) (WebViewHost.cpp:810)
==23643==    by 0x4C8358: WebKit::FrameLoaderClientImpl::createDocumentLoader(WebCore::ResourceRequest const&, WebCore::SubstituteData const&) (FrameLoaderClientImpl.cpp:1313)
==23643==    by 0x10601D5: WebCore::FrameLoader::init() (FrameLoader.cpp:234)
==23643==    by 0x47A883: WebCore::Frame::init() (Frame.h:253)
==23643==    by 0x476F0D: WebKit::WebFrameImpl::initializeAsMainFrame(WebKit::WebViewImpl*) (WebFrameImpl.cpp:1826)
==23643==    by 0x49AC92: WebKit::WebViewImpl::initializeMainFrame(WebKit::WebFrameClient*) (WebViewImpl.cpp:242)
==23643==    by 0x45074D: TestShell::createNewWindow(WebKit::WebURL const&) (TestShell.cpp:577)
==23643==    by 0x450677: TestShell::createWebView() (TestShell.cpp:568)
==23643==    by 0x44E011: TestShell::TestShell(bool) (TestShell.cpp:110)
==23643==    by 0x42A0D0: main (DumpRenderTree.cpp:178)
==23643== 
==23643== Conditional jump or move depends on uninitialised value(s)
==23643==    at 0x43A025: LayoutTestController::reset() (LayoutTestController.cpp:562)
==23643==    by 0x44EA47: TestShell::resetTestController() (TestShell.cpp:217)
==23643==    by 0x4298BB: runTest(TestShell&, TestParams&, std::string const&, bool) (DumpRenderTree.cpp:90)
==23643==    by 0x42A324: main (DumpRenderTree.cpp:200)
==23643==
Comment 2 Stephen White 2010-11-02 14:33:38 PDT
Created attachment 72737 [details]
Patch
Comment 3 Stephen White 2010-11-02 14:34:25 PDT
This patch only initializes the members caught by valgrind.  I wasn't sure if maybe the whole thing should be initialized.
Comment 4 Tony Chang 2010-11-02 14:42:12 PDT
Comment on attachment 72737 [details]
Patch

thanks!
Comment 5 Dirk Pranke 2010-11-02 14:47:16 PDT
should m_closeRemainingWindows be set using bindMethod()?
Comment 6 Stephen White 2010-11-02 14:57:50 PDT
(In reply to comment #5)
> should m_closeRemainingWindows be set using bindMethod()?
Comment 7 Stephen White 2010-11-02 14:59:31 PDT
(In reply to comment #5)
> should m_closeRemainingWindows be set using bindMethod()?

To be honest, I don't know this code well enough to say.  I was trying to do the minimum necessary to get rid of the valgrind warnings.

At a glance, it seems bindMethod() binds javascript functions to member functions in the class.  So I don't think it's directly related to initializing member vars.  But I could be wrong.
Comment 8 Stephen White 2010-11-03 07:55:26 PDT
Landed as http://trac.webkit.org/changeset/71238.  Closing bug.