RESOLVED FIXED 91857
[chromium] MediaStream API: Clean up the MockWebKitPlatformSupport object at shutdown
https://bugs.webkit.org/show_bug.cgi?id=91857
Summary [chromium] MediaStream API: Clean up the MockWebKitPlatformSupport object at ...
Tommy Widenflycht
Reported 2012-07-20 07:00:59 PDT
It caused valgrind issues.
Attachments
Patch (3.41 KB, patch)
2012-07-20 07:03 PDT, Tommy Widenflycht
no flags
Patch (4.66 KB, patch)
2012-07-20 07:20 PDT, Tommy Widenflycht
no flags
Patch (5.07 KB, patch)
2012-07-23 01:05 PDT, Tommy Widenflycht
no flags
Tommy Widenflycht
Comment 1 2012-07-20 07:03:26 PDT
Tommy Widenflycht
Comment 2 2012-07-20 07:20:42 PDT
WebKit Review Bot
Comment 3 2012-07-20 07:22:21 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Adam Barth
Comment 4 2012-07-20 09:07:55 PDT
Comment on attachment 153501 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=153501&action=review > Tools/DumpRenderTree/chromium/DumpRenderTree.cpp:85 > + OwnPtr<MockWebKitPlatformSupport> mockPlatform; mockPlatform -> m_mockPlatform > Tools/DumpRenderTree/chromium/MockWebKitPlatformSupport.cpp:38 > -Platform* MockWebKitPlatformSupport::create() > +MockWebKitPlatformSupport* MockWebKitPlatformSupport::create() Should this return a PassOwnPtr? > Tools/DumpRenderTree/chromium/MockWebKitPlatformSupport.h:39 > + ~MockWebKitPlatformSupport() { } Please move this declaration out of line.
Tommy Widenflycht
Comment 5 2012-07-23 01:05:13 PDT
Tommy Widenflycht
Comment 6 2012-07-23 01:06:19 PDT
Comment on attachment 153501 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=153501&action=review >> Tools/DumpRenderTree/chromium/DumpRenderTree.cpp:85 >> + OwnPtr<MockWebKitPlatformSupport> mockPlatform; > > mockPlatform -> m_mockPlatform Done. >> Tools/DumpRenderTree/chromium/MockWebKitPlatformSupport.cpp:38 >> +MockWebKitPlatformSupport* MockWebKitPlatformSupport::create() > > Should this return a PassOwnPtr? Yes, done. >> Tools/DumpRenderTree/chromium/MockWebKitPlatformSupport.h:39 >> + ~MockWebKitPlatformSupport() { } > > Please move this declaration out of line. Done.
Adam Barth
Comment 7 2012-07-23 09:02:13 PDT
Comment on attachment 153750 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=153750&action=review > Tools/DumpRenderTree/chromium/MockWebKitPlatformSupport.cpp:40 > - return new MockWebKitPlatformSupport(); > + return WTF::adoptPtr(new MockWebKitPlatformSupport()); I think the WTF:: here is optional.
WebKit Review Bot
Comment 8 2012-07-23 09:15:45 PDT
Comment on attachment 153750 [details] Patch Clearing flags on attachment: 153750 Committed r123345: <http://trac.webkit.org/changeset/123345>
WebKit Review Bot
Comment 9 2012-07-23 09:15:50 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.