Bug 119701 - Encapsulate access to documentNamedItemMap and windowNamedItemMap
Summary: Encapsulate access to documentNamedItemMap and windowNamedItemMap
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on: 119700
Blocks:
  Show dependency treegraph
 
Reported: 2013-08-12 14:43 PDT by Ryosuke Niwa
Modified: 2013-08-12 18:11 PDT (History)
7 users (show)

See Also:


Attachments
Cleanup (14.66 KB, patch)
2013-08-12 16:12 PDT, Ryosuke Niwa
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2013-08-12 14:43:59 PDT
We should encapsulate documentNamedItemMap and windowNamedItemMap so that random code in WebCore doesn't modify them.
Comment 1 Ryosuke Niwa 2013-08-12 16:12:53 PDT
Created attachment 208569 [details]
Cleanup
Comment 2 Darin Adler 2013-08-12 16:19:44 PDT
Comment on attachment 208569 [details]
Cleanup

The need to not call these functions with a null string is less obvious when calling through these new functions than it was when dealing with the map directly; it’s easy to get that wrong at call sites. I worry a little about that.
Comment 3 Ryosuke Niwa 2013-08-12 16:25:07 PDT
(In reply to comment #2)
> (From update of attachment 208569 [details])
> The need to not call these functions with a null string is less obvious when calling through these new functions than it was when dealing with the map directly; it’s easy to get that wrong at call sites. I worry a little about that.

Yeah, I can't think of a good way to mitigate that. At least there is an assertion in the hash table not to insert null strings.
Comment 4 Ryosuke Niwa 2013-08-12 16:30:20 PDT
Ben says we also assert in HashTable::get so we can at least check it at runtime.
Comment 5 Ryosuke Niwa 2013-08-12 18:11:03 PDT
Committed r153970: <http://trac.webkit.org/changeset/153970>