Bug 121378 – Shrink factory functions

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Darin Adler]

Shrink factory functions

[Darin Adler]

Created attachment 211691 [details]
Patch

[Darin Adler]

Created attachment 211693 [details]
Patch

[Andreas Kling]

Comment on attachment 211693 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=211693&action=review

r=me

> Source/WebCore/ChangeLog:55
> +        * html/HTMLElement.cpp:
> +        (WebCore::populateEventNameForAttributeLocalNameMap): Changed to use an
> +        AtomicStringImpl* as the key instead of AtomicString, since all the strings
> +        come from the local names of tags, and so don't need to be ref'd. This saves
> +        a bit of reference count churn when building the map and is the same pattern
> +        used in some maps in the make_names.pl script above.

Clever.

> Source/WebCore/ChangeLog:84
> +        (WebCore::WebVTTParser::constructTreeFromToken): Pass docuemnt reference rather than

Typo, document.

> Source/WebCore/dom/make_names.pl:999
> -        print F "        if (PassRefPtr<$parameters{namespace}Element> element = function(qName, document, formElement, createdByParser))\n";
> -        print F "            return element;\n";
> +        print F "        if (RefPtr<$parameters{namespace}Element> element = function(name, document, formElement, createdByParser))\n";
> +        print F "            return element.release();\n";

Heh, I resisted telling Sam to make this change just a few hours ago.

[Darin Adler]

Created attachment 211695 [details]
Patch

[Darin Adler]

I’ll land this once either EWS or my local build is done.

[Early Warning System Bot]

Comment on attachment 211695 [details]
Patch

Attachment 211695 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/1801789

[Build Bot]

Comment on attachment 211695 [details]
Patch

Attachment 211695 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1808554

[Early Warning System Bot]

Comment on attachment 211695 [details]
Patch

Attachment 211695 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/1916277

[Darin Adler]

Created attachment 211696 [details]
Patch

[Darin Adler]

Committed r155801: <http://trac.webkit.org/changeset/155801>

[Brent Fulgham]

Comment on attachment 211696 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=211696&action=review

> Source/WebCore/dom/make_names.pl:755
> +    print(F "    new (NotNull, static_cast<void*>(&${lowerNamespace}NamespaceURI)) AtomicString(${lowerNamespace}NS);\n");

This change breaks the Windows build:

10>C:\Projects\WebKit\OpenSource\WebKitBuild\Release\obj32\WebCore\DerivedSources\WebKitFontFamilyNames.cpp(143): error C2440: 'static_cast' : cannot convert from 'const WTF::AtomicString *' to 'void *'
10>          Conversion loses qualifiers

Perhaps this was doing the wrong thing, and the C-style cast was masking the problem. But the build is now broken.

This may be related to the declaration of SKIP_STATIC_CONSTRUCTORS_ON_GCC, which is not active on Windows.

Any ideas for resolving this?

[Darin Adler]

Seems fine to change it back to a C-style cast.

[Darin Adler]

According to http://andreaskling.com/~kling/sizes/r155849/WebCore.txt, this patch actually made HTMLNames::init bigger. Something is going wrong.