Fix chromium webview build break

Fix chromium webview build break

CL 12191005 broke the chromium based webview due to differences in system
headers regarding the icu library. This CL provides a workaround.

TBR=jamesr@chromium.org

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

[sgurun-gerrit only, 2013-02-08 04:50:04]

Please take a look, thanks.

[Miguel Garcia, 2013-02-08 10:09:18]

lgtm

You will need Joth or another owner to review it, it goes without saying but
please make sure that content/renderer/date_time_formatter_unittest.cc still
passes fine ok?

https://codereview.chromium.org/12224065/diff/4/content/renderer/date_time_fo...
File content/renderer/date_time_formatter.cc (right):

https://codereview.chromium.org/12224065/diff/4/content/renderer/date_time_fo...
content/renderer/date_time_formatter.cc:109: result =
UTF16ToUTF8(string16(formattedTime.getBuffer(),
Can you add a comment on why we need to convert to string16 and then to
std::string ? I now understand it's a workaround for WebView because the icu
library there is different but it is not straight forward from just looking at
the implementation and there is a (theoretical at least) performance impact

[benm (inactive), 2013-02-08 10:22:08]

https://codereview.chromium.org/12224065/diff/4/content/renderer/date_time_fo...
File content/renderer/date_time_formatter.cc (right):

https://codereview.chromium.org/12224065/diff/4/content/renderer/date_time_fo...
content/renderer/date_time_formatter.cc:109: result =
UTF16ToUTF8(string16(formattedTime.getBuffer(),
I guess we can mitigate this perf impact with an ifdef ANDROID (and a comment as
you suggest)? WDYT?

[Miguel Garcia, 2013-02-08 10:29:10]

Note that Today this is only used in Android anyway (the desktop implementation
uses a multifield semicross platform dialog instead of a native one and does not
need this parsing from WebKit). Therefore I'd say it is somewhat an overkill of
add the ifdef, I am not against it if you feel it's important.

On 2013/02/08 10:22:08, benm wrote:
>
https://codereview.chromium.org/12224065/diff/4/content/renderer/date_time_fo...
> File content/renderer/date_time_formatter.cc (right):
> 
>
https://codereview.chromium.org/12224065/diff/4/content/renderer/date_time_fo...
> content/renderer/date_time_formatter.cc:109: result =
> UTF16ToUTF8(string16(formattedTime.getBuffer(),
> I guess we can mitigate this perf impact with an ifdef ANDROID (and a comment
as
> you suggest)? WDYT?

[benm (inactive), 2013-02-08 11:04:28]

On 2013/02/08 10:29:10, Miguel Garcia wrote:
> Note that Today this is only used in Android anyway (the desktop
implementation
> uses a multifield semicross platform dialog instead of a native one and does
not
> need this parsing from WebKit). Therefore I'd say it is somewhat an overkill
of
> add the ifdef, I am not against it if you feel it's important.

Ah, OK thanks for the clarification. In that case I'll just update the patch
with the comment. Looks like the only OWNER that can approve this is jamesr who
is out at the moment AFAIK, so I will land TBR to fix our build.


> 
> On 2013/02/08 10:22:08, benm wrote:
> >
>
https://codereview.chromium.org/12224065/diff/4/content/renderer/date_time_fo...
> > File content/renderer/date_time_formatter.cc (right):
> > 
> >
>
https://codereview.chromium.org/12224065/diff/4/content/renderer/date_time_fo...
> > content/renderer/date_time_formatter.cc:109: result =
> > UTF16ToUTF8(string16(formattedTime.getBuffer(),
> > I guess we can mitigate this perf impact with an ifdef ANDROID (and a
comment
> as
> > you suggest)? WDYT?

[commit-bot: I haz the power, 2013-02-08 11:28:07]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[mkosiba (inactive), 2013-02-08 11:29:57]

lgtm

[commit-bot: I haz the power, 2013-02-08 11:32:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sgurun@chromium.org/12224065/7001

[commit-bot: I haz the power, 2013-02-08 13:45:52]

Retried try job too often on linux_chromeos for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2013-02-08 13:48:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sgurun@chromium.org/12224065/7001

[commit-bot: I haz the power, 2013-02-08 15:01:44]

Change committed as 181498