Bug 9698 – Switch JSC Bindings from CFDictionary to HashMap

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Justin Haygood]

I poked around a bit yesterday at this, seems quite simple to do. It will be a good first step to allow a portable bindings system.

[Justin Haygood]

Created attachment 9148 [details]
Make NPRuntime use HashMap

First initial patch for porting of bindings to HashMap.

Untested, since I don't have a Mac available, but it does cleanly compile on VS2005. Also includes <stdint.h> into npruntime.h so that the integer and unsigned types are defined (int32_t, uint32_t, etc..) so that it compiles.

[Timothy Hatcher]

Patch includes tabs, please remove.

[Geoffrey Garen]

Comment on attachment 9148 [details]
Make NPRuntime use HashMap

Our style guidlines also call for splitting lines like this into two:
+	if (!stringIdentifierDictionary) stringIdentifierDictionary = new HashMap< const char *, PrivateIdentifier *>();

The patch looks correct overall.

[Justin Haygood]

Created attachment 9162 [details]
Make NPRuntime use HashMap

Here is a revised patch to follow code style guidelines.

[Geoffrey Garen]

A few things I tweaked before landing:
- The new patch you submitted still had tabs in it. You can use tools like emacs (Ctrl-s) and less (/) to search for tabs. A better solution is just to set your editor up to use spaces instead of tabs.
- A typedef can often make template code more readable.
- Our style guildines call for Type* rather than Type * in c++ code.

Please take note of these in the future.

Committed revision 15146.

[David Kilzer (:ddkilzer)]

This was rolled out in r15148 by Darin.  Log message:

- Rolled out HashMap implementation of NPRuntime, at least temporarily. Fixes hang in the bindings section of layout tests seen on the buildbot. This code was using HashMap<const char*, PrivateIdentifier*>. But that hashes based on pointer identity, not string value. The default hash for any pointer type is to hash based on the pointer. And WTF doesn't currently have a string hash for char*. We'll need to fix that before re-landing this patch. (Formatting was also incorrect -- extra spaces in parentheses.)

[Geoffrey Garen]

OK, marking r-.

[Eric Seidel (no email)]

This looks like it has been long since done.