Bug 25425

Summary: DOM attribute getter/setter functions should use const AtomicString& type
Product: WebKit Reporter: Darin Adler <darin>
Component: DOMAssignee: Darin Adler <darin>
Status: RESOLVED WONTFIX    
Severity: Normal CC: arv, cdumez, dglazkov
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 27273    
Attachments:
Description Flags
patch with some work on this and other minor style issues
none
patch for part 1
none
patch for part 2
none
patch that does this for eight classes
none
patch that does this for four more classes
none
More of this type of thing none

Darin Adler
Reported 2009-04-27 09:17:11 PDT
It’s more efficient for the DOM binding if DOM getter/setter functions use const AtomicString& for their results and arguments. Existing code uses String for results and const String& for arguments. The former results in unnecessary reference counting churn and re-lookup in the atomic string hash table. The latter can result in unnecessary memory allocation when the string in question is already present in the atomic string hash table.
Attachments
patch with some work on this and other minor style issues (232.65 KB, patch)
2009-04-27 09:17 PDT, Darin Adler 		no flags
DetailsFormatted DiffDiff
patch for part 1 (16.13 KB, patch)
2009-06-21 22:30 PDT, Darin Adler 		no flags
DetailsFormatted DiffDiff
patch for part 2 (10.64 KB, patch)
2009-06-22 09:33 PDT, Darin Adler 		no flags
DetailsFormatted DiffDiff
patch that does this for eight classes (48.71 KB, patch)
2009-07-14 11:19 PDT, Darin Adler 		no flags
DetailsFormatted DiffDiff
patch that does this for four more classes (9.70 KB, patch)
2009-08-16 22:05 PDT, Darin Adler 		no flags
DetailsFormatted DiffDiff
More of this type of thing (25.49 KB, patch)
2010-03-15 09:45 PDT, Darin Adler 		no flags
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Darin Adler
Comment 1 2009-04-27 09:17:44 PDT
Created attachment 29820 [details] patch with some work on this and other minor style issues
Darin Adler
Comment 2 2009-06-21 22:30:36 PDT
Created attachment 31632 [details] patch for part 1
Darin Adler
Comment 3 2009-06-22 01:21:05 PDT
Comment on attachment 31632 [details] patch for part 1 Clear review flag since this is landed and there is more work to do. (Sorry, bdash.)
Dimitri Glazkov (Google)
Comment 4 2009-06-22 08:07:31 PDT
I'll take care of the V8 bindings.
Darin Adler
Comment 5 2009-06-22 09:33:28 PDT
Created attachment 31652 [details] patch for part 2
Darin Adler
Comment 6 2009-06-22 18:06:18 PDT
Comment on attachment 31652 [details] patch for part 2 Clear review flag since this is landed and there is more work to do.
Darin Adler
Comment 7 2009-07-14 11:19:37 PDT
Created attachment 32722 [details] patch that does this for eight classes
Dimitri Glazkov (Google)
Comment 8 2009-07-14 12:58:23 PDT
Can you hold off landing this for a bit -- let me catch up w/V8 bindings.
Dimitri Glazkov (Google)
Comment 9 2009-07-14 15:09:25 PDT
Comment on attachment 32722 [details] patch that does this for eight classes LGTM :)
Dimitri Glazkov (Google)
Comment 10 2009-07-14 15:09:54 PDT
V8 side landed as http://trac.webkit.org/changeset/45874, so we're good to go.
Darin Adler
Comment 11 2009-08-16 22:03:22 PDT
Comment on attachment 32722 [details] patch that does this for eight classes Landed as http://trac.webkit.org/changeset/45893
Darin Adler
Comment 12 2009-08-16 22:05:26 PDT
Created attachment 34947 [details] patch that does this for four more classes
Darin Adler
Comment 13 2009-08-17 00:13:23 PDT
Comment on attachment 34947 [details] patch that does this for four more classes Landed as http://trac.webkit.org/changeset/47352
Darin Adler
Comment 14 2010-03-15 09:45:15 PDT
Created attachment 50715 [details] More of this type of thing
Darin Adler
Comment 15 2018-12-15 15:03:25 PST
I don’t think we need this bug any more. We’ve done lots more of this type of work over the years.
Note You need to log in before you can comment on or make changes to this bug.