[Extensions] Reduce string copy in renderer/ UserScript.

[Extensions] Reduce string copy in renderer/ UserScript.

Before passing StringPiece script (JS/CSS) content to blink (to create
WebString or WebScriptSource), we sometimes unnecessarily created
std::string. This CL removes the intermediate state.

Greasemonkey scripts still require the intermediate std::string to
pad it with header and tail though. Use reserve and
StringPiece.appendToString() in that case to at least do that
efficiently.

BUG=622464
Test=No visible change.

Committed: https://crrev.com/49cc0b3a4e425a884a0b12639951f84b4f84f693
Cr-Commit-Position: refs/heads/master@{#412946}

[lazyboy, 2016-08-18 02:02:57]

lazyboy@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[lazyboy, 2016-08-18 02:02:58]

Follow up from the other CL.

[lazyboy, 2016-08-18 17:06:02]

The CQ bit was checked by lazyboy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-18 17:06:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Devlin, 2016-08-18 17:08:17]

lgtm

https://codereview.chromium.org/2250263004/diff/1/extensions/renderer/user_sc...
File extensions/renderer/user_script_injector.cc (right):

https://codereview.chromium.org/2250263004/diff/1/extensions/renderer/user_sc...
extensions/renderer/user_script_injector.cc:206: 
sources.reserve()?  (was this done in your other patch and needs to be rebased?

https://codereview.chromium.org/2250263004/diff/1/extensions/renderer/user_sc...
extensions/renderer/user_script_injector.cc:213: // Greasemonkey does:
|script_content| becomes:
nit: replace the first ':' (after 'does') with a '.'.

https://codereview.chromium.org/2250263004/diff/1/extensions/renderer/user_sc...
extensions/renderer/user_script_injector.cc:224:
sources.push_back(blink::WebScriptSource(
We should keep some kind of TODO here to optimize this yet more.

https://codereview.chromium.org/2250263004/diff/1/extensions/renderer/user_sc...
extensions/renderer/user_script_injector.cc:225:
blink::WebString::fromUTF8(script_content.data(),
hmm... I find it slightly more readable when we reuse the construction of the
WebScriptSource and appending to sources, i.e.

blink::WebString source;
if (greasemonkey) {
  std::string content;
  ...
  source = blink::WebString::fromUTF8(content);
} else {
  source = blink::WebString::fromUTF8(script_content.data(),
script_content.lenght());
}
sources.push_back(blink::WebScriptSource(source, iter->url()));

But that's just me, and I don't feel super strongly.

[lazyboy, 2016-08-18 18:50:26]

The CQ bit was checked by lazyboy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-18 18:50:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lazyboy, 2016-08-18 18:53:23]

https://codereview.chromium.org/2250263004/diff/1/extensions/renderer/user_sc...
File extensions/renderer/user_script_injector.cc (right):

https://codereview.chromium.org/2250263004/diff/1/extensions/renderer/user_sc...
extensions/renderer/user_script_injector.cc:206: 
On 2016/08/18 17:08:16, Devlin wrote:
> sources.reserve()?  (was this done in your other patch and needs to be
rebased?

Done, thanks.
No, this wasn't done on my other CL.

https://codereview.chromium.org/2250263004/diff/1/extensions/renderer/user_sc...
extensions/renderer/user_script_injector.cc:213: // Greasemonkey does:
|script_content| becomes:
On 2016/08/18 17:08:16, Devlin wrote:
> nit: replace the first ':' (after 'does') with a '.'.

Done.

https://codereview.chromium.org/2250263004/diff/1/extensions/renderer/user_sc...
extensions/renderer/user_script_injector.cc:224:
sources.push_back(blink::WebScriptSource(
On 2016/08/18 17:08:16, Devlin wrote:
> We should keep some kind of TODO here to optimize this yet more.

Done.

https://codereview.chromium.org/2250263004/diff/1/extensions/renderer/user_sc...
extensions/renderer/user_script_injector.cc:225:
blink::WebString::fromUTF8(script_content.data(),
On 2016/08/18 17:08:16, Devlin wrote:
> hmm... I find it slightly more readable when we reuse the construction of the
> WebScriptSource and appending to sources, i.e.
> 
> blink::WebString source;
> if (greasemonkey) {
>   std::string content;
>   ...
>   source = blink::WebString::fromUTF8(content);
> } else {
>   source = blink::WebString::fromUTF8(script_content.data(),
> script_content.lenght());
> }
> sources.push_back(blink::WebScriptSource(source, iter->url()));
> 
> But that's just me, and I don't feel super strongly.

Done.

[commit-bot: I haz the power, 2016-08-18 20:40:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-18 20:40:46]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[lazyboy, 2016-08-18 21:03:11]

The CQ bit was checked by lazyboy@chromium.org

[lazyboy, 2016-08-18 21:03:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
rdevlin.cronin@chromium.org
Link to the patchset: https://codereview.chromium.org/2250263004/#ps30001
(title: "address comments")

[commit-bot: I haz the power, 2016-08-18 21:04:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-18 21:55:46]

Committed patchset #3 (id:30001)

[commit-bot: I haz the power, 2016-08-18 22:00:19]

Description was changed from

==========
[Extensions] Reduce string copy in renderer/ UserScript.

Before passing StringPiece script (JS/CSS) content to blink (to create
WebString or WebScriptSource), we sometimes unnecessarily created
std::string. This CL removes the intermediate state.

Greasemonkey scripts still require the intermediate std::string to
pad it with header and tail though. Use reserve and
StringPiece.appendToString() in that case to at least do that
efficiently.

BUG=622464
Test=No visible change.
==========

to

==========
[Extensions] Reduce string copy in renderer/ UserScript.

Before passing StringPiece script (JS/CSS) content to blink (to create
WebString or WebScriptSource), we sometimes unnecessarily created
std::string. This CL removes the intermediate state.

Greasemonkey scripts still require the intermediate std::string to
pad it with header and tail though. Use reserve and
StringPiece.appendToString() in that case to at least do that
efficiently.

BUG=622464
Test=No visible change.

Committed: https://crrev.com/49cc0b3a4e425a884a0b12639951f84b4f84f693
Cr-Commit-Position: refs/heads/master@{#412946}
==========

[commit-bot: I haz the power, 2016-08-18 22:00:20]

Patchset 3 (id:??) landed as
https://crrev.com/49cc0b3a4e425a884a0b12639951f84b4f84f693
Cr-Commit-Position: refs/heads/master@{#412946}