Bug 32389

Summary: [v8] refactor WeakReferenceMap
Product: WebKit Reporter: anton muhin <antonm>
Component: WebKit Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: christian.plesner.hansen, commit-queue, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
first take
abarth: review+
just say no to virtuals none

Description anton muhin 2009-12-10 12:25:25 PST
As spotted by Christian, there are some pieces of repeated code.  Bring those refactorings plus some more cleanups.
Comment 1 anton muhin 2009-12-10 12:30:20 PST
Created attachment 44633 [details]
first take
Comment 2 WebKit Review Bot 2009-12-10 12:34:30 PST
style-queue ran check-webkit-style on attachment 44633 [details] without any errors.
Comment 3 Adam Barth 2009-12-10 12:46:30 PST
Comment on attachment 44633 [details]
first take

Wow, this looks great.  I didn't consider using the visitor interface internally, but it makes for some prettier code.

+ virtual bool removeIfPresent
+ virtual void clear

Do these need to be virtual?  It doesn't look like they need to be overridden.
Comment 4 anton muhin 2009-12-11 06:16:55 PST
Created attachment 44681 [details]
just say no to virtuals
Comment 5 WebKit Review Bot 2009-12-11 06:17:58 PST
style-queue ran check-webkit-style on attachment 44681 [details] without any errors.
Comment 6 anton muhin 2009-12-11 06:20:26 PST
(In reply to comment #3)
> (From update of attachment 44633 [details])
> Wow, this looks great.  I didn't consider using the visitor interface
> internally, but it makes for some prettier code.
> 
> + virtual bool removeIfPresent
> + virtual void clear
> 
> Do these need to be virtual?  It doesn't look like they need to be overridden.

If we're lucky, they would be overridden. But conditionals are not for SVN, removing.

Thanks a lot for review.If we're lucky, they would be overridden. But conditionals are not for SVN, removing.

Thanks a lot for review.
Comment 7 WebKit Commit Bot 2009-12-11 09:21:23 PST
Comment on attachment 44681 [details]
just say no to virtuals

Clearing flags on attachment: 44681

Committed r51998: <http://trac.webkit.org/changeset/51998>
Comment 8 WebKit Commit Bot 2009-12-11 09:21:28 PST
All reviewed patches have been landed.  Closing bug.