RESOLVED FIXED 32389
[v8] refactor WeakReferenceMap 
https://bugs.webkit.org/show_bug.cgi?id=32389
Summary [v8] refactor WeakReferenceMap
anton muhin
Reported 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.
Attachments
first take (12.97 KB, patch)
2009-12-10 12:30 PST, anton muhin 		abarth: review+
DetailsFormatted DiffDiff
just say no to virtuals (13.00 KB, patch)
2009-12-11 06:16 PST, anton muhin 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
anton muhin
Comment 1 2009-12-10 12:30:20 PST
Created attachment 44633 [details] first take
WebKit Review Bot
Comment 2 2009-12-10 12:34:30 PST
style-queue ran check-webkit-style on attachment 44633 [details] without any errors.
Adam Barth
Comment 3 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.
anton muhin
Comment 4 2009-12-11 06:16:55 PST
Created attachment 44681 [details] just say no to virtuals
WebKit Review Bot
Comment 5 2009-12-11 06:17:58 PST
style-queue ran check-webkit-style on attachment 44681 [details] without any errors.
anton muhin
Comment 6 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.
WebKit Commit Bot
Comment 7 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>
WebKit Commit Bot
Comment 8 2009-12-11 09:21:28 PST
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.