Introduce webkit_glue::Latin1OrUTF16ToUTF16

Introduce webkit_glue::Latin1OrUTF16ToUTF16

This function is helpful for converting WebStrings to string16s. See
https://codereview.chromium.org/15866003/ for more context about why we need
this function defined in base rather than in Blink.

R=darin

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=202644

[abarth-chromium, 2013-05-24 07:35:20]

+brettw: Darin thought we might benefit from your taking a look at this CL (and
the related Blink CL I just CCed you on).

[darin (slow to review), 2013-05-24 23:53:31]

Thinking about this more, perhaps this should live under src/webkit/common/
instead.

My main concern with doing that is that it introduces yet another include
from Blink to Chromium, but I guess we'll just have to deal with that.

-Darin


On Fri, May 24, 2013 at 12:35 AM, <abarth@chromium.org> wrote:

> +brettw: Darin thought we might benefit from your taking a look at this CL
> (and
> the related Blink CL I just CCed you on).
>
>
https://codereview.chromium.**org/15720004/<https://codereview.chromium.org/1...
>

[darin (slow to review), 2013-05-24 23:54:08]

Shortly, src/webkit/common/ is going to be folded into src/content/common/,
so we will have to fixup WebString.h at least once.

-Darin


On Fri, May 24, 2013 at 4:53 PM, Darin Fisher <darin@chromium.org> wrote:

> Thinking about this more, perhaps this should live under
> src/webkit/common/ instead.
>
> My main concern with doing that is that it introduces yet another include
> from Blink to Chromium, but I guess we'll just have to deal with that.
>
> -Darin
>
>
> On Fri, May 24, 2013 at 12:35 AM, <abarth@chromium.org> wrote:
>
>> +brettw: Darin thought we might benefit from your taking a look at this
>> CL (and
>> the related Blink CL I just CCed you on).
>>
>>
https://codereview.chromium.**org/15720004/<https://codereview.chromium.org/1...
>>
>
>

[brettw, 2013-05-28 18:18:44]

If we can put this in content/common I think that would be better. It does seem
weird to have this depend on content, though, but I have a hard time picturing
the final design. 

Will there be any other content dependencies? It seems like this may happen more
with Blink, and maybe we should have a plan. It seems like if we are going to do
this for more stuff, adding all shared code to base doesn't seem scalable.

https://codereview.chromium.org/15720004/diff/1/base/strings/latin1_string_co...
File base/strings/latin1_string_conversions.h (right):

https://codereview.chromium.org/15720004/diff/1/base/strings/latin1_string_co...
base/strings/latin1_string_conversions.h:26: BASE_EXPORT string16
Latin1OrUTF16ToUTF16(
I'd make the args align horizontally (all on separate lines). The style guide
encourages this implicitly for declarations and implementations.

[abarth-chromium, 2013-05-28 18:19:27]

There doesn't seem to be a way to put code in webkit/common.  The code in that
directory gets sharded up into a bunch of different DLLs, none of which seem
appropriate for this code.

Instead, I've moved it to webkit/glue.  PTAL.

[abarth-chromium, 2013-05-28 18:21:04]

On 2013/05/28 18:18:44, brettw wrote:
> If we can put this in content/common I think that would be better.

It can't go in content/common because it needs to be linked into DumpRenderTree
while that still exists.

>
https://codereview.chromium.org/15720004/diff/1/base/strings/latin1_string_co...
> File base/strings/latin1_string_conversions.h (right):
> 
>
https://codereview.chromium.org/15720004/diff/1/base/strings/latin1_string_co...
> base/strings/latin1_string_conversions.h:26: BASE_EXPORT string16
> Latin1OrUTF16ToUTF16(
> I'd make the args align horizontally (all on separate lines). The style guide
> encourages this implicitly for declarations and implementations.

Will do.

Thanks for taking a look.

[darin (slow to review), 2013-05-28 18:21:42]

we're working to delete webkit/glue by sharding into one of
webkit/{browser,common,renderer} all in anticipation of moving all of that code
to content/{browser,common,renderer}.  Both webkit/browser and webkit/renderer
get to depend on webkit/common.  That makes it kind of like base/.  Seems like
this function should go there somehow.  +jamesr

[abarth-chromium, 2013-05-28 18:24:23]

On 2013/05/28 18:21:42, darin wrote:
> Seems like this function should go there somehow.

Ok, I can make a new compilation target to hold this code if you like.

[darin (slow to review), 2013-05-28 18:30:01]

Oh, I see... we don't have that target yet :-/

In that case, using glue is probably fine for now.

[jamesr, 2013-05-28 18:30:35]

On 2013/05/28 18:24:23, abarth wrote:
> On 2013/05/28 18:21:42, darin wrote:
> > Seems like this function should go there somehow.
> 
> Ok, I can make a new compilation target to hold this code if you like.

That sounds right.  The mapping of files in webkit/ to targets generally
reflects the topic of the code and not the directory structure.  I would
generally not follow the pattern set by the files there.

[jamesr, 2013-05-28 18:31:12]

By putting this in content/common/, do we mean putting it in
content/public/common/ or do we intend to forbid use of WebString outside of
content/ ?

[jamesr, 2013-05-28 18:36:28]

lgtm

at some point soon we'll want to move this header, which'll mean an #ifdef dance
since this will be #included in WebString.h, but I think that's manageable

[commit-bot: I haz the power, 2013-05-28 18:37:52]

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

[abarth-chromium, 2013-05-28 18:38:01]

Thanks everyone.  Sorry to causing so much consternation over this tiny
function.  I blame wstring for being 32 bits wide.

[commit-bot: I haz the power, 2013-05-28 20:34:27]

Change committed as 202644