Bug 9698 - Switch JSC Bindings from CFDictionary to HashMap
Summary: Switch JSC Bindings from CFDictionary to HashMap
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 420+
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-07-02 10:33 PDT by Justin Haygood
Modified: 2007-10-01 08:48 PDT (History)
0 users

See Also:


Attachments
Make NPRuntime use HashMap (5.42 KB, patch)
2006-07-02 10:58 PDT, Justin Haygood
ggaren: review-
Details | Formatted Diff | Diff
Make NPRuntime use HashMap (5.41 KB, patch)
2006-07-02 18:48 PDT, Justin Haygood
ggaren: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Justin Haygood 2006-07-02 10:33:55 PDT
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.
Comment 1 Justin Haygood 2006-07-02 10:58:33 PDT
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.
Comment 2 Timothy Hatcher 2006-07-02 15:06:28 PDT
Patch includes tabs, please remove.
Comment 3 Geoffrey Garen 2006-07-02 16:22:00 PDT
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.
Comment 4 Justin Haygood 2006-07-02 18:48:51 PDT
Created attachment 9162 [details]
Make NPRuntime use HashMap

Here is a revised patch to follow code style guidelines.
Comment 5 Geoffrey Garen 2006-07-03 10:33:31 PDT
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.
Comment 6 David Kilzer (:ddkilzer) 2006-07-03 18:42:22 PDT
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.)

Comment 7 Geoffrey Garen 2006-07-03 19:54:27 PDT
OK, marking r-.
Comment 8 Eric Seidel (no email) 2007-10-01 08:48:02 PDT
This looks like it has been long since done.