Reduce the copying of string data between C++ and javascript in proxy_resolver_v8.cc.

Optimization: reduce the copying of string data between C++ and javascript in proxy_resolver_v8.cc. 

This is done by sharing the string storage using ExternalStringResource. 

An accompanying change was to pass around the PAC script data as a UTF16 string16 rather than a UTF8 std::string -- this required changing plumbing in the other files. 

This optimization will be important when creating multiple ProxyResolverV8's so they don't end up duplicating the script text. 

BUG=11079
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=51434

[eroman, 2010-06-30 02:37:28]

iposva: Can you please review the single file: net/proxy/proxy_resolver_v8.cc
(http://codereview.chromium.org/2817043/diff/17003/25010)

willchan: Can you please review the whole CL (aside from proxy_resolver_v8.cc it
is just plumbing changes).

[willchan no longer on Chromium, 2010-07-01 06:40:18]

LGTM

http://codereview.chromium.org/2817043/diff/17003/25009
File net/proxy/proxy_resolver_perftest.cc (right):

http://codereview.chromium.org/2817043/diff/17003/25009#newcode169
net/proxy/proxy_resolver_perftest.cc:169: int rv =
resolver_->SetPacScriptByData(ASCIIToUTF16(file_contents), NULL);
i think you need to include string_util.h for this

http://codereview.chromium.org/2817043/diff/17003/25010
File net/proxy/proxy_resolver_v8.cc (right):

http://codereview.chromium.org/2817043/diff/17003/25010#newcode135
net/proxy/proxy_resolver_v8.cc:135:
DISALLOW_COPY_AND_ASSIGN(V8ExternalASCIIString);
Need basictypes.h

http://codereview.chromium.org/2817043/diff/17003/25022
File net/proxy/single_threaded_proxy_resolver_unittest.cc (right):

http://codereview.chromium.org/2817043/diff/17003/25022#newcode6
net/proxy/single_threaded_proxy_resolver_unittest.cc:6: #include
"base/string_util.h"
alphabetical

[iposva, 2010-07-01 17:13:13]

http://codereview.chromium.org/2817043/diff/17003/25010
File net/proxy/proxy_resolver_v8.cc (right):

http://codereview.chromium.org/2817043/diff/17003/25010#newcode292
net/proxy/proxy_resolver_v8.cc:292:
global_template->Set(ASCIILiteralToV8String("alert"), alert_template);
For short strings like these the memory savings of using external strings is
either none or even negative. You might want to keep the old code for this.

Probably a balanced approach would be better for the string handling here.

[eroman, 2010-07-01 18:32:54]

http://codereview.chromium.org/2817043/diff/17003/25009
File net/proxy/proxy_resolver_perftest.cc (right):

http://codereview.chromium.org/2817043/diff/17003/25009#newcode169
net/proxy/proxy_resolver_perftest.cc:169: int rv =
resolver_->SetPacScriptByData(ASCIIToUTF16(file_contents), NULL);
On 2010/07/01 06:40:19, willchan wrote:
> i think you need to include string_util.h for this

Done.

http://codereview.chromium.org/2817043/diff/17003/25010
File net/proxy/proxy_resolver_v8.cc (right):

http://codereview.chromium.org/2817043/diff/17003/25010#newcode135
net/proxy/proxy_resolver_v8.cc:135:
DISALLOW_COPY_AND_ASSIGN(V8ExternalASCIIString);
On 2010/07/01 06:40:19, willchan wrote:
> Need basictypes.h

Done.

http://codereview.chromium.org/2817043/diff/17003/25010#newcode292
net/proxy/proxy_resolver_v8.cc:292:
global_template->Set(ASCIILiteralToV8String("alert"), alert_template);
On 2010/07/01 17:13:14, iposva wrote:
> For short strings like these the memory savings of using external strings is
> either none or even negative. You might want to keep the old code for this.
> 
> Probably a balanced approach would be better for the string handling here.

Done.

I have introduces a cut-off length of 256 bytes.

So for strings whose byte length is <= 256, we make a copy as before.
But for strings >256 bytes, we wrap the storage.

http://codereview.chromium.org/2817043/diff/17003/25022
File net/proxy/single_threaded_proxy_resolver_unittest.cc (right):

http://codereview.chromium.org/2817043/diff/17003/25022#newcode6
net/proxy/single_threaded_proxy_resolver_unittest.cc:6: #include
"base/string_util.h"
On 2010/07/01 06:40:19, willchan wrote:
> alphabetical

Done.

[iposva, 2010-07-01 18:39:36]

The V8 pieces LGTM!

-Ivan