Bug 23977

Summary: Unnecessary timer related headers in files and unnecessary public constructor.
Product: WebKit Reporter: David Levin <levin>
Component: DOMAssignee: David Levin <levin>
Status: RESOLVED FIXED    
Severity: Normal CC: ap
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch for bug.
none
Proposed fix.
ap: review+
Proposed fix. (trivial cleanup) none

David Levin
Reported 2009-02-16 12:00:34 PST
A few timer related headers are included where they aren't needed. The WorkerSharedTimer constructor is public but it is only used by doing "new WorkerSharedTimer" so it should be exposed using the static std::auto_ptr<> create() pattern.
Attachments
Patch for bug. (2.60 KB, patch)
2009-02-16 12:08 PST, David Levin 		no flags
DetailsFormatted DiffDiff
Proposed fix. (2.60 KB, patch)
2009-02-16 14:31 PST, David Levin 		ap: review+
DetailsFormatted DiffDiff
Proposed fix. (trivial cleanup) (1.64 KB, patch)
2009-02-17 10:49 PST, David Levin 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
David Levin
Comment 1 2009-02-16 12:08:47 PST
Created attachment 27705 [details] Patch for bug.
David Levin
Comment 2 2009-02-16 14:31:29 PST
Created attachment 27710 [details] Proposed fix. Removed extra parenthesis from last patch.
Dmitry Titov
Comment 3 2009-02-16 23:19:17 PST
Is there indeed a value in adding the ::create to this private (for this cpp) and very simple class? It's a trivial helper class and seems to be more readable with just a simple constructor. I see there could be value in having create() method where it'd return PassRefPtr (to support refcounting), or there is an involved initialization that can have failures... Not sure there is a benefit in this case that is worthy of including extra header and more code.
Alexey Proskuryakov
Comment 4 2009-02-17 00:32:34 PST
Comment on attachment 27710 [details] Proposed fix. r=me
Alexey Proskuryakov
Comment 5 2009-02-17 00:34:06 PST
I think that the benefit is in following the same model everywhere, so searches (or automated tools) that someone may use to analyze code for possible leaks will not be tripped.
Alexey Proskuryakov
Comment 6 2009-02-17 00:34:54 PST
But I agree that this is a border case.
David Levin
Comment 7 2009-02-17 10:49:29 PST
Created attachment 27736 [details] Proposed fix. (trivial cleanup) Removed static create method at dimich's request.
Alexey Proskuryakov
Comment 8 2009-02-17 10:56:53 PST
Comment on attachment 27736 [details] Proposed fix. (trivial cleanup) r=me
David Levin
Comment 9 2009-02-17 16:56:18 PST
Comment on attachment 27736 [details] Proposed fix. (trivial cleanup) Committed as r41047.
Note You need to log in before you can comment on or make changes to this bug.