Make HTMLNames threadsafe

Make HTMLNames threadsafe

This CL makes the localNames for HTMLNames (and friends) threadsafe by making
them static strings that are created at compile time.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=148132

[abarth-chromium, 2013-04-10 20:48:01]

This is ready for review now!

[eseidel, 2013-04-10 20:53:50]

lgtm.  Please consider the nits below.  Also CCing cevans since he's expressed
interest in these types of core classes before and might be curious to see this
go by.

https://codereview.chromium.org/13686020/diff/5001/Source/WTF/wtf/text/String...
File Source/WTF/wtf/text/StringImpl.h (right):

https://codereview.chromium.org/13686020/diff/5001/Source/WTF/wtf/text/String...
Source/WTF/wtf/text/StringImpl.h:722: const UChar* m_copyData16;
I'm confused by the rename.

https://codereview.chromium.org/13686020/diff/5001/Source/WTF/wtf/text/String...
File Source/WTF/wtf/text/StringStatics.cpp (right):

https://codereview.chromium.org/13686020/diff/5001/Source/WTF/wtf/text/String...
Source/WTF/wtf/text/StringStatics.cpp:84: new (NotNull, (void*)&emptyAtom)
AtomicString("");
I assume this is going to correctly end up with "emptyString()" as the
underlying StringImpl?

https://codereview.chromium.org/13686020/diff/5001/Source/WTF/wtf/text/String...
File Source/WTF/wtf/text/StringStatics.h (left):

https://codereview.chromium.org/13686020/diff/5001/Source/WTF/wtf/text/String...
Source/WTF/wtf/text/StringStatics.h:33: ] interface StorageQuotaCallback {
I love how rietveld picks a random file to compare against when adding new
files. :)

https://codereview.chromium.org/13686020/diff/5001/Source/WebCore/page/Frame.cpp
File Source/WebCore/page/Frame.cpp (right):

https://codereview.chromium.org/13686020/diff/5001/Source/WebCore/page/Frame....
Source/WebCore/page/Frame.cpp:141: void init()
We should just move this to its own file.  WebCoreInit.cpp/WebCoreInit.h, no?

https://codereview.chromium.org/13686020/diff/5001/Source/WebKit/chromium/src...
File Source/WebKit/chromium/src/WebKit.cpp (right):

https://codereview.chromium.org/13686020/diff/5001/Source/WebKit/chromium/src...
Source/WebKit/chromium/src/WebKit.cpp:148: WebCore::init();
This is also really just about initalializing static strings and QualifiedNames.
 Maybe that should be exposed in the naming?

[abarth-chromium, 2013-04-10 21:00:51]

https://codereview.chromium.org/13686020/diff/5001/Source/WTF/wtf/text/String...
File Source/WTF/wtf/text/StringImpl.h (right):

https://codereview.chromium.org/13686020/diff/5001/Source/WTF/wtf/text/String...
Source/WTF/wtf/text/StringImpl.h:722: const UChar* m_copyData16;
On 2013/04/10 20:53:50, Eric Seidel (Google) wrote:
> I'm confused by the rename.

It's related to the union on line 746.  I changed the name to be clear about
which member of the union we're using.

https://codereview.chromium.org/13686020/diff/5001/Source/WTF/wtf/text/String...
File Source/WTF/wtf/text/StringStatics.cpp (right):

https://codereview.chromium.org/13686020/diff/5001/Source/WTF/wtf/text/String...
Source/WTF/wtf/text/StringStatics.cpp:84: new (NotNull, (void*)&emptyAtom)
AtomicString("");
On 2013/04/10 20:53:50, Eric Seidel (Google) wrote:
> I assume this is going to correctly end up with "emptyString()" as the
> underlying StringImpl?

Yes.  This part isn't changing.  I'm just switching the if-statement to be an
early return.

https://codereview.chromium.org/13686020/diff/5001/Source/WTF/wtf/text/String...
File Source/WTF/wtf/text/StringStatics.h (left):

https://codereview.chromium.org/13686020/diff/5001/Source/WTF/wtf/text/String...
Source/WTF/wtf/text/StringStatics.h:33: ] interface StorageQuotaCallback {
On 2013/04/10 20:53:50, Eric Seidel (Google) wrote:
> I love how rietveld picks a random file to compare against when adding new
> files. :)

Yeah, I'm not sure if it's rietveld or git.

https://codereview.chromium.org/13686020/diff/5001/Source/WebCore/page/Frame.cpp
File Source/WebCore/page/Frame.cpp (right):

https://codereview.chromium.org/13686020/diff/5001/Source/WebCore/page/Frame....
Source/WebCore/page/Frame.cpp:141: void init()
On 2013/04/10 20:53:50, Eric Seidel (Google) wrote:
> We should just move this to its own file.  WebCoreInit.cpp/WebCoreInit.h, no?

Yes.  Would you mind if I do that in the next CL?  I want to make all the
initialization explicit, which will dovetail nicely with that change.

https://codereview.chromium.org/13686020/diff/5001/Source/WebKit/chromium/src...
File Source/WebKit/chromium/src/WebKit.cpp (right):

https://codereview.chromium.org/13686020/diff/5001/Source/WebKit/chromium/src...
Source/WebKit/chromium/src/WebKit.cpp:148: WebCore::init();
On 2013/04/10 20:53:50, Eric Seidel (Google) wrote:
> This is also really just about initalializing static strings and
QualifiedNames.
>  Maybe that should be exposed in the naming?

I'd like to move all the initialization code into it.  For example,
WebCore::ImageDecodingStore::initializeOnce() should probably go in there too.

[commit-bot: I haz the power, 2013-04-10 21:03:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/abarth@chromium.org/13686020/5001

[eseidel, 2013-04-10 21:03:40]

sgtm

[commit-bot: I haz the power, 2013-04-10 21:09:39]

Presubmit check for 13686020-5001 failed and returned exit status -2001.
The presubmit check was hung. It took 360.0 seconds to execute and the time
limit is 360.0 seconds.

INFO:root:Found 9 file(s).

[abarth-chromium, 2013-04-11 01:26:37]

Committed patchset #3 manually as r148132 (presubmit successful).

[darin (slow to review), 2013-04-11 07:02:24]

https://codereview.chromium.org/13686020/diff/5001/Source/WTF/wtf/text/String...
File Source/WTF/wtf/text/StringStatics.h (left):

https://codereview.chromium.org/13686020/diff/5001/Source/WTF/wtf/text/String...
Source/WTF/wtf/text/StringStatics.h:33: ] interface StorageQuotaCallback {
On 2013/04/10 21:00:51, abarth wrote:
> On 2013/04/10 20:53:50, Eric Seidel (Google) wrote:
> > I love how rietveld picks a random file to compare against when adding new
> > files. :)
> 
> Yeah, I'm not sure if it's rietveld or git.

I believe this is a side-effect of the similarity thinger tuned to aggressively
try to catch the equivalent of a "svn cp" when your using git.  This was done to
help preserve history in svn.  We could probably come up with a heuristic that
would cause this one to be treated like a new file addition.  @maruel would be
the guy to talk to IIRC.