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:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
first take	abarth: review+
just say no to virtuals	none

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+	Details Formatted Diff Diff
just say no to virtuals (13.00 KB, patch) 2009-12-11 06:16 PST, anton muhin	no flags	Details Formatted Diff Diff
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.