MakeNames: Generate global constants in a loop rather than in sequence.

Source/build/scripts/templates/MakeNames.cpp.tmpl

Index: Source/build/scripts/templates/MakeNames.cpp.tmpl
diff --git a/Source/build/scripts/templates/MakeNames.cpp.tmpl b/Source/build/scripts/templates/MakeNames.cpp.tmpl
index 27bad7d23aeb5cf5dbd3a557f3bb638c568248d9..8674a64558b7c2f0f17aba0d5bc1be29a93db7f2 100644
--- a/Source/build/scripts/templates/MakeNames.cpp.tmpl
+++ b/Source/build/scripts/templates/MakeNames.cpp.tmpl
@@ -5,32 +5,42 @@
 
 #include "{{namespace}}{{suffix}}Names.h"
 
-#include "wtf/StaticConstructors.h"
+#include "wtf/StdLibExtras.h"
 
 namespace blink {
 namespace {{namespace}}Names {
 
 using namespace WTF;
 
-{% for entry in entries|sort %}
+const int k{{suffix}}NameCount = {{entries|length}};
+
+void* {{suffix}}NamesStorage[k{{suffix}}NameCount * ((sizeof(AtomicString) + sizeof(void *) - 1) / sizeof(void *))];
+
+{% for entry in entries|sort(case_sensitive=True) %}

[Jens Widell, 2014/09/26 06:09:46]

Since we're sorting a list of structures (not strings) Jinja won't do
case-insensitive sorting either way, unless you also use attribute='name'. If
clarity is what's desired, adding attribute= is perhaps what we should do here.
Having just case_sensitive= is misleading. 

[Daniel Bratell, 2014/09/26 07:59:28]

Done.

 {% filter enable_conditional(entry.Conditional) %}
-DEFINE_GLOBAL(AtomicString, {{entry|symbol}})
+const AtomicString& {{entry|symbol}} = reinterpret_cast<AtomicString*>(&{{suffix}}NamesStorage)[{{loop.index0}}];
 {% endfilter %}
 {% endfor %}
 
 void init{{suffix}}()
 {
-    {% for entry in entries|sort %}
-    {% filter enable_conditional(entry.Conditional) %}
-    StringImpl* {{entry|symbol}}Impl = StringImpl::createStatic("{{entry|cpp_name}}", {{entry|cpp_name|length}}, {{entry|cpp_name|hash}});
-    {% endfilter %}
+    struct NameEntry {
+        const char* name;
+        unsigned hash;
+        unsigned char length;
+    };
+
+    static const NameEntry kNames[] = {
+    {% for entry in entries|sort(case_sensitive=True) %}
+        { "{{entry|cpp_name}}", {{entry|cpp_name|hash}}, {{entry|cpp_name|length}} },

[Jens Widell, 2014/09/26 06:09:46]

I don't really understand why "|cpp_name" is used here instead of ".name".

The "|cpp_name" filter is the same as the "|script_name" filter unless there's
an ImplementedAs= override. The "|script_name" filter returns
os.path.basename(entry['name']). This may make sense when the name is the name
of an interface or something like that, but for tag names, it seems irrelevant.
And *if* there was a tag with an ImplementedAs= for some reason, surely the real
name would be what should be used in this string literal in either case?

(Both implemented in Source/build/scripts/name_utilities.py.)

[Daniel Bratell, 2014/09/26 07:59:29]

I am not expert on this, but it seems that while MakeQualifiedNames are fed with
lists of attributes and tags, MakeNames are fed with paths to implementions, so
it can be name="core/xml/XMLHttpRequestProgressEvent" ...
name="core/storage/StorageEvent". So you need something to get the name out of
that. I can't see anything in this code that cares about the paths but I guess
they are there for a reason. Or maybe someone just thought this was the best
layer to strip them if they are there as a side effect.

[Jens Widell, 2014/09/26 08:44:57]

Right. (I'm sorry I didn't fully investigate before commenting.)

Amended comment:

I think this ought to be "|script_name" rather than "|cpp_name". For interfaces,
the "C++ name" is usually the same as the actual interface name, but sometimes
has to be different for some reason. WebSocket seems to be one such case; it's
implemented C++-side by a class named DOMWebSocket. (This appears to be the only
such case that affects make_names.py and this template.)

In the resulting generated gen/blink/modules/EventTargetModulesNames.cpp file,
the interface name string is "DOMWebSocket", but the interface's name is
actually "WebSocket". Using "|script_name" here, we get "WebSocket" instead.

This mostly affects what DOMWebSocket::interfaceName() returns, which in turn,
AFAICT, only affects behavior in InspectorDOMDebuggerAgent::willHandleEvent(). I
don't know exactly how.

In short: this is probably a bug, but I don't think we should change it in this
CL.

     {% endfor %}
+    };
 
-    {% for entry in entries|sort %}
-    {% filter enable_conditional(entry.Conditional) %}
-    new ((void*)&{{entry|symbol}}) AtomicString({{entry|symbol}}Impl);
-    {% endfilter %}
-    {% endfor %}
+    for (size_t i = 0; i < WTF_ARRAY_LENGTH(kNames); i++) {
+        StringImpl* stringImpl = StringImpl::createStatic(kNames[i].name, kNames[i].length, kNames[i].hash);
+        void* address = reinterpret_cast<AtomicString*>(&{{suffix}}NamesStorage) + i;
+        new (address) AtomicString(stringImpl);
+    }
 }
 
 } // {{namespace}}Names