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

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

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

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

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

[eseidel, 2013-12-31 23:14:45]

This seems like more evidence that KURL should have a toAtomicString()?

Also, frameName() probably should be an AtomicString?

Otherwise lgtm.

On Tue, Dec 31, 2013 at 1:00 PM,  <ch.dumez@samsung.com> wrote:
> Reviewers: eseidel,
>
> Description:
> Make calls to AtomicString(const String&) explicit in page/
>
> Make calls to AtomicString(const String&) explicit in page/.
>
> This is one step towards making the AtomicString(const String&) constructor
> explicit. This CL takes care only of the page/ part so that it is more
> easily reviewable.
>
> R=eseidel
> BUG=323739
>
> Please review this at https://codereview.chromium.org/112653006/
>
> SVN Base: https://chromium.googlesource.com/chromium/blink.git@master
>
> Affected files (+10, -10 lines):
>   M Source/core/page/CreateWindow.cpp
>   M Source/core/page/DragController.cpp
>   M Source/core/page/EventHandler.cpp
>   M Source/core/page/EventSource.cpp
>   M Source/core/page/Page.cpp
>   M Source/core/page/PageSerializer.cpp
>
>
> Index: Source/core/page/CreateWindow.cpp
> diff --git a/Source/core/page/CreateWindow.cpp
> b/Source/core/page/CreateWindow.cpp
> index
>
2ba2123c7d6d74504822d705275be27d0da45e50..3bff3de79a295c56baef0d0987d702c782bc4db5
> 100644
> --- a/Source/core/page/CreateWindow.cpp
> +++ b/Source/core/page/CreateWindow.cpp
> @@ -48,7 +48,7 @@ static Frame* createWindow(Frame* openerFrame, Frame*
> lookupFrame, const FrameLo
>      ASSERT(!features.dialog || request.frameName().isEmpty());
>
>      if (!request.frameName().isEmpty() && request.frameName() != "_blank"
> && policy == NavigationPolicyIgnore) {
> -        if (Frame* frame =
> lookupFrame->loader().findFrameForNavigation(request.frameName(),
> openerFrame->document())) {
> +        if (Frame* frame =
>
lookupFrame->loader().findFrameForNavigation(AtomicString(request.frameName()),
> openerFrame->document())) {
>              if (request.frameName() != "_self") {
>                  // FIXME: Why does this call directly to chrome instead of
> FocusController?
>                  if (FrameHost* host = frame->host())
> @@ -85,7 +85,7 @@ static Frame* createWindow(Frame* openerFrame, Frame*
> lookupFrame, const FrameLo
>
> frame->loader().forceSandboxFlags(openerFrame->document()->sandboxFlags());
>
>      if (request.frameName() != "_blank")
> -        frame->tree().setName(request.frameName());
> +        frame->tree().setName(AtomicString(request.frameName()));
>
>      host->chrome().setWindowFeatures(features);
>
> @@ -130,8 +130,8 @@ Frame* createWindow(const String& urlString, const
> AtomicString& frameName, cons
>      // For whatever reason, Firefox uses the first frame to determine the
> outgoingReferrer. We replicate that behavior here.
>      String referrer =
>
SecurityPolicy::generateReferrerHeader(firstFrame->document()->referrerPolicy(),
> completedURL, firstFrame->document()->outgoingReferrer());
>
> -    ResourceRequest request(completedURL, referrer);
> -    FrameLoader::addHTTPOriginIfNeeded(request,
> firstFrame->document()->outgoingOrigin());
> +    ResourceRequest request(completedURL, AtomicString(referrer));
> +    FrameLoader::addHTTPOriginIfNeeded(request,
> AtomicString(firstFrame->document()->outgoingOrigin()));
>      FrameLoadRequest frameRequest(activeWindow->document(), request,
> frameName);
>
>      // We pass the opener frame for the lookupFrame in case the active
> frame is different from
> @@ -151,7 +151,7 @@ Frame* createWindow(const String& urlString, const
> AtomicString& frameName, cons
>          function(newFrame->domWindow(), functionContext);
>
>      if (created) {
> -        FrameLoadRequest request(activeWindow->document(),
> ResourceRequest(completedURL, referrer));
> +        FrameLoadRequest request(activeWindow->document(),
> ResourceRequest(completedURL, AtomicString(referrer)));
>          newFrame->loader().load(request);
>      } else if (!urlString.isEmpty()) {
>
>
newFrame->navigationScheduler().scheduleLocationChange(activeWindow->document(),
> completedURL.string(), referrer, false);
> Index: Source/core/page/DragController.cpp
> diff --git a/Source/core/page/DragController.cpp
> b/Source/core/page/DragController.cpp
> index
>
8a56f75d594ef36f29ce509c49306c31157c5ca4..2eb8351eaf68e30e7f6fb232a0a135c4f383c9fc
> 100644
> --- a/Source/core/page/DragController.cpp
> +++ b/Source/core/page/DragController.cpp
> @@ -166,7 +166,7 @@ static PassRefPtr<DocumentFragment>
> documentFragmentFromDragData(DragData* dragD
>              String url = dragData->asURL(DragData::DoNotConvertFilenames,
> &title);
>              if (!url.isEmpty()) {
>                  RefPtr<HTMLAnchorElement> anchor =
> HTMLAnchorElement::create(document);
> -                anchor->setHref(url);
> +                anchor->setHref(AtomicString(url));
>                  if (title.isEmpty()) {
>                      // Try the plain text first because the url might be
> normalized or escaped.
>                      if (dragData->containsPlainText())
> Index: Source/core/page/EventHandler.cpp
> diff --git a/Source/core/page/EventHandler.cpp
> b/Source/core/page/EventHandler.cpp
> index
>
31400d6cb59d30dc80320912714f506afb879024..8bf1fc90ae38870c911025a96f7edb95ff69eb99
> 100755
> --- a/Source/core/page/EventHandler.cpp
> +++ b/Source/core/page/EventHandler.cpp
> @@ -3196,7 +3196,7 @@ void
> EventHandler::defaultKeyboardEventHandler(KeyboardEvent* event)
>          else if (event->keyIdentifier() == "U+001B")
>              defaultEscapeEventHandler(event);
>          else {
> -            FocusDirection direction =
> focusDirectionForKey(event->keyIdentifier());
> +            FocusDirection direction =
> focusDirectionForKey(AtomicString(event->keyIdentifier()));
>              if (direction != FocusDirectionNone)
>                  defaultArrowEventHandler(direction, event);
>          }
> Index: Source/core/page/EventSource.cpp
> diff --git a/Source/core/page/EventSource.cpp
> b/Source/core/page/EventSource.cpp
> index
>
fcb5e1c7bbff88d5404145c76f7c10ec86a5ec4a..4f2be4ac605d8d3f80e44c4fa2e6cec78ab9f0a3
> 100644
> --- a/Source/core/page/EventSource.cpp
> +++ b/Source/core/page/EventSource.cpp
> @@ -130,7 +130,7 @@ void EventSource::connect()
>      request.setHTTPHeaderField("Accept", "text/event-stream");
>      request.setHTTPHeaderField("Cache-Control", "no-cache");
>      if (!m_lastEventId.isEmpty())
> -        request.setHTTPHeaderField("Last-Event-ID", m_lastEventId);
> +        request.setHTTPHeaderField("Last-Event-ID",
> AtomicString(m_lastEventId));
>
>      SecurityOrigin* origin = executionContext()->securityOrigin();
>
> Index: Source/core/page/Page.cpp
> diff --git a/Source/core/page/Page.cpp b/Source/core/page/Page.cpp
> index
>
2030795e5eb6584c15fcd4400bcc3b572517c5dd..411ad0bb01ece25b6655d08e20f096c9acf292b2
> 100644
> --- a/Source/core/page/Page.cpp
> +++ b/Source/core/page/Page.cpp
> @@ -490,7 +490,7 @@ void Page::settingsChanged(SettingsDelegate::ChangeType
> changeType)
>              mainFrame()->document()->updateViewportDescription();
>          break;
>      case SettingsDelegate::MediaTypeChange:
> -        m_mainFrame->view()->setMediaType(settings().mediaTypeOverride());
> +
>
m_mainFrame->view()->setMediaType(AtomicString(settings().mediaTypeOverride()));
>          setNeedsRecalcStyleInAllFrames();
>          break;
>      case SettingsDelegate::DNSPrefetchingChange:
> Index: Source/core/page/PageSerializer.cpp
> diff --git a/Source/core/page/PageSerializer.cpp
> b/Source/core/page/PageSerializer.cpp
> index
>
401bcbd51b9660f23c64c7a03209ab948fa6b4e1..033f8c592242092ab4025a892c7296d51e664b8c
> 100644
> --- a/Source/core/page/PageSerializer.cpp
> +++ b/Source/core/page/PageSerializer.cpp
> @@ -163,7 +163,7 @@ void
> SerializerMarkupAccumulator::appendCustomAttributes(StringBuilder& out, Ele
>
>      // We need to give a fake location to blank frames so they can be
> referenced by the serialized frame.
>      url = m_serializer->urlForBlankFrame(frame);
> -    appendAttribute(out, element,
> Attribute(frameOwnerURLAttributeName(*frameOwner), url.string()),
> namespaces);
> +    appendAttribute(out, element,
> Attribute(frameOwnerURLAttributeName(*frameOwner),
> AtomicString(url.string())), namespaces);
>  }
>
>  void SerializerMarkupAccumulator::appendEndTag(Node* node)
>
>

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

[Inactive, 2014-01-01 17:10:06]

On 2013/12/31 23:14:45, eseidel wrote:
> This seems like more evidence that KURL should have a toAtomicString()?

Yes, I am planning to introduce it in a follow-up CL.

> Also, frameName() probably should be an AtomicString?

Yes, I updated frameName() in another patch to return an AtomicString and
updated this CL accordingly.

[commit-bot: I haz the power, 2014-01-01 17:12:16]

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

[commit-bot: I haz the power, 2014-01-01 18:27:13]

Change committed as 164415