ASCIICaseFoldTable should be defined in a .cc file

ASCIICaseFoldTable should be defined in a .cc file

In VS 2015 builds of chrome.dll there are 2473 copies of the ASCIICaseFoldTable array, totaling 633088 bytes.

BUG=541668
Committed: https://crrev.com/e3e1da9db2a3916d8680772c7d399f208c295474
Cr-Commit-Position: refs/heads/master@{#353922}

[Mikhail, 2015-10-12 13:06:32]

Patchset #1 (id:1) has been deleted

[Mikhail, 2015-10-12 14:14:10]

Patchset #1 (id:20001) has been deleted

[Mikhail, 2015-10-12 14:15:29]

mikhail.pozdnyakov@intel.com changed reviewers:
+ tkent@chromium.org

[Mikhail, 2015-10-12 14:18:54]

PTAL

[brucedawson, 2015-10-12 16:49:39]

brucedawson@chromium.org changed reviewers:
+ brucedawson@chromium.org

[brucedawson, 2015-10-12 16:49:40]

Perfect. Much better having the definition in the .cc file rather than .h file.

[tkent, 2015-10-12 23:09:03]

https://codereview.chromium.org/1398973005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/wtf/ASCIICType.cpp (right):

https://codereview.chromium.org/1398973005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/wtf/ASCIICType.cpp:1: // Copyright 2015 The Chromium
Authors. All rights reserved.
Copyright header should be copied from ASCIIType.h.

https://codereview.chromium.org/1398973005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/wtf/ASCIICType.cpp:10: WTF_EXPORT const LChar
ASCIICaseFoldTable[256] = {
Remove WTF_EXPORT.

[brucedawson, 2015-10-12 23:11:03]

https://codereview.chromium.org/1398973005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/wtf/ASCIICType.cpp (right):

https://codereview.chromium.org/1398973005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/wtf/ASCIICType.cpp:10: WTF_EXPORT const LChar
ASCIICaseFoldTable[256] = {
On 2015/10/12 23:09:03, tkent wrote:
> Remove WTF_EXPORT.

WTF_EXPORT has to be there. Little known C++ fact: const implies static (which
is why the definition in the header file didn't cause duplicate definition
errors). Therefore without WTF_EXPORT the array will be invisible outside of
this translation unit.

Strange but true.

[Mikhail, 2015-10-13 11:09:35]

On 2015/10/12 23:11:03, brucedawson wrote:
>
https://codereview.chromium.org/1398973005/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/wtf/ASCIICType.cpp (right):
> 
>
https://codereview.chromium.org/1398973005/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/wtf/ASCIICType.cpp:10: WTF_EXPORT const LChar
> ASCIICaseFoldTable[256] = {
> On 2015/10/12 23:09:03, tkent wrote:
> > Remove WTF_EXPORT.
> 
> WTF_EXPORT has to be there. Little known C++ fact: const implies static (which
> is why the definition in the header file didn't cause duplicate definition
> errors). Therefore without WTF_EXPORT the array will be invisible outside of
> this translation unit.
> 
> Strange but true.

I should have just placed 'extern' there instead (WTF_EXPORT gets ignored
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...).
Fixed in the latest patch.

[tkent, 2015-10-13 23:55:51]

The CQ bit was checked by tkent@chromium.org

[tkent, 2015-10-13 23:55:52]

lgtm

[commit-bot: I haz the power, 2015-10-13 23:57:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1398973005/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1398973005/60001

[commit-bot: I haz the power, 2015-10-14 00:24:00]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2015-10-14 00:25:02]

Patchset 2 (id:??) landed as
https://crrev.com/e3e1da9db2a3916d8680772c7d399f208c295474
Cr-Commit-Position: refs/heads/master@{#353922}

[tfarina, 2015-10-14 00:40:49]

Just curious, which tool did you use on chrome.dll to discover that it had 2473
copies of ASCIICaseFoldTable? If you could paste the command line that would be
cool.

I think on Linux that could be done using nm (I dunno).

[brucedawson, 2015-10-14 00:59:52]

On 2015/10/14 00:40:49, tfarina wrote:
> Just curious, which tool did you use on chrome.dll to discover that it had
2473
> copies of ASCIICaseFoldTable? If you could paste the command line that would
be
> cool.
> 
> I think on Linux that could be done using nm (I dunno).

I used a modified version of dia2dump, configured to collect and report on
duplicates and on large globals. dia2dump ships with Visual Studio in source
form:

https://msdn.microsoft.com/en-us/library/b5ke49f5.aspx