Bug 23977 - Unnecessary timer related headers in files and unnecessary public constructor.
Summary: Unnecessary timer related headers in files and unnecessary public constructor.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: David Levin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-02-16 12:00 PST by David Levin
Modified: 2009-02-17 16:58 PST (History)
1 user (show)

See Also:


Attachments
Patch for bug. (2.60 KB, patch)
2009-02-16 12:08 PST, David Levin
no flags Details | Formatted Diff | Diff
Proposed fix. (2.60 KB, patch)
2009-02-16 14:31 PST, David Levin
ap: review+
Details | Formatted Diff | Diff
Proposed fix. (trivial cleanup) (1.64 KB, patch)
2009-02-17 10:49 PST, David Levin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Levin 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.
Comment 1 David Levin 2009-02-16 12:08:47 PST
Created attachment 27705 [details]
Patch for bug.
Comment 2 David Levin 2009-02-16 14:31:29 PST
Created attachment 27710 [details]
Proposed fix.

Removed extra parenthesis from last patch.
Comment 3 Dmitry Titov 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.
Comment 4 Alexey Proskuryakov 2009-02-17 00:32:34 PST
Comment on attachment 27710 [details]
Proposed fix.

r=me
Comment 5 Alexey Proskuryakov 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.
Comment 6 Alexey Proskuryakov 2009-02-17 00:34:54 PST
But I agree that this is a border case.
Comment 7 David Levin 2009-02-17 10:49:29 PST
Created attachment 27736 [details]
Proposed fix. (trivial cleanup)

Removed static create method at dimich's request.
Comment 8 Alexey Proskuryakov 2009-02-17 10:56:53 PST
Comment on attachment 27736 [details]
Proposed fix. (trivial cleanup)

r=me
Comment 9 David Levin 2009-02-17 16:56:18 PST
Comment on attachment 27736 [details]
Proposed fix. (trivial cleanup)

Committed as r41047.