Make calls to AtomicString(const String&) explicit in html/

Make calls to AtomicString(const String&) explicit in html/

Make calls to AtomicString(const String&) explicit in html/ and avoid
useless conversions from String to AtomicString when possible.

This is one step towards making the AtomicString(const String&) constructor
explicit. This CL takes care only of the html/ part so that it is more
easily reviewable.

R=eseidel
BUG=323739
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=164418

[eseidel, 2013-12-31 23:22:35]

lgtm

https://codereview.chromium.org/108313015/diff/1/Source/core/html/HTMLAnchorE...
File Source/core/html/HTMLAnchorElement.cpp (left):

https://codereview.chromium.org/108313015/diff/1/Source/core/html/HTMLAnchorE...
Source/core/html/HTMLAnchorElement.cpp:347: void
HTMLAnchorElement::setInput(const String& value)
Probably makes sense as an AtomicString.  AtomicString is all about storage for
long-lived strings.  Exposing accesors which return AtomicSTrings means "I'm
holding onto this common string", and taking an AtomicString is "whatever you
apss into me is likely a common string which I'm holding on to".

In any case, if tehre are reasons (like this being a virtual) why this shouldn't
be AtomicString yet, that's fine too.

https://codereview.chromium.org/108313015/diff/1/Source/core/html/HTMLFrameEl...
File Source/core/html/HTMLFrameElementBase.cpp (right):

https://codereview.chromium.org/108313015/diff/1/Source/core/html/HTMLFrameEl...
Source/core/html/HTMLFrameElementBase.cpp:79: m_URL =
AtomicString(blankURL().string());
KURL::toAtomicString().

https://codereview.chromium.org/108313015/diff/1/Source/core/html/HTMLImageEl...
File Source/core/html/HTMLImageElement.cpp (right):

https://codereview.chromium.org/108313015/diff/1/Source/core/html/HTMLImageEl...
Source/core/html/HTMLImageElement.cpp:130: m_bestFitImageURL =
AtomicString(candidate.toString());
toAtomicSTring?

https://codereview.chromium.org/108313015/diff/1/Source/core/html/HTMLInputEl...
File Source/core/html/HTMLInputElement.cpp (right):

https://codereview.chromium.org/108313015/diff/1/Source/core/html/HTMLInputEl...
Source/core/html/HTMLInputElement.cpp:1804: parameters.locale =
AtomicString(defaultLanguage());
language shoudl always be an AtomicSTring, surprised defaultlanguage isn't. 
Maybe beacuse it's used on more than one thread?

https://codereview.chromium.org/108313015/diff/1/Source/core/html/HTMLMediaEl...
File Source/core/html/HTMLMediaElement.cpp (right):

https://codereview.chromium.org/108313015/diff/1/Source/core/html/HTMLMediaEl...
Source/core/html/HTMLMediaElement.cpp:2086: WTF_LOG(Media,
"HTMLMediaElement::setPreload(%s)", preload.string().utf8().data());
I'm a little surprsied that AtomicSTring doens't have a utf8 accessor yet.

I had wanted STringView to be something like this.  A common interface onto all
UChar buffers which exposed string-like fuctions w/o actually holding onto its
data.  But we we haven't gotten to that yet.

https://codereview.chromium.org/108313015/diff/1/Source/core/html/forms/FormC...
File Source/core/html/forms/FormController.cpp (right):

https://codereview.chromium.org/108313015/diff/1/Source/core/html/forms/FormC...
Source/core/html/forms/FormController.cpp:456: AtomicString formKey =
AtomicString(stateVector[i++]);
I guess the stateVector should eventually be Vector<AtomicSTring> but I can also
understand the desire to kepe this change small.

[Inactive, 2014-01-01 19:26:28]

https://codereview.chromium.org/108313015/diff/1/Source/core/html/HTMLAnchorE...
File Source/core/html/HTMLAnchorElement.cpp (left):

https://codereview.chromium.org/108313015/diff/1/Source/core/html/HTMLAnchorE...
Source/core/html/HTMLAnchorElement.cpp:347: void
HTMLAnchorElement::setInput(const String& value)
On 2013/12/31 23:22:35, eseidel wrote:
> Probably makes sense as an AtomicString.  AtomicString is all about storage
for
> long-lived strings.  Exposing accesors which return AtomicSTrings means "I'm
> holding onto this common string", and taking an AtomicString is "whatever you
> apss into me is likely a common string which I'm holding on to".
> 
> In any case, if tehre are reasons (like this being a virtual) why this
shouldn't
> be AtomicString yet, that's fine too.

The reason I did not update it is because input() / setInput() are virtual and
overridden in WorkerLocation.h as well.

https://codereview.chromium.org/108313015/diff/1/Source/core/html/HTMLFrameEl...
File Source/core/html/HTMLFrameElementBase.cpp (right):

https://codereview.chromium.org/108313015/diff/1/Source/core/html/HTMLFrameEl...
Source/core/html/HTMLFrameElementBase.cpp:79: m_URL =
AtomicString(blankURL().string());
On 2013/12/31 23:22:35, eseidel wrote:
> KURL::toAtomicString().

Yes, planning to add this KURL method in a follow-up.

https://codereview.chromium.org/108313015/diff/1/Source/core/html/HTMLInputEl...
File Source/core/html/HTMLInputElement.cpp (right):

https://codereview.chromium.org/108313015/diff/1/Source/core/html/HTMLInputEl...
Source/core/html/HTMLInputElement.cpp:1804: parameters.locale =
AtomicString(defaultLanguage());
On 2013/12/31 23:22:35, eseidel wrote:
> language shoudl always be an AtomicSTring, surprised defaultlanguage isn't. 
> Maybe beacuse it's used on more than one thread?

It does not look like it is used in more than one thread so I will give it a
try.

https://codereview.chromium.org/108313015/diff/1/Source/core/html/HTMLMediaEl...
File Source/core/html/HTMLMediaElement.cpp (right):

https://codereview.chromium.org/108313015/diff/1/Source/core/html/HTMLMediaEl...
Source/core/html/HTMLMediaElement.cpp:2086: WTF_LOG(Media,
"HTMLMediaElement::setPreload(%s)", preload.string().utf8().data());
On 2013/12/31 23:22:35, eseidel wrote:
> I'm a little surprsied that AtomicSTring doens't have a utf8 accessor yet.

Right, this would be convenient. I will propose this in a follow-up CL as I know
it would be useful in other places as well.

https://codereview.chromium.org/108313015/diff/1/Source/core/html/forms/FormC...
File Source/core/html/forms/FormController.cpp (right):

https://codereview.chromium.org/108313015/diff/1/Source/core/html/forms/FormC...
Source/core/html/forms/FormController.cpp:456: AtomicString formKey =
AtomicString(stateVector[i++]);
On 2013/12/31 23:22:35, eseidel wrote:
> I guess the stateVector should eventually be Vector<AtomicSTring> but I can
also
> understand the desire to kepe this change small.

I took a look at it and it looks like this change may be big. I'd rather leave
it out of this CL and address it separately.

[Inactive, 2014-01-01 19:28:06]

+haraken for bindings/

I needed to add support for returning a Vector<AtomicString> to the bindings.

[haraken, 2014-01-02 03:33:49]

LGTM for bindings/

[Inactive, 2014-01-02 13:11:11]

On 2014/01/02 03:33:49, haraken wrote:
> LGTM for bindings/

Thanks.
Eric, does the latest patchset still look Ok to you?

[eseidel, 2014-01-02 19:18:27]

lgtm

https://codereview.chromium.org/108313015/diff/340001/Source/core/html/HTMLFr...
File Source/core/html/HTMLFrameElementBase.cpp (right):

https://codereview.chromium.org/108313015/diff/340001/Source/core/html/HTMLFr...
Source/core/html/HTMLFrameElementBase.cpp:79: m_URL =
AtomicString(blankURL().string());
I guess you're adding KURL::toAtomicString() in a later patch?

https://codereview.chromium.org/108313015/diff/340001/Source/core/html/HTMLMe...
File Source/core/html/HTMLMediaElement.cpp (right):

https://codereview.chromium.org/108313015/diff/340001/Source/core/html/HTMLMe...
Source/core/html/HTMLMediaElement.cpp:2086: WTF_LOG(Media,
"HTMLMediaElement::setPreload(%s)", preload.string().utf8().data());
Yup, definitely should add an AtomicString::utf8().  Obviously in a follow-up
patch.

https://codereview.chromium.org/108313015/diff/340001/Source/core/html/HTMLVi...
File Source/core/html/HTMLViewSourceDocument.h (right):

https://codereview.chromium.org/108313015/diff/340001/Source/core/html/HTMLVi...
Source/core/html/HTMLViewSourceDocument.h:61: int addRange(const String& source,
int start, int end, const AtomicString& className, bool isLink = false, bool
isAnchor = false, const AtomicString& link = nullAtom);
Bools as arguments are sooooo bad.  We really need to make a style rule to yell
at us when we try to add more.  (I know this has nothing to do with your patch.

https://codereview.chromium.org/108313015/diff/340001/Source/core/html/parser...
File Source/core/html/parser/AtomicHTMLToken.h (left):

https://codereview.chromium.org/108313015/diff/340001/Source/core/html/parser...
Source/core/html/parser/AtomicHTMLToken.h:167: // FIXME: This is N^2 for the
number of attributes.
I love that this has still never shown up on any benchmark. :)

https://codereview.chromium.org/108313015/diff/340001/Source/core/svg/SVGTest...
File Source/core/svg/SVGTests.cpp (right):

https://codereview.chromium.org/108313015/diff/340001/Source/core/svg/SVGTest...
Source/core/svg/SVGTests.cpp:113: if (value !=
defaultLanguage().string().substring(0, 2))
AtomicString might need substring() too.  not sure.

[Inactive, 2014-01-02 19:34:06]

https://codereview.chromium.org/108313015/diff/340001/Source/core/html/HTMLFr...
File Source/core/html/HTMLFrameElementBase.cpp (right):

https://codereview.chromium.org/108313015/diff/340001/Source/core/html/HTMLFr...
Source/core/html/HTMLFrameElementBase.cpp:79: m_URL =
AtomicString(blankURL().string());
On 2014/01/02 19:18:28, eseidel wrote:
> I guess you're adding KURL::toAtomicString() in a later patch?

Yes, this is on my TODO list.

[commit-bot: I haz the power, 2014-01-02 19:34:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/108313015/340001

[commit-bot: I haz the power, 2014-01-02 19:41:17]

Change committed as 164418