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

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

Make calls to AtomicString(const String&) explicit in web/ 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 web/ part so that it is more
easily reviewable.

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

[Inactive, 2013-12-31 20:57:18]

https://codereview.chromium.org/122993002/diff/1/Source/web/WebViewImpl.cpp
File Source/web/WebViewImpl.cpp (right):

https://codereview.chromium.org/122993002/diff/1/Source/web/WebViewImpl.cpp#n...
Source/web/WebViewImpl.cpp:2474:
m_page->mainFrame()->loader().reload(NormalReload, KURL(), newEncodingName);
reload() takes the encoding as an AtomicString in argument.

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

framename probably wanst to return an ATomicString.

Surprises me a bit that referrer is an AtomicString everywehre.  If
so, we should just make referrer accessors also atomic srings.

otherwise lgtm.

On Tue, Dec 31, 2013 at 12:57 PM,  <ch.dumez@samsung.com> wrote:
> Reviewers: abarth, eseidel,
>
>
> https://codereview.chromium.org/122993002/diff/1/Source/web/WebViewImpl.cpp
> File Source/web/WebViewImpl.cpp (right):
>
>
https://codereview.chromium.org/122993002/diff/1/Source/web/WebViewImpl.cpp#n...
> Source/web/WebViewImpl.cpp:2474:
> m_page->mainFrame()->loader().reload(NormalReload, KURL(),
> newEncodingName);
> reload() takes the encoding as an AtomicString in argument.
>
> Description:
> Make calls to AtomicString(const String&) explicit in web/
>
> Make calls to AtomicString(const String&) explicit in web/ 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 web/ part so that it is more
> easily reviewable.
>
> R=abarth, eseidel
> BUG=323739
>
> Please review this at https://codereview.chromium.org/122993002/
>
> SVN Base: https://chromium.googlesource.com/chromium/blink.git@master
>
> Affected files (+3, -3 lines):
>   M Source/web/FrameLoaderClientImpl.cpp
>   M Source/web/WebFrameImpl.cpp
>   M Source/web/WebViewImpl.cpp
>
>
> Index: Source/web/FrameLoaderClientImpl.cpp
> diff --git a/Source/web/FrameLoaderClientImpl.cpp
> b/Source/web/FrameLoaderClientImpl.cpp
> index
>
ea1ce2432f9fad4e869b3aff770812fb8218f4a1..fb68b1012ce38573486ec6e8d336a67068f7fcab
> 100644
> --- a/Source/web/FrameLoaderClientImpl.cpp
> +++ b/Source/web/FrameLoaderClientImpl.cpp
> @@ -582,7 +582,7 @@ PassRefPtr<Frame> FrameLoaderClientImpl::createFrame(
>      HTMLFrameOwnerElement* ownerElement)
>  {
>      FrameLoadRequest frameRequest(m_webFrame->frame()->document(),
> -        ResourceRequest(url, referrer), name);
> +        ResourceRequest(url, AtomicString(referrer)), name);
>      return m_webFrame->createChildFrame(frameRequest, ownerElement);
>  }
>
> Index: Source/web/WebFrameImpl.cpp
> diff --git a/Source/web/WebFrameImpl.cpp b/Source/web/WebFrameImpl.cpp
> index
>
a1e3885e53d6abae1546e2c726619178435497f7..3ed6ee14798e68e4fef553b3e4a43fd714b1bcda
> 100644
> --- a/Source/web/WebFrameImpl.cpp
> +++ b/Source/web/WebFrameImpl.cpp
> @@ -2153,7 +2153,7 @@ PassRefPtr<Frame> WebFrameImpl::createChildFrame(const
> FrameLoadRequest& request
>      RefPtr<Frame> childFrame = Frame::create(webframe->m_frameInit);
>      webframe->setWebCoreFrame(childFrame);
>
> -    childFrame->tree().setName(request.frameName());
> +    childFrame->tree().setName(AtomicString(request.frameName()));
>
>      frame()->tree().appendChild(childFrame);
>
> Index: Source/web/WebViewImpl.cpp
> diff --git a/Source/web/WebViewImpl.cpp b/Source/web/WebViewImpl.cpp
> index
>
90b794ad98a9b90f59d1e412771e5147fbacecaa..50420131df9ce8596f2ccacae7417f05548b8c26
> 100644
> --- a/Source/web/WebViewImpl.cpp
> +++ b/Source/web/WebViewImpl.cpp
> @@ -2468,7 +2468,7 @@ void WebViewImpl::setPageEncoding(const WebString&
> encodingName)
>
>      // Only change override encoding, don't change default encoding.
>      // Note that the new encoding must be 0 if it isn't supposed to be set.
> -    String newEncodingName;
> +    AtomicString newEncodingName;
>      if (!encodingName.isEmpty())
>          newEncodingName = encodingName;
>      m_page->mainFrame()->loader().reload(NormalReload, KURL(),
> newEncodingName);
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[Inactive, 2014-01-01 16:59:47]

On 2013/12/31 23:15:35, eseidel wrote:
> framename probably wanst to return an ATomicString.
> 
> Surprises me a bit that referrer is an AtomicString everywehre.  If
> so, we should just make referrer accessors also atomic srings.

I updated the CL to use AtomicString type more consistently for frameName and
target.

Could you please take another look?

I haven't updated referrer yet because I am not sure which way to go for it yet
and I'd like to address it in a follow-up patch if that's OK. The reason
referrer is currently converted to AtomicString frequently is because of
ResourceRequest which stores all its HTTP header fields in a HTTPHeaderMap
(which uses AtomicString for keys and values to save memory).

[eseidel, 2014-01-01 17:03:55]

lgtm

[commit-bot: I haz the power, 2014-01-01 17:04:00]

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

[commit-bot: I haz the power, 2014-01-01 18:00:29]

Change committed as 164414